Re: [PATCH] fs/coda: ensure the header peeked at is the same in the actual message

[Meng Xu]

Hi Jaharkes and Coda filesystem developers,

I am resending the email on a potential race condition bug I found in the
Coda filesystem as well as the patch I propose. Please feel free to comment
whether you think this is a serious problem and whether the patch will work.
Thank you.

Best Regards,
Meng

> On Sep 19, 2017, at 11:23 AM, Meng Xu <mengxu.gatech@xxxxxxxxx> wrote:
> 
> In coda_psdev_write(), the header of the buffer is fetched twice from the
> userspace. The first fetch is used to peek at the opcode and unique id while
> the second fetch copies the whole message. However, there could be
> inconsistency in these two fields between two fetches as buf resides in
> userspace memory and a user process can rush to change it across fetches.
> Which means that the corresponding opcode and unique id fields in
> req->uc_data could be different from what is fetched in for the first time.
> 
> Whether this double-fetch situation is a security critical bug depends on
> how req->uc_data will be used later. However, given that it is hard to
> enumerate all the possible use cases, a safer way is to ensure that the
> peeked header is actually the same message header after the second fetch.
> 
> This patch enforces that the header of the message fetched into req->uc_data
> is the same as what is fetched in originally. In other words, hdr.opcode and
> hdr.unique do not change.
> 
> Signed-off-by: Meng Xu <mengxu.gatech@xxxxxxxxx>
> ---
> fs/coda/psdev.c | 6 ++++++
> 1 file changed, 6 insertions(+)
> 
> diff --git a/fs/coda/psdev.c b/fs/coda/psdev.c
> index f40e395..b9dbdd8 100644
> --- a/fs/coda/psdev.c
> +++ b/fs/coda/psdev.c
> @@ -178,6 +178,12 @@ static ssize_t coda_psdev_write(struct file *file, const char __user *buf,
> 		goto out;
> 	}
> 
> +	/*
> +	 * Override the request header to make sure that it matches the
> +	 * first fetch from buf
> +	 */
> +	memcpy(req->uc_data, &hdr, 2 * sizeof(u_long));
> +
> 	/* adjust outsize. is this useful ?? */
> 	req->uc_outSize = nbytes;
> 	req->uc_flags |= CODA_REQ_WRITE;
> -- 
> 2.7.4
>