Re: Linux 5.12-rc7
From: Eric Dumazet <edumazet@google.com>
Date: 2021-04-13 10:43:41
Also in:
lkml
Subsystem:
networking [general], superh, the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz, Linus Torvalds
On Tue, Apr 13, 2021 at 11:24 AM Eric Dumazet [off-list ref] wrote:
quoted hunk ↗ jump to hunk
On Mon, Apr 12, 2021 at 10:05 PM Guenter Roeck [off-list ref] wrote:quoted
On 4/12/21 10:38 AM, Eric Dumazet wrote: [ ... ]quoted
Yes, I think this is the real issue here. This smells like some memory corruption. In my traces, packet is correctly received in AF_PACKET queue. I have checked the skb is well formed. But the user space seems to never call poll() and recvmsg() on this af_packet socket.After sprinkling the kernel with debug messages: 424 00:01:33.674181 sendto(6, "E\0\1H\0\0\0\0@\21y\246\0\0\0\0\377\377\377\377\0D\0C\00148\346\1\1\6\0\246\336\333\v\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0RT\0\ 424 00:01:33.693873 close(6) = 0 424 00:01:33.694652 fcntl64(5, F_SETFD, FD_CLOEXEC) = 0 424 00:01:33.695213 clock_gettime64(CLOCK_MONOTONIC, 0x7be18a18) = -1 EFAULT (Bad address) 424 00:01:33.695889 write(2, "udhcpc: clock_gettime(MONOTONIC) failed\n", 40) = -1 EFAULT (Bad address) 424 00:01:33.697311 exit_group(1) = ? 424 00:01:33.698346 +++ exited with 1 +++ I only see that after adding debug messages in the kernel, so I guess there must be a heisenbug somehere. Anyway, indeed, I see (another kernel debug message): __do_sys_clock_gettime: Returning -EFAULT on address 0x7bacc9a8 So udhcpc doesn't even try to read the reply because it crashes after sendto() when trying to read the current time. Unless I am missing something, that means that the problem happens somewhere on the send side. To make things even more interesting, it looks like the failing system call isn't always clock_gettime(). GuenterI think GRO fast path has never worked on SUPERH. Probably SUPERH has never used a fast NIC (10Gbit+) The following hack fixes the issue.diff --git a/net/core/dev.c b/net/core/dev.c index af8c1ea040b9364b076e2d72f04dc3de2d7e2f11..91ba89a645ff91d4cd4f3d8dc8a009bcb67da344100644--- a/net/core/dev.c +++ b/net/core/dev.c@@ -5916,13 +5916,16 @@ static struct list_head*gro_list_prepare(struct napi_struct *napi, static void skb_gro_reset_offset(struct sk_buff *skb) { +#if !defined(CONFIG_SUPERH) const struct skb_shared_info *pinfo = skb_shinfo(skb); const skb_frag_t *frag0 = &pinfo->frags[0]; +#endif NAPI_GRO_CB(skb)->data_offset = 0; NAPI_GRO_CB(skb)->frag0 = NULL; NAPI_GRO_CB(skb)->frag0_len = 0; +#if !defined(CONFIG_SUPERH) if (!skb_headlen(skb) && pinfo->nr_frags && !PageHighMem(skb_frag_page(frag0))) { NAPI_GRO_CB(skb)->frag0 = skb_frag_address(frag0);@@ -5930,6 +5933,7 @@ static void skb_gro_reset_offset(struct sk_buff *skb) skb_frag_size(frag0), skb->end - skb->tail); } +#endif } static void gro_pull_from_frag0(struct sk_buff *skb, int grow)
OK ... more sh debugging :
diff --git a/arch/sh/mm/alignment.c b/arch/sh/mm/alignment.c
index fb517b82a87b1065cf38c06cb3c178ce86587b00..5d18f9f792991105a8aa05cc6231b7d4532d72c9100644
--- a/arch/sh/mm/alignment.c
+++ b/arch/sh/mm/alignment.c@@ -27,7 +27,7 @@ static unsigned long se_multi; valid! */ static int se_usermode = UM_WARN | UM_FIXUP; /* 0: no warning 1: print a warning message, disabled by default */ -static int se_kernmode_warn; +static int se_kernmode_warn = 1; core_param(alignment, se_usermode, int, 0600);
@@ -103,7 +103,7 @@ void unaligned_fixups_notify(struct task_struct*tsk, insn_size_t insn,
(void *)instruction_pointer(regs), insn);
else if (se_kernmode_warn)
pr_notice_ratelimited("Fixing up unaligned kernel access "
- "in \"%s\" pid=%d pc=0x%p ins=0x%04hx\n",
+ "in \"%s\" pid=%d pc=%px ins=0x%04hx\n",
tsk->comm, task_pid_nr(tsk),
(void *)instruction_pointer(regs), insn);
}
I now see something of interest :
Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc2e ins=0x6236
Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc2e ins=0x6236
Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc30 ins=0x6636
Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc30 ins=0x6636
Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc3a ins=0x6636
Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc3a ins=0x6636
Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc3a ins=0x6636
Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc3a ins=0x6636
Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc3a ins=0x6636
Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc3a ins=0x6636
So basically the frag0 idea only works if drivers respect NET_IP_ALIGN
(So that IP header is 4-byte aligned)
It seems either virtio_net or qemu does not respect the contract.
A possible generic fix would then be :
diff --git a/net/core/dev.c b/net/core/dev.c
index af8c1ea040b9364b076e2d72f04dc3de2d7e2f11..1f79b9aa9a3f2392fddd1401f95ad098b5e03204100644
--- a/net/core/dev.c
+++ b/net/core/dev.c@@ -5924,7 +5924,8 @@ static void skb_gro_reset_offset(struct sk_buff *skb) NAPI_GRO_CB(skb)->frag0_len = 0; if (!skb_headlen(skb) && pinfo->nr_frags && - !PageHighMem(skb_frag_page(frag0))) { + !PageHighMem(skb_frag_page(frag0)) && + (!NET_IP_ALIGN || !(skb_frag_off(frag0) & 3))) { NAPI_GRO_CB(skb)->frag0 = skb_frag_address(frag0); NAPI_GRO_CB(skb)->frag0_len = min_t(unsigned int, skb_frag_size(frag0),