Skip to content

Commit

Permalink
nfsd: fix double fget() bug in __write_ports_addfd()
Browse files Browse the repository at this point in the history
[ Upstream commit c034203 ]

The bug here is that you cannot rely on getting the same socket
from multiple calls to fget() because userspace can influence
that.  This is a kind of double fetch bug.

The fix is to delete the svc_alien_sock() function and instead do
the checking inside the svc_addsock() function.

Fixes: 3064639 ("nfsd: check passed socket's net matches NFSd superblock's one")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
Reviewed-by: NeilBrown <neilb@suse.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
Dan Carpenter authored and gregkh committed Jun 9, 2023
1 parent 401a1cf commit 4e5a5cd
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 28 deletions.
7 changes: 1 addition & 6 deletions fs/nfsd/nfsctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -710,16 +710,11 @@ static ssize_t __write_ports_addfd(char *buf, struct net *net, const struct cred
if (err != 0 || fd < 0)
return -EINVAL;

if (svc_alien_sock(net, fd)) {
printk(KERN_ERR "%s: socket net is different to NFSd's one\n", __func__);
return -EINVAL;
}

err = nfsd_create_serv(net);
if (err != 0)
return err;

err = svc_addsock(nn->nfsd_serv, fd, buf, SIMPLE_TRANSACTION_LIMIT, cred);
err = svc_addsock(nn->nfsd_serv, net, fd, buf, SIMPLE_TRANSACTION_LIMIT, cred);

if (err >= 0 &&
!nn->nfsd_serv->sv_nrthreads && !xchg(&nn->keep_active, 1))
Expand Down
7 changes: 3 additions & 4 deletions include/linux/sunrpc/svcsock.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,9 @@ int svc_recv(struct svc_rqst *, long);
int svc_send(struct svc_rqst *);
void svc_drop(struct svc_rqst *);
void svc_sock_update_bufs(struct svc_serv *serv);
bool svc_alien_sock(struct net *net, int fd);
int svc_addsock(struct svc_serv *serv, const int fd,
char *name_return, const size_t len,
const struct cred *cred);
int svc_addsock(struct svc_serv *serv, struct net *net,
const int fd, char *name_return, const size_t len,
const struct cred *cred);
void svc_init_xprt_sock(void);
void svc_cleanup_xprt_sock(void);
struct svc_xprt *svc_sock_create(struct svc_serv *serv, int prot);
Expand Down
24 changes: 6 additions & 18 deletions net/sunrpc/svcsock.c
Original file line number Diff line number Diff line change
Expand Up @@ -1338,25 +1338,10 @@ static struct svc_sock *svc_setup_socket(struct svc_serv *serv,
return svsk;
}

bool svc_alien_sock(struct net *net, int fd)
{
int err;
struct socket *sock = sockfd_lookup(fd, &err);
bool ret = false;

if (!sock)
goto out;
if (sock_net(sock->sk) != net)
ret = true;
sockfd_put(sock);
out:
return ret;
}
EXPORT_SYMBOL_GPL(svc_alien_sock);

/**
* svc_addsock - add a listener socket to an RPC service
* @serv: pointer to RPC service to which to add a new listener
* @net: caller's network namespace
* @fd: file descriptor of the new listener
* @name_return: pointer to buffer to fill in with name of listener
* @len: size of the buffer
Expand All @@ -1366,8 +1351,8 @@ EXPORT_SYMBOL_GPL(svc_alien_sock);
* Name is terminated with '\n'. On error, returns a negative errno
* value.
*/
int svc_addsock(struct svc_serv *serv, const int fd, char *name_return,
const size_t len, const struct cred *cred)
int svc_addsock(struct svc_serv *serv, struct net *net, const int fd,
char *name_return, const size_t len, const struct cred *cred)
{
int err = 0;
struct socket *so = sockfd_lookup(fd, &err);
Expand All @@ -1378,6 +1363,9 @@ int svc_addsock(struct svc_serv *serv, const int fd, char *name_return,

if (!so)
return err;
err = -EINVAL;
if (sock_net(so->sk) != net)
goto out;
err = -EAFNOSUPPORT;
if ((so->sk->sk_family != PF_INET) && (so->sk->sk_family != PF_INET6))
goto out;
Expand Down

0 comments on commit 4e5a5cd

Please sign in to comment.