-
Notifications
You must be signed in to change notification settings - Fork 5
bpf: Fix FIONREAD and copied_seq issues #6408
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
bpf: Fix FIONREAD and copied_seq issues #6408
Conversation
|
Upstream branch: e0940c6 |
|
Upstream branch: e0940c6 |
aa402d4 to
6c6447a
Compare
623bab9 to
fe03c14
Compare
|
Upstream branch: 792f258 |
6c6447a to
3bfba62
Compare
fe03c14 to
65bfb85
Compare
|
Upstream branch: 878ee3c |
3bfba62 to
961dd85
Compare
65bfb85 to
b1f8b58
Compare
|
Upstream branch: ae24fc8 |
961dd85 to
d2dd506
Compare
b1f8b58 to
8f7081b
Compare
|
Upstream branch: b7f7d76 |
d2dd506 to
e84f273
Compare
8f7081b to
c347688
Compare
|
Upstream branch: 4dd3a48 |
e84f273 to
530d2cf
Compare
c347688 to
fb42a92
Compare
|
Upstream branch: 8f7cf30 |
530d2cf to
607479a
Compare
fb42a92 to
3282beb
Compare
|
Upstream branch: c427320 |
607479a to
8fc2902
Compare
3282beb to
067f842
Compare
|
Upstream branch: fad8040 |
8fc2902 to
4ce8776
Compare
067f842 to
3bd2c43
Compare
|
Upstream branch: acf8726 |
4ce8776 to
05767e2
Compare
3bd2c43 to
71c4be1
Compare
|
Upstream branch: 4617b30 |
05767e2 to
8093220
Compare
71c4be1 to
9b3817c
Compare
A socket using sockmap has its own independent receive queue: ingress_msg.
This queue may contain data from its own protocol stack or from other
sockets.
The issue is that when reading from ingress_msg, we update tp->copied_seq
by default. However, if the data is not from its own protocol stack,
tcp->rcv_nxt is not increased. Later, if we convert this socket to a
native socket, reading from this socket may fail because copied_seq might
be significantly larger than rcv_nxt.
This fix also addresses the syzkaller-reported bug referenced in the
Closes tag.
This patch marks the skmsg objects in ingress_msg. When reading, we update
copied_seq only if the data is from its own protocol stack.
FD1:read()
-- FD1->copied_seq++
| [read data]
|
[enqueue data] v
[sockmap] -> ingress to self -> ingress_msg queue
FD1 native stack ------> ^
-- FD1->rcv_nxt++ -> redirect to other | [enqueue data]
| |
| ingress to FD1
v ^
... | [sockmap]
FD2 native stack
Closes: https://syzkaller.appspot.com/bug?extid=06dbd397158ec0ea4983
Fixes: 04919be ("tcp: Introduce tcp_read_skb()")
Signed-off-by: Jiayuan Chen <[email protected]>
A socket using sockmap has its own independent receive queue: ingress_msg. This queue may contain data from its own protocol stack or from other sockets. Therefore, for sockmap, relying solely on copied_seq and rcv_nxt to calculate FIONREAD is not enough. This patch adds a new ingress_size field in the psock structure to record the data length in ingress_msg. Additionally, we implement new ioctl interfaces for TCP and UDP to intercept FIONREAD operations. While Unix and VSOCK also support sockmap and have similar FIONREAD calculation issues, fixing them would require more extensive changes (please let me know if modifications are needed). I believe it's not appropriate to include those changes under this fix patch. Previous work by John Fastabend made some efforts towards FIONREAD support: commit e5c6de5 ("bpf, sockmap: Incorrectly handling copied_seq") Although the current patch is based on the previous work by John Fastabend, it is acceptable for our Fixes tag to point to the same commit. FD1:read() -- FD1->copied_seq++ | [read data] | [enqueue data] v [sockmap] -> ingress to self -> ingress_msg queue FD1 native stack ------> ^ -- FD1->rcv_nxt++ -> redirect to other | [enqueue data] | | | ingress to FD1 v ^ ... | [sockmap] FD2 native stack Fixes: 04919be ("tcp: Introduce tcp_read_skb()") Signed-off-by: Jiayuan Chen <[email protected]>
This commit adds two new test functions: one to reproduce the bug reported by syzkaller [1], and another to cover the calculation of copied_seq. The tests primarily involve installing and uninstalling sockmap on sockets, then reading data to verify proper functionality. Additionally, extend the do_test_sockmap_skb_verdict_fionread() function to support UDP FIONREAD testing. [1] https://syzkaller.appspot.com/bug?extid=06dbd397158ec0ea4983 Signed-off-by: Jiayuan Chen <[email protected]>
|
Upstream branch: 590699d |
8093220 to
9f9a60d
Compare
Pull request for series with
subject: bpf: Fix FIONREAD and copied_seq issues
version: 3
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1026368