You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/08/29 06:40:22 UTC
[GitHub] [incubator-nuttx] anchao opened a new pull request, #6947: net/tcp: fix devif callback list corruption on tcp_close()
anchao opened a new pull request, #6947:
URL: https://github.com/apache/incubator-nuttx/pull/6947
## Summary
net/tcp: fix devif callback list corruption on tcp_close()
devif_conn_event() will be called recursively in the psock_send_eventhandler(),
if the tcp event tcp_close_eventhandler() is marked as "next" in first devif_conn_event()
and released from sencond recursive call, the "next" event in the first devif_conn_event()
will become a wild pointer.
```
479 uint16_t devif_conn_event(FAR struct net_driver_s *dev, uint16_t flags,
480 FAR struct devif_callback_s *list)
481 {
482 FAR struct devif_callback_s *next;
...
488 net_lock();
489 while (list && flags)
490 {
...
496 next = list->nxtconn; <------------------ event tcp_close_eventhandler() on next
...
500 if (list->event != NULL && devif_event_trigger(flags, list->flags))
501 {
...
507 flags = list->event(dev, list->priv, flags); <---------------- perform psock_send_eventhandler(), event tcp_close_eventhandler() will be remove from tcp_lost_connection()
508 }
...
512 list = next; <---------------- event tcp_close_eventhandler() has been released, wild pointer
513 }
514
515 net_unlock();
516 return flags;
517 }
```
The callstack as below:
```
Breakpoint 1, tcp_close_eventhandler (dev=0x56607d80 <g_sim_dev>, pvpriv=0x566084a0 <g_tcp_connections>, flags=65) at tcp/tcp_close.c:83
(gdb) bt
| #0 tcp_close_eventhandler (dev=0x56607d80 <g_sim_dev>, pvpriv=0x566084a0 <g_tcp_connections>, flags=65) at tcp/tcp_close.c:83
| #1 0x5658bb57 in devif_conn_event (dev=0x56607d80 <g_sim_dev>, flags=65, list=0x56609498 <g_cbprealloc+312>) at devif/devif_callback.c:507
----------------> devif_conn_event() recursively
| #2 0x56589f8c in tcp_callback (dev=0x56607d80 <g_sim_dev>, conn=0x566084a0 <g_tcp_connections>, flags=65) at tcp/tcp_callback.c:169
| #3 0x565c55e4 in tcp_shutdown_monitor (conn=0x566084a0 <g_tcp_connections>, flags=65) at tcp/tcp_monitor.c:211
| #4 0x565c584b in tcp_lost_connection (conn=0x566084a0 <g_tcp_connections>, cb=0x566094b0 <g_cbprealloc+336>, flags=65) at tcp/tcp_monitor.c:391
| #5 0x565c028a in psock_send_eventhandler (dev=0x56607d80 <g_sim_dev>, pvpriv=0x566084a0 <g_tcp_connections>, flags=65) at tcp/tcp_send_buffered.c:544
----------------> call psock_send_eventhandler() before tcp_close_eventhandler()
| #6 0x5658bb57 in devif_conn_event (dev=0x56607d80 <g_sim_dev>, flags=65, list=0x566094b0 <g_cbprealloc+336>) at devif/devif_callback.c:507
| #7 0x56589f8c in tcp_callback (dev=0x56607d80 <g_sim_dev>, conn=0x566084a0 <g_tcp_connections>, flags=65) at tcp/tcp_callback.c:169
| #8 0x5658e8cc in tcp_input (dev=0x56607d80 <g_sim_dev>, domain=2 '\002', iplen=20) at tcp/tcp_input.c:1059
| #9 0x5658ed77 in tcp_ipv4_input (dev=0x56607d80 <g_sim_dev>) at tcp/tcp_input.c:1355
| #10 0x5658c0a2 in ipv4_input (dev=0x56607d80 <g_sim_dev>) at devif/ipv4_input.c:358
| #11 0x56577017 in netdriver_recv_work (arg=0x56607d80 <g_sim_dev>) at sim/up_netdriver.c:182
| #12 0x5655999e in work_thread (argc=2, argv=0xf3db5dd0) at wqueue/kwork_thread.c:178
| #13 0x5655983f in nxtask_start () at task/task_start.c:129
(gdb) c
Continuing.
Breakpoint 1, tcp_close_eventhandler (dev=0x56607d80 <g_sim_dev>, pvpriv=0x566084a0 <g_tcp_connections>, flags=65) at tcp/tcp_close.c:83
(gdb) bt
| #0 tcp_close_eventhandler (dev=0x56607d80 <g_sim_dev>, pvpriv=0x566084a0 <g_tcp_connections>, flags=65) at tcp/tcp_close.c:83
----------------------> "next" corrupted, invaild call tcp_close_eventhandler()
| #1 0x5658bb57 in devif_conn_event (dev=0x56607d80 <g_sim_dev>, flags=65, list=0x56609498 <g_cbprealloc+312>) at devif/devif_callback.c:507
| #2 0x56589f8c in tcp_callback (dev=0x56607d80 <g_sim_dev>, conn=0x566084a0 <g_tcp_connections>, flags=65) at tcp/tcp_callback.c:169
| #3 0x5658e8cc in tcp_input (dev=0x56607d80 <g_sim_dev>, domain=2 '\002', iplen=20) at tcp/tcp_input.c:1059
| #4 0x5658ed77 in tcp_ipv4_input (dev=0x56607d80 <g_sim_dev>) at tcp/tcp_input.c:1355
| #5 0x5658c0a2 in ipv4_input (dev=0x56607d80 <g_sim_dev>) at devif/ipv4_input.c:358
| #6 0x56577017 in netdriver_recv_work (arg=0x56607d80 <g_sim_dev>) at sim/up_netdriver.c:182
| #7 0x5655999e in work_thread (argc=2, argv=0xf3db5dd0) at wqueue/kwork_thread.c:178
| #8 0x5655983f in nxtask_start () at task/task_start.c:129
(gdb) c
Continuing.
[ 2.680000] up_assert: Assertion failed at file:devif/devif_callback.c line: 85 task: lpwork
```
Signed-off-by: chao.an <an...@xiaomi.com>
## Impact
## Testing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6947: net/tcp: fix devif callback list corruption on tcp_close()
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6947:
URL: https://github.com/apache/incubator-nuttx/pull/6947#issuecomment-1229962860
@fjpanag could you try this patch with your stability test?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #6947: net/tcp: fix devif callback list corruption on tcp_close()
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6947:
URL: https://github.com/apache/incubator-nuttx/pull/6947#issuecomment-1230075199
> > @fjpanag could you try this patch with your stability test?
>
> Yes, I will!
>
> Today it is a bit rough, we are deploying. At most by tomorrow morning, the tests will have been started.
>
> I will report back.
Ok. So let's wait with merge till tomorrow
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] fjpanag commented on pull request #6947: net/tcp: fix devif callback list corruption on tcp_close()
Posted by GitBox <gi...@apache.org>.
fjpanag commented on PR #6947:
URL: https://github.com/apache/incubator-nuttx/pull/6947#issuecomment-1231382769
I did lot's of testing on this, both manually and stress tests. I tested various error conditions.
No crashes anymore.
I noticed that `tcp_close_eventhandler()` is not called in all cases, but in my limited time I couldn't determine whether this is intentional, so I concentrated on stability tests. Since the issue is identified on #6956, I think we can ignore it here.
I propose to merge this, everything seems fine.
The system seems very stable now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #6947: net/tcp: fix devif callback list corruption on tcp_close()
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6947:
URL: https://github.com/apache/incubator-nuttx/pull/6947
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anchao commented on pull request #6947: net/tcp: fix devif callback list corruption on tcp_close()
Posted by GitBox <gi...@apache.org>.
anchao commented on PR #6947:
URL: https://github.com/apache/incubator-nuttx/pull/6947#issuecomment-1229849549
@fjpanag ,related issue:
https://github.com/apache/incubator-nuttx/pull/6923#issuecomment-1228726044
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] fjpanag commented on pull request #6947: net/tcp: fix devif callback list corruption on tcp_close()
Posted by GitBox <gi...@apache.org>.
fjpanag commented on PR #6947:
URL: https://github.com/apache/incubator-nuttx/pull/6947#issuecomment-1230010428
> @fjpanag could you try this patch with your stability test?
Yes, I will!
Today it is a bit rough, we are deploying.
At most by tomorrow morning, the tests will have been started.
I will report back.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org