netdev - Re: [PATCH] net/xen-netback: Break build if netback slots > max

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 5 Oct 2023 09:03:28 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: David Kahurani <k.kahurani@...il.com>
Cc: Juergen Gross <jgross@...e.com>, xen-devel@...ts.xenproject.org,
 netdev@...r.kernel.org, wei.liu@...nel.org, paul@....org
Subject: Re: [PATCH] net/xen-netback: Break build if netback slots >
 max_skbs + 1

On Thu, 5 Oct 2023 18:39:51 +0300 David Kahurani wrote:
> > MAX_SKB_FRAGS can now be set via Kconfig, this allows us to create
> > larger super-packets. Can XEN_NETBK_LEGACY_SLOTS_MAX be made relative
> > to MAX_SKB_FRAGS, or does the number have to match between guest and
> > host?  
> 
> Historically, netback driver allows for a maximum of 18 fragments.
> With recent changes, it also relies on the assumption that the
> difference between MAX_SKB_FRAGS and XEN_NETBK_LEGACY_SLOTS_MAX is one
> and MAX_SKB_FRAGS is the lesser value.
> 
> Now, look at Ubuntu kernel for instance( a change has been made and,
> presumably, with good reason so we have reason to assume that the
> change will persist in future releases).
> 
> /* To allow 64K frame to be packed as single skb without frag_list we
>  * require 64K/PAGE_SIZE pages plus 1 additional page to allow for
>  * buffers which do not start on a page boundary.
>  *
>  * Since GRO uses frags we allocate at least 16 regardless of page
>  * size.
>  */
> #if (65536/PAGE_SIZE + 1) < 16
> #define MAX_SKB_FRAGS 16UL
> #else
> #define MAX_SKB_FRAGS (65536/PAGE_SIZE + 1)
> #endif
> 
> So, MAX_SKB_FRAGS can sometimes be 16. This is exactly what we're
> trying to avoid with this patch. I host running with this change is
> vulnerable to attack by the guest(though, this will only happen when
> PAGE_SIZE > 4096).

My bad, you're protecting from the inverse condition than I thought.

But to be clear the code you're quoting (the defines for MAX_SKB_FRAGS)
are what has been there upstream forever until 3948b059 was merged.
Not 100% sure why 3948b059 switched the min from 16 to 17, I think it
was just to keep consistency between builds.

If this change gets backported to 6.1 stable it will break ppc build
of stable, right? Since ppc has 64k pages.