You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by xi...@apache.org on 2022/08/30 11:41:23 UTC
[incubator-nuttx] branch master updated: net/tcp: fix devif callback list corruption on tcp_close()
This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
The following commit(s) were added to refs/heads/master by this push:
new bf6cbbca5d net/tcp: fix devif callback list corruption on tcp_close()
bf6cbbca5d is described below
commit bf6cbbca5d165ed9be6712cd1ce8b7353d38e356
Author: chao.an <an...@xiaomi.com>
AuthorDate: Mon Aug 29 13:30:05 2022 +0800
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>
---
net/tcp/tcp_monitor.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/tcp/tcp_monitor.c b/net/tcp/tcp_monitor.c
index 3df9a061e9..b9c24bca29 100644
--- a/net/tcp/tcp_monitor.c
+++ b/net/tcp/tcp_monitor.c
@@ -208,7 +208,6 @@ static void tcp_shutdown_monitor(FAR struct tcp_conn_s *conn, uint16_t flags)
*/
net_lock();
- tcp_callback(conn->dev, conn, flags);
/* Free all allocated connection event callback structures */