bpf: Fix FIONREAD and copied_seq issues #6416
Open
+331
−17
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Author
|
Upstream branch: 590699d |
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
bpf-next_base
branch
from
9b3817c to
90dfd48
Compare
Author
|
Upstream branch: f2cb066 |
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
series/1027335=>bpf-next
branch
from
73f6092 to
a84d606
Compare
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
bpf-next_base
branch
from
90dfd48 to
1559a3a
Compare
Author
|
Upstream branch: 8c868a3 |
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
series/1027335=>bpf-next
branch
from
a84d606 to
7bf9758
Compare
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
bpf-next_base
branch
from
1559a3a to
ae9b520
Compare
Author
|
Upstream branch: 8f6ddc0 |
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
series/1027335=>bpf-next
branch
from
7bf9758 to
cb32b71
Compare
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
bpf-next_base
branch
from
ae9b520 to
399fdcb
Compare
Author
|
Upstream branch: 5262cb2 |
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
series/1027335=>bpf-next
branch
from
cb32b71 to
7e906ee
Compare
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
bpf-next_base
branch
from
399fdcb to
8c83cb5
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]>
Author
|
Upstream branch: 688b745 |
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]>
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
series/1027335=>bpf-next
branch
from
7e906ee to
df10136
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull request for series with
subject: bpf: Fix FIONREAD and copied_seq issues
version: 4
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1027335