On 4/13/21 5:41 AM, Eric Dumazet wrote:
> From: Eric Dumazet <eduma...@google.com>
>
> After commit 0f6925b3e8da ("virtio_net: Do not pull payload in skb->head")
> Guenter Roeck reported one failure in his tests using sh architecture.
>
> After much debugging, we have been able to spot silent unaligned accesses
> in inet_gro_receive()
>
> The issue at hand is that upper networking stacks assume their header
> is word-aligned. Low level drivers are supposed to reserve NET_IP_ALIGN
> bytes before the Ethernet header to make that happen.
>
> This patch hardens skb_gro_reset_offset() to not allow frag0 fast-path
> if the fragment is not properly aligned.
>
> Some arches like x86, arm64 and powerpc do not care and define NET_IP_ALIGN
> as 0, this extra check will be a NOP for them.
>
> Note that if frag0 is not used, GRO will call pskb_may_pull()
> as many times as needed to pull network and transport headers.
>
> Fixes: 0f6925b3e8da ("virtio_net: Do not pull payload in skb->head")
> Fixes: 78a478d0efd9 ("gro: Inline skb_gro_header and cache frag0 virtual
> address")
> Signed-off-by: Eric Dumazet <eduma...@google.com>
> Reported-by: Guenter Roeck <li...@roeck-us.net>
> Cc: Xuan Zhuo <xuanz...@linux.alibaba.com>
> Cc: "Michael S. Tsirkin" <m...@redhat.com>
> Cc: Jason Wang <jasow...@redhat.com>
Nice catch.
Tested-by: Guenter Roeck <li...@roeck-us.net>
.... and thanks a lot for tracking this down!
Guenter
> ---
> net/core/dev.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index
> af8c1ea040b9364b076e2d72f04dc3de2d7e2f11..1f79b9aa9a3f2392fddd1401f95ad098b5e03204
> 100644
> --- 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),
>
