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

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: <2A262D40-191F-4C3E-9BF8-CCF2289BE547@gmail.com>
Date:   Sat, 23 Sep 2017 22:35:45 -0400
From:   Meng Xu <mengxu.gatech@...il.com>
To:     jaharkes@...cmu.edu, coda@...cmu.edu, codalist@...a.cs.cmu.edu,
        linux-kernel@...r.kernel.org
Cc:     Meng Xu <meng.xu@...ech.edu>, sanidhya@...ech.edu,
        Taesoo Kim <taesoo@...ech.edu>
Subject: Re: [PATCH] fs/coda: ensure the header peeked at is the same in the
 actual message

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@...il.com> 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@...il.com>
> ---
> 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
>