mbox series

[SRU,F/J/OEM-5.17,v2,0/2] CVE-2022-28893

Message ID 20220705045549.274705-1-cengiz.can@canonical.com
Headers show
Series CVE-2022-28893 | expand

Message

Cengiz Can July 5, 2022, 4:55 a.m. UTC
[Impact]
The SUNRPC subsystem in the Linux kernel through 5.17.2 can call
xs_xprt_free before ensuring that sockets are in the intended state.

Issue was introduced with 5.1 and fixed with 5.18.

[Fix]
Fixing commit exports `__fput_sync` symbol for non-GPL and GPL users
with `EXPORT_SYMBOL(..)`. However we already have exported the same
symbol with `EXPORT_SYMBOL_GPL(..)` with a SAUCE patch. After
discussion, we decided to keep that export as GPL-only and ignore the
wider exports of fixing commit.

Second patch supposedly fixes a new issue which was introduced with 
the fix.

[Test]
Compile and boot tested on focal, jammy and jammy with oem-5.17.

[Potential Regression]
It's hard to guess since the exact flow is not shared by author. However
unlikely to cause major issues since sunrpc is only used by NFS, KNFSD
et al.

Trond Myklebust (2):
  SUNRPC: Ensure we flush any closed sockets before xs_xprt_free()
  SUNRPC: Don't leak sockets in xs_local_connect()

 net/sunrpc/xprt.c     |  5 +----
 net/sunrpc/xprtsock.c | 27 +++++++++++++++++++++++----
 2 files changed, 24 insertions(+), 8 deletions(-)

Comments

Tim Gardner July 5, 2022, 2:26 p.m. UTC | #1
On 7/4/22 22:55, Cengiz Can wrote:
> [Impact]
> The SUNRPC subsystem in the Linux kernel through 5.17.2 can call
> xs_xprt_free before ensuring that sockets are in the intended state.
> 
> Issue was introduced with 5.1 and fixed with 5.18.
> 
> [Fix]
> Fixing commit exports `__fput_sync` symbol for non-GPL and GPL users
> with `EXPORT_SYMBOL(..)`. However we already have exported the same
> symbol with `EXPORT_SYMBOL_GPL(..)` with a SAUCE patch. After
> discussion, we decided to keep that export as GPL-only and ignore the
> wider exports of fixing commit.
> 
> Second patch supposedly fixes a new issue which was introduced with
> the fix.
> 
> [Test]
> Compile and boot tested on focal, jammy and jammy with oem-5.17.
> 
> [Potential Regression]
> It's hard to guess since the exact flow is not shared by author. However
> unlikely to cause major issues since sunrpc is only used by NFS, KNFSD
> et al.
> 
> Trond Myklebust (2):
>    SUNRPC: Ensure we flush any closed sockets before xs_xprt_free()
>    SUNRPC: Don't leak sockets in xs_local_connect()
> 
>   net/sunrpc/xprt.c     |  5 +----
>   net/sunrpc/xprtsock.c | 27 +++++++++++++++++++++++----
>   2 files changed, 24 insertions(+), 8 deletions(-)
> 
Acked-by: Tim Gardner <tim.gardner@canonical.com>
Timo Aaltonen July 7, 2022, 7:56 a.m. UTC | #2
Cengiz Can kirjoitti 5.7.2022 klo 7.55:
> [Impact]
> The SUNRPC subsystem in the Linux kernel through 5.17.2 can call
> xs_xprt_free before ensuring that sockets are in the intended state.
> 
> Issue was introduced with 5.1 and fixed with 5.18.
> 
> [Fix]
> Fixing commit exports `__fput_sync` symbol for non-GPL and GPL users
> with `EXPORT_SYMBOL(..)`. However we already have exported the same
> symbol with `EXPORT_SYMBOL_GPL(..)` with a SAUCE patch. After
> discussion, we decided to keep that export as GPL-only and ignore the
> wider exports of fixing commit.
> 
> Second patch supposedly fixes a new issue which was introduced with
> the fix.
> 
> [Test]
> Compile and boot tested on focal, jammy and jammy with oem-5.17.
> 
> [Potential Regression]
> It's hard to guess since the exact flow is not shared by author. However
> unlikely to cause major issues since sunrpc is only used by NFS, KNFSD
> et al.
> 
> Trond Myklebust (2):
>    SUNRPC: Ensure we flush any closed sockets before xs_xprt_free()
>    SUNRPC: Don't leak sockets in xs_local_connect()
> 
>   net/sunrpc/xprt.c     |  5 +----
>   net/sunrpc/xprtsock.c | 27 +++++++++++++++++++++++----
>   2 files changed, 24 insertions(+), 8 deletions(-)
> 

oem-5.17 has had these since -1007.7
Stefan Bader July 8, 2022, 9 a.m. UTC | #3
On 05.07.22 06:55, Cengiz Can wrote:
> [Impact]
> The SUNRPC subsystem in the Linux kernel through 5.17.2 can call
> xs_xprt_free before ensuring that sockets are in the intended state.
> 
> Issue was introduced with 5.1 and fixed with 5.18.
> 
> [Fix]
> Fixing commit exports `__fput_sync` symbol for non-GPL and GPL users
> with `EXPORT_SYMBOL(..)`. However we already have exported the same
> symbol with `EXPORT_SYMBOL_GPL(..)` with a SAUCE patch. After
> discussion, we decided to keep that export as GPL-only and ignore the
> wider exports of fixing commit.
> 
> Second patch supposedly fixes a new issue which was introduced with
> the fix.
> 
> [Test]
> Compile and boot tested on focal, jammy and jammy with oem-5.17.
> 
> [Potential Regression]
> It's hard to guess since the exact flow is not shared by author. However
> unlikely to cause major issues since sunrpc is only used by NFS, KNFSD
> et al.
> 
> Trond Myklebust (2):
>    SUNRPC: Ensure we flush any closed sockets before xs_xprt_free()
>    SUNRPC: Don't leak sockets in xs_local_connect()
> 
>   net/sunrpc/xprt.c     |  5 +----
>   net/sunrpc/xprtsock.c | 27 +++++++++++++++++++++++----
>   2 files changed, 24 insertions(+), 8 deletions(-)
> 

I would tend to say if upstream exports something as EXPORT_SYMBOL and we did 
for some reason an EXPORT_SYMBOL_GPL, then we could follow that export change as 
GPL is more restrictive than the normal export. But at least this cannot make 
things worse as it is.

I would probably also mildly disagree about the regression potential. NFS, as 
bad as it can be, probably is used a lot and if things go wrong there, you will 
have a lot of people with pitch forks and torches outside...

Anyhow:

Acked-by: Stefan Bader <stefan.bader@canonical.com>
Stefan Bader July 8, 2022, 12:48 p.m. UTC | #4
On 05.07.22 06:55, Cengiz Can wrote:
> [Impact]
> The SUNRPC subsystem in the Linux kernel through 5.17.2 can call
> xs_xprt_free before ensuring that sockets are in the intended state.
> 
> Issue was introduced with 5.1 and fixed with 5.18.
> 
> [Fix]
> Fixing commit exports `__fput_sync` symbol for non-GPL and GPL users
> with `EXPORT_SYMBOL(..)`. However we already have exported the same
> symbol with `EXPORT_SYMBOL_GPL(..)` with a SAUCE patch. After
> discussion, we decided to keep that export as GPL-only and ignore the
> wider exports of fixing commit.
> 
> Second patch supposedly fixes a new issue which was introduced with
> the fix.
> 
> [Test]
> Compile and boot tested on focal, jammy and jammy with oem-5.17.
> 
> [Potential Regression]
> It's hard to guess since the exact flow is not shared by author. However
> unlikely to cause major issues since sunrpc is only used by NFS, KNFSD
> et al.
> 
> Trond Myklebust (2):
>    SUNRPC: Ensure we flush any closed sockets before xs_xprt_free()
>    SUNRPC: Don't leak sockets in xs_local_connect()
> 
>   net/sunrpc/xprt.c     |  5 +----
>   net/sunrpc/xprtsock.c | 27 +++++++++++++++++++++++----
>   2 files changed, 24 insertions(+), 8 deletions(-)
> 

Applied to jammy,focal:linux/master-next. Thanks.

-Stefan