On 2024/1/25 16:57, Eric Dumazet wrote:
On Thu, Jan 25, 2024 at 3:18 AM zhangpeng (AS) <zhangpeng362@xxxxxxxxxx> wrote:On 2024/1/24 18:11, Eric Dumazet wrote:On Wed, Jan 24, 2024 at 10:30 AM zhangpeng (AS) <zhangpeng362@xxxxxxxxxx> wrote:By using git-bisect, the patch that introduces this issue is 05255b823a617 ("tcp: add TCP_ZEROCOPY_RECEIVE support for zerocopy receive."). v4.18-rc1. Currently, there are no other repro or c reproduction programs can reproduce the issue. The syz log used to reproduce the issue is as follows: r3 = socket$inet_tcp(0x2, 0x1, 0x0) mmap(&(0x7f0000ff9000/0x4000)=nil, 0x4000, 0x0, 0x12, r3, 0x0) r4 = socket$inet_tcp(0x2, 0x1, 0x0) bind$inet(r4, &(0x7f0000000000)={0x2, 0x4e24, @multicast1}, 0x10) connect$inet(r4, &(0x7f00000006c0)={0x2, 0x4e24, @empty}, 0x10) r5 = openat$dir(0xffffffffffffff9c, &(0x7f00000000c0)='./file0\x00', 0x181e42, 0x0) fallocate(r5, 0x0, 0x0, 0x85b8818) sendfile(r4, r5, 0x0, 0x3000) getsockopt$inet_tcp_TCP_ZEROCOPY_RECEIVE(r4, 0x6, 0x23, &(0x7f00000001c0)={&(0x7f0000ffb000/0x3000)=nil, 0x3000, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, &(0x7f0000000440)=0x10) r6 = openat$dir(0xffffffffffffff9c, &(0x7f00000000c0)='./file0\x00', 0x181e42, 0x0)Could you try the following fix then ? (We also could remove the !skb_frag_off(frag) condition, as the !PageCompound() is necessary it seems :/) Thanks a lot ! diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c index 1baa484d21902d2492fc2830d960100dc09683bf..ee954ae7778a651a9da4de057e3bafe35a6e10d6 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -1785,7 +1785,9 @@ static skb_frag_t *skb_advance_to_frag(struct sk_buff *skb, u32 offset_skb, static bool can_map_frag(const skb_frag_t *frag) { - return skb_frag_size(frag) == PAGE_SIZE && !skb_frag_off(frag); + return skb_frag_size(frag) == PAGE_SIZE && + !skb_frag_off(frag) && + !PageCompound(skb_frag_page(frag)); } static int find_next_mappable_frag(const skb_frag_t *frag,This patch doesn't fix this issue. The page cache that can trigger this issue doesn't necessarily need to be compound. 🙁Ah, too bad :/ So the issue is that the page had a mapping. I am no mm expert, I am not sure if we need to add more tests (like testing various illegal page flags) ? Can you test this ? (I am still converting the repro into C) Thanks. diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c index 1baa484d21902d2492fc2830d960100dc09683bf..2128015227a5066ea74b3911ecaefe7992da132f 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -1785,7 +1785,17 @@ static skb_frag_t *skb_advance_to_frag(struct sk_buff *skb, u32 offset_skb, static bool can_map_frag(const skb_frag_t *frag) { - return skb_frag_size(frag) == PAGE_SIZE && !skb_frag_off(frag); + struct page *page; + + if (skb_frag_size(frag) != PAGE_SIZE || skb_frag_off(frag)) + return false; + + page = skb_frag_page(frag); + + if (PageCompound(page) || page->mapping) + return false; + + return true; } static int find_next_mappable_frag(const skb_frag_t *frag,
This patch can fix this issue. In this scenario, page->mapping is inode->i_mapping of ext4, but VMA is tcp VMA. It's weird. If all the pages that need to be inserted by TCP zerocopy are page->mapping == NULL, this solution could be used. Thanks! -- Best Regards, Peng
