Swatantra-66 opened a new pull request, #20641:
URL: https://github.com/apache/nuttx/pull/20641
## Summary
Fixes #19849.
Under network disruption (rapid reconnects, socket resets, peer resets), a
race condition between `tcp_timer_expiry()` and `tcp_stop_timer()` could cause
a kernel crash (`Load Access Fault` on `offsetof(struct net_driver_s, d_lock)`,
mcause 5, mtval 0x24) or leave the network stack permanently wedged.
### Root Causes
1. **Use-after-free race condition**:
`tcp_stop_timer()` used asynchronous `work_cancel(LPWORK, &conn->work)`.
When `tcp_timer_expiry()` had already been dequeued and was executing on the
`LPWORK` worker thread, `work_cancel()` returned without cancelling the active
callback. As a result, `tcp_free()` proceeded to clear and zero out the
connection memory via `net_bufpool_free()` (`memset(net_bufnode, 0, ...)`),
leaving the running `tcp_timer_expiry()` callback to access zeroed/freed
connection memory.
2. **Missing NULL check on `conn->dev`**:
`tcp_timer_expiry()` read `conn->dev` without checking for `NULL` before
calling `netdev_lock(conn->dev)`, `netdev_txnotify_dev(conn->dev, TCP_POLL)`,
and `netdev_unlock(conn->dev)`. When `conn->dev` was `NULL` (or cleared by
`tcp_free`), dereferencing `dev->d_lock` triggered a `Load Access Fault`.
### Changes Made
1. **Synchronous timer cancellation**: Replaced `work_cancel(LPWORK,
&conn->work)` with `work_cancel_sync(LPWORK, &conn->work)` in
`tcp_stop_timer()`. This guarantees that if a timer expiry callback is
currently executing, `tcp_free()` will wait for it to finish before releasing
or zeroing the connection structure. (`work_cancel_sync()` safely excludes the
calling worker thread to prevent self-deadlock if ever called from the same
worker).
2. **Defensive NULL validation**: In `tcp_timer_expiry()`, captured `dev =
conn->dev` under `tcp_conn_list_lock()` and guarded the lock and notify
operations with `if (dev != NULL)`.
## Impact
- **Is new feature added?** No
- **Is existing feature changed?** No (bugfix only)
- **Impact on user**: Resolves crashes and hangs during rapid socket closes,
cable disconnects, and network reconnects.
- **Impact on build**: None
- **Impact on hardware**: All architectures using TCP over NuttX work queues.
## Testing
- Verified `tcp_timer_expiry()` safely handles `conn->dev == NULL`.
- Confirmed `work_cancel_sync(LPWORK, &conn->work)` safely synchronizes work
cancellation and excludes worker self-deadlock via `kwork_cancel.c`.
- Follows NuttX C coding style standards.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
