netdev - Re: [PATCH net] sunrpc: fix one UAF issue caused by sunrpc kernel tcp socket

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]
Message-ID: <20241025212038.31584-1-kuniyu@amazon.com>
Date: Fri, 25 Oct 2024 14:20:38 -0700
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <liujian56@...wei.com>
CC: <Dai.Ngo@...cle.com>, <anna@...nel.org>, <chuck.lever@...cle.com>,
	<davem@...emloft.net>, <ebiederm@...ssion.com>, <edumazet@...gle.com>,
	<jlayton@...nel.org>, <kuba@...nel.org>, <linux-nfs@...r.kernel.org>,
	<neilb@...e.de>, <netdev@...r.kernel.org>, <okorniev@...hat.com>,
	<pabeni@...hat.com>, <tom@...pey.com>, <trondmy@...merspace.com>,
	<kuniyu@...zon.com>
Subject: Re: [PATCH net] sunrpc: fix one UAF issue caused by sunrpc kernel tcp socket

From: "liujian (CE)" <liujian56@...wei.com>
Date: Fri, 25 Oct 2024 11:32:52 +0800
> >>> If not, then what prevents it from happening?
> >> The socket created by the userspace program obtains the reference
> >> counting of the namespace, but the kernel socket does not.
> >>
> >> There's some discussion here:
> >> https://lore.kernel.org/all/CANn89iJE5anTbyLJ0TdGAqGsE+GichY3YzQECjNUVMz=G3bcQg@mail.gmail.com/
> > OK... So then it looks to me as if NFS, SMB, AFS, and any other
> > networked filesystem that can be started from inside a container is
> > going to need to do the same thing that rds appears to be doing.

FWIW, recently we saw a similar UAF on CIFS.


> >
> > Should there perhaps be a helper function in the networking layer for
> > this?
> 
> There should be no such helper function at present, right?.
> 
> If get net's reference to fix this problem, the following test is 
> performed. There's nothing wrong with this case. I don't know if there's 
> anything else to consider.
> 
> I don't have any other ideas other than these two methods. Do you have 
> any suggestions on this problem? @Eric @Jakub ... @All

The netns lifetime should be managed by the upper layer rather than
the networking layer.  If the netns is already dead, the upper layer
must discard the net pointer anyway.

I suggest checking maybe_get_net() in NFS, CIFS, etc and then calling
__sock_create() with kern 0.


> 
> diff --git a/include/linux/net.h b/include/linux/net.h
> index b75bc534c1b3..58216da3b62c 100644
> --- a/include/linux/net.h
> +++ b/include/linux/net.h
> @@ -255,6 +255,7 @@ int __sock_create(struct net *net, int family, int 
> type, int proto,
>                    struct socket **res, int kern);
>   int sock_create(int family, int type, int proto, struct socket **res);
>   int sock_create_kern(struct net *net, int family, int type, int proto, 
> struct socket **res);
> +int sock_create_kern_getnet(struct net *net, int family, int type, int 
> proto, struct socket **res);
>   int sock_create_lite(int family, int type, int proto, struct socket 
> **res);
>   struct socket *sock_alloc(void);
>   void sock_release(struct socket *sock);
> diff --git a/net/socket.c b/net/socket.c
> index 042451f01c65..e64a02445b1a 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -1651,6 +1651,34 @@ int sock_create_kern(struct net *net, int family, 
> int type, int protocol, struct
>   }
>   EXPORT_SYMBOL(sock_create_kern);
> 
> +int sock_create_kern_getnet(struct net *net, int family, int type, int 
> proto, struct socket **res)
> +{
> +       struct sock *sk;
> +       int ret;
> +
> +       if (!maybe_get_net(net))
> +               return -EINVAL;
> +
> +       ret = sock_create_kern(net, family, type, proto, res);
> +       if (ret < 0) {
> +               put_net(net);
> +               return ret;
> +       }
> +
> +       sk = (*res)->sk;
> +       lock_sock(sk);
> +       /* Update ns_tracker to current stack trace and refcounted 
> tracker */
> +       __netns_tracker_free(net, &sk->ns_tracker, false);
> +
> +       sk->sk_net_refcnt = 1;
> +       netns_tracker_alloc(net, &sk->ns_tracker, GFP_KERNEL);
> +       sock_inuse_add(net, 1);
> +       release_sock(sk);
> +
> +       return ret;
> +}
> +EXPORT_SYMBOL(sock_create_kern_getnet);
> +
>   static struct socket *__sys_socket_create(int family, int type, int 
> protocol)
>   {
>          struct socket *sock;
> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> index 825ec5357691..31dc291446fb 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -1526,7 +1526,10 @@ static struct svc_xprt *svc_create_socket(struct 
> svc_serv *serv,
>                  return ERR_PTR(-EINVAL);
>          }
> 
> -       error = __sock_create(net, family, type, protocol, &sock, 1);
> +       if (protocol == IPPROTO_TCP)
> +               error = sock_create_kern_getnet(net, family, type, 
> protocol, &sock);
> +       else
> +               error = sock_create_kern(net, family, type, protocol, 
> &sock);
>          if (error < 0)
>                  return ERR_PTR(error);
> 
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 0e1691316f42..d2304010daeb 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -1922,7 +1922,10 @@ static struct socket *xs_create_sock(struct 
> rpc_xprt *xprt,
>          struct socket *sock;
>          int err;
> 
> -       err = __sock_create(xprt->xprt_net, family, type, protocol, 
> &sock, 1);
> +       if (protocol == IPPROTO_TCP)
> +               err = sock_create_kern_getnet(xprt->xprt_net, family, 
> type, protocol, &sock);
> +       else
> +               err = sock_create_kern(xprt->xprt_net, family, type, 
> protocol, &sock);
>          if (err < 0) {
>                  dprintk("RPC:       can't create %d transport socket 
> (%d).\n",
>                                  protocol, -err);