Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net/tcp: Fix clear condition in ofoseg input #9041

Merged
merged 2 commits into from Apr 22, 2023

Conversation

wengzhe
Copy link
Contributor

@wengzhe wengzhe commented Apr 19, 2023

Summary

Fix clear condition in TCP ofoseg input (when receiving an out-of-ordered ACK)

We have a case that an http server gives out-of-ordered ACKs, and NuttX client makes ofosegs with length 0, trying to rebuild / put them into ofosegs array, which is not intended (no available data and should be skipped). This breaks later logic and finally crashed in tcp_ofoseg_bufsize (ofosegs[i].data is NULL, which should never happen in normal logic).

Note:

  • iob_trimhead won't return NULL when it's applying on normal IOB.
    • Keep dev->d_iob == NULL to avoid iob_trimhead changed.
  • iob_free_chain will do nothing when applied to NULL.

Patch also included:

  • mm/iob: Don't return NULL in iob_pack
    • The direct reason which causes ofosegs[i].data become NULL in tcp_ofoseg_bufsize when there are several ofosegs with length 0.

Impact

TCP Out-of-order segment process, especially when receiving an out-of-ordered ACK.

Testing

Manually.

@alancassis
Copy link

@wengzhe please explain the issue you were facing (like you started to do in the Testing comment: "We have a case that an http server gives an out-of-ordered ACK, and triggers problem on NuttX client."). Also please include it in the log message

@wengzhe
Copy link
Contributor Author

wengzhe commented Apr 20, 2023

@wengzhe please explain the issue you were facing (like you started to do in the Testing comment: "We have a case that an http server gives an out-of-ordered ACK, and triggers problem on NuttX client."). Also please include it in the log message

Done.

@xiaoxiang781216
Copy link
Contributor

@wengzhe please rebase your change.

We have a case that an http server gives out-of-ordered ACKs, and NuttX client makes `ofoseg`s with length 0, trying to rebuild / put them into `ofosegs` array, which is not intended (no available data and should be skipped). This breaks later logic and finally crashed in `tcp_ofoseg_bufsize` (`ofosegs[i].data` is `NULL`, which should never happen in normal logic).

Note:
- `iob_trimhead` won't return `NULL` when it's applying on normal IOB.
  - Keep `dev->d_iob == NULL` to avoid `iob_trimhead` changed.
- `iob_free_chain` will do nothing when applied to `NULL`.

Signed-off-by: Zhe Weng <wengzhe@xiaomi.com>
We don't want to get a NULL pointer after iob_pack on an IOB chain with
several iobs with length 0, it should return one IOB with length 0.
Otherwise each place calls iob_pack needs to check the result.

Signed-off-by: Zhe Weng <wengzhe@xiaomi.com>
@xiaoxiang781216 xiaoxiang781216 merged commit 44a0473 into apache:master Apr 22, 2023
26 checks passed
@jerpelea jerpelea added this to To-Add in Release Notes - 12.2.0 Jun 13, 2023
@jerpelea jerpelea moved this from To-Add to In Progress in Release Notes - 12.2.0 Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants