linux-kernel - Re: remap_file_pages() broken in 2.6.23?

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: <20071129233058.GA10359@wotan.suse.de>
Date:	Fri, 30 Nov 2007 00:30:58 +0100
From:	Nick Piggin <npiggin@...e.de>
To:	Chuck Ebbert <cebbert@...hat.com>
Cc:	linux-kernel <linux-kernel@...r.kernel.org>,
	Hugh Dickins <hugh@...itas.com>, Ingo Molnar <mingo@...e.hu>
Subject: Re: remap_file_pages() broken in 2.6.23?

On Thu, Nov 29, 2007 at 02:45:23PM -0500, Chuck Ebbert wrote:
> Original report: https://bugzilla.redhat.com/show_bug.cgi?id=404201
> 
> The test case below, taken from the LTP test code, prints -1 (as
> expected) on 2.6.22 and 0 on 2.6.23. It tries to remap an out-of-range
> page. Proposed patch follows the program. Bug was apparently caused by
> commit 54cb8821de07f2ffcd28c380ce9b93d5784b40d7.

Ah, that's not such good behaviour anyway. mmap is allowed to map
outside the file offset, so you're telling me that remap_file_pages
just magically should not be allowed to remap these...?
 

> Patch:
> 
> Signed-off-by: Supriya Kannery <supriyak@...ibm.com>
> 
> --- linux-2.6.23/mm/fremap.c.orig	2007-11-22 00:56:09.000000000 -0600
> +++ linux-2.6.23/mm/fremap.c	2007-11-26 03:08:55.000000000 -0600
> @@ -124,6 +124,7 @@ asmlinkage long sys_remap_file_pages(uns
>  	struct vm_area_struct *vma;
>  	int err = -EINVAL;
>  	int has_write_lock = 0;
> +	unsigned long f_size = 0;
>  
>  	if (__prot)
>  		return err;
> @@ -181,6 +182,14 @@ asmlinkage long sys_remap_file_pages(uns
>  			goto retry;
>  		}
>  		mapping = vma->vm_file->f_mapping;
> +
> +		f_size = i_size_read(mapping->host) + PAGE_CACHE_SIZE - 1;
> +		f_size = f_size >> PAGE_CACHE_SHIFT;
> +		if ((pgoff + size >> PAGE_CACHE_SHIFT) > f_size) {
> +			err = -EINVAL;
> +			goto out;
> +		}
> +
>  		/*
>  		 * page_mkclean doesn't work on nonlinear vmas, so if
>  		 * dirty pages need to be accounted, emulate with linear


I don't think there is anything preventing truncate races here. Theoretically
we could do it by taking i_mutex around here, but anyway then a subsequent
truncate is just going to be able to cause the mapping to be out of bounds
anyway.

If it were any other syscall than remap_file_pages, I'd be much more
hesitant to say this: I propose we change the test case instead. I
also changed other elements of the API, and we had the result tested
and verified by Oracle...

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/