From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sasha Levin Subject: Re: [PATCH v2] vfs: read file_handle only once in handle_to_path Date: Tue, 02 Jun 2015 11:23:35 -0400 Message-ID: <556DCA77.2040001@oracle.com> References: <1422477048-4550-1-git-send-email-sasha.levin@oracle.com> <55424464.9030906@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: Alexander Viro , linux-fsdevel@vger.kernel.org, "security@kernel.org" To: linux-kernel@vger.kernel.org, Linus Torvalds Return-path: Received: from userp1040.oracle.com ([156.151.31.81]:21710 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759180AbbFBPXr (ORCPT ); Tue, 2 Jun 2015 11:23:47 -0400 In-Reply-To: <55424464.9030906@oracle.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: Hi, This commit fixes a race condition an unprivileged user can exploit to corrupt kernel memory. I can't seem to get anyone to grab it for the past 4+ months. Help please? Thanks, Sasha On 04/30/2015 11:04 AM, Sasha Levin wrote: > ping? > > On 01/28/2015 03:30 PM, Sasha Levin wrote: >> We used to read file_handle twice. Once to get the amount of extra bytes, and >> once to fetch the entire structure. >> >> This may be problematic since we do size verifications only after the first >> read, so if the number of extra bytes changes in userspace between the first >> and second calls, we'll have an incoherent view of file_handle. >> >> Instead, read the constant size once, and copy that over to the final >> structure without having to re-read it again. >> >> Signed-off-by: Sasha Levin >> --- >> Change in v2: >> - Use the f_handle pointer rather than size of struct >> >> fs/fhandle.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/fs/fhandle.c b/fs/fhandle.c >> index 999ff5c..d59712d 100644 >> --- a/fs/fhandle.c >> +++ b/fs/fhandle.c >> @@ -195,8 +195,9 @@ static int handle_to_path(int mountdirfd, struct file_handle __user *ufh, >> goto out_err; >> } >> /* copy the full handle */ >> - if (copy_from_user(handle, ufh, >> - sizeof(struct file_handle) + >> + *handle = f_handle; >> + if (copy_from_user(&handle->f_handle, >> + &ufh->f_handle, >> f_handle.handle_bytes)) { >> retval = -EFAULT; >> goto out_handle; >> >