Re: [PATCH] fs/coda: ensure the header peeked at is the same in the actual message
From: Meng Xu
Date: Sat Sep 23 2017 - 22:39:54 EST
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.
> 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;