Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1740238

Re: [PATCH] nfsd4: ensure cm_xid does not change across userspace fetches

From "J. Bruce Fields" <bfields@fieldses.org>
Newsgroups linux.kernel
Subject Re: [PATCH] nfsd4: ensure cm_xid does not change across userspace fetches
Date 2017-09-27 00:00 +0200
Message-ID <uu63w-1PM-11@gated-at.bofh.it> (permalink)
References <utizM-2Y4-17@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Sun, Sep 24, 2017 at 12:59:40PM -0400, Meng Xu wrote:
> cld_pipe_downcall() has two fetches from an overapped userspace memory.
> The first fetch copy_from_user(&xid, &cmsg->cm_xid, sizeof(xid)) get
> the xid and use xid to lookup the parent struct cld_upcall *cup.
> The second fetch copy_from_user(&cup->cu_msg, src, mlen) place the whole
> message into &cup->cu_msg.
> 
> Since the userspace thread has full control over this &cmsg->cm_xid,
> it can race condition to change the cm_xid value across the two fetches,
> (say, change from 1 to 2), therefore, de-listing the cup with
> cu_msg.cm_xid == 1 but later put cu_msg.cm_xid = 2.
> 
> Whether this double-fetch situation is a security critical bug depends
> on how cup->cu_msg is used later. However, given that it is hard to
> enumerate all the possible use cases, a safer way might be to ensure
> that the xid does not change across the fetches, which is what this
> patch is for.

Alternatively, couldn't we copy the whole message just once at the
start?  It doesn't look like there very large, so there's room for it on
the stack, if that's the problem.

--b.

> 
> Signed-off-by: Meng Xu <mengxu.gatech@gmail.com>
> ---
>  fs/nfsd/nfs4recover.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> index 66eaeb1..1abe9ed 100644
> --- a/fs/nfsd/nfs4recover.c
> +++ b/fs/nfsd/nfs4recover.c
> @@ -753,6 +753,11 @@ cld_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
>  	if (copy_from_user(&cup->cu_msg, src, mlen) != 0)
>  		return -EFAULT;
>  
> +	/* ensure that the xid has not been changed */
> +	if (cup->cu_msg.cm_xid != xid) {
> +		return -EFAULT;
> +	}
> +
>  	wake_up_process(cup->cu_task);
>  	return mlen;
>  }
> -- 
> 2.7.4

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] nfsd4: ensure cm_xid does not change across userspace fetches Meng Xu <mengxu.gatech@gmail.com> - 2017-09-24 19:10 +0200
  Re: [PATCH] nfsd4: ensure cm_xid does not change across userspace  fetches "J. Bruce Fields" <bfields@fieldses.org> - 2017-09-27 00:00 +0200
    Re: [PATCH] nfsd4: ensure cm_xid does not change across userspace  fetches "J. Bruce Fields" <bfields@fieldses.org> - 2017-09-27 23:10 +0200

csiph-web