ACK/Cmnt: [SRU][v3][F][J][L][M][PATCH 0/1] net: Avoid address overwrite in kernel_connect

[Andrea Righi]

On Wed, Sep 13, 2023 at 09:59:43AM +0200, Stefan Bader wrote:
> On 12.09.23 21:49, Khalid Elmously wrote:
> > BugLink: https://bugs.launchpad.net/bugs/2035163
> > 
> > An upstream fix is requested to resolve a use-case where sockaddr gets re-written which causes NFS-breakage.
> > 
> > 
> > Testing:
> >   - GCP has confirmed that the fix works before proposing it upstream. I have verified basic network sanity with the fix applied. I will also ask them to confirm the fix with the kernel in -proposed.
> > 
> > 
> > Regression potenial:
> >   - The fix modifies kernel_connect() which can have an effect on all kinds of network connections. The change itself is very minor and only converts a pass-by-reference to a pass-by-value - so the risk is considered minimal.
> > 
> > 
> > 
> > v2:
> >   - Added nomination for J/gke as well
> > 
> > v3:
> >   - nominating for all generic kernels
> > 
> > 
> > Jordan Rife (1):
> >    net: Avoid address overwrite in kernel_connect
> > 
> >   net/socket.c | 7 ++++++-
> >   1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> The READ_ONCE() which was worked around was added in v6.6 when I checked
> correctly. That was done to avoid problematic compiler optimizations.
> Mentioning this to set awareness that this might lead to different results
> than upstream and to watch out for possible socket issues in the cycle this
> gets applied to.
> Also, would this not be relevant for the various OEM kernels as well?
> 
> Acked-by: Stefan Bader <stefan.bader at canonical.com>

The READ_ONCE() is provided by:

 1ded5e5a5931 ("net: annotate data-races around sock->ops")

Maybe we should apply also this one? Otherwise we have a race with ipv6,
because IPV6_ADDRFORM seems to change sock->ops without any protection.

I haven't checked in F/J/L, but in M it's definitely safer to have the
READ_ONCE() (the whole 1ded5e5a5931 actually).

-Andrea