Re: [PATCH] mm: fix mmap overflow checking
On 09/08/2012 09:58 AM, KOSAKI Motohiro wrote: >>> I've seen the exactly same patch from another fujitsu guys several >>> month ago. and as I pointed >>> out at that time, this line don't work when 32bit kernel + mmap2 syscall >>> case. >>> >>> Please don't think do_mmap_pgoff() is for mmap(2) specific and read a >>> past thread before resend >>> a patch. >> >> So, what's your opinion about this bug? How to fix it in your mind? > > Fix glibc instead of kernel. > OK, I will send a patch to glibc community. Thank you. Wanlong Gao -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: fix mmap overflow checking
>> I've seen the exactly same patch from another fujitsu guys several >> month ago. and as I pointed >> out at that time, this line don't work when 32bit kernel + mmap2 syscall >> case. >> >> Please don't think do_mmap_pgoff() is for mmap(2) specific and read a >> past thread before resend >> a patch. > > So, what's your opinion about this bug? How to fix it in your mind? Fix glibc instead of kernel. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: fix mmap overflow checking
On Tue, Sep 4, 2012 at 5:23 AM, Wanlong Gao wrote:
> POSIX said that if the file is a regular file and the value of "off"
> plus "len" exceeds the offset maximum established in the open file
> description associated with fildes, mmap should return EOVERFLOW.
>
> The following test from LTP can reproduce this bug.
>
> char tmpfname[256];
> void *pa = NULL;
> void *addr = NULL;
> size_t len;
> int flag;
> int fd;
> off_t off = 0;
> int prot;
>
> long page_size = sysconf(_SC_PAGE_SIZE);
>
> snprintf(tmpfname, sizeof(tmpfname), "/tmp/mmap_test_%d", getpid());
> unlink(tmpfname);
> fd = open(tmpfname, O_CREAT | O_RDWR | O_EXCL, S_IRUSR | S_IWUSR);
> if (fd == -1) {
> printf(" Error at open(): %s\n", strerror(errno));
> return 1;
> }
> unlink(tmpfname);
>
> flag = MAP_SHARED;
> prot = PROT_READ | PROT_WRITE;
>
> /* len + off > maximum offset */
>
> len = ULONG_MAX;
> if (len % page_size) {
> /* Lower boundary */
> len &= ~(page_size - 1);
> }
>
> off = ULONG_MAX;
> if (off % page_size) {
> /* Lower boundary */
> off &= ~(page_size - 1);
> }
>
> printf("off: %lx, len: %lx\n", (unsigned long)off, (unsigned
> long)len);
> pa = mmap(addr, len, prot, flag, fd, off);
> if (pa == MAP_FAILED && errno == EOVERFLOW) {
> printf("Test Pass: Error at mmap: %s\n", strerror(errno));
> return 0;
> }
>
> if (pa == MAP_FAILED)
> perror("Test FAIL: expect EOVERFLOW but get other error");
> else
> printf("Test FAIL : Expect EOVERFLOW but got no error\n");
>
> close(fd);
> munmap(pa, len);
> return 1;
>
> Cc: Andrew Morton
> Cc: KOSAKI Motohiro
> Cc: linux...@kvack.org (open list:MEMORY MANAGEMENT)
> Signed-off-by: Wanlong Gao
> ---
> mm/mmap.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index ae18a48..5380764 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -980,6 +980,7 @@ unsigned long do_mmap_pgoff(struct file *file, unsigned
> long addr,
> struct mm_struct * mm = current->mm;
> struct inode *inode;
> vm_flags_t vm_flags;
> + off_t off = pgoff << PAGE_SHIFT;
I've seen the exactly same patch from another fujitsu guys several
month ago. and as I pointed
out at that time, this line don't work when 32bit kernel + mmap2 syscall case.
Please don't think do_mmap_pgoff() is for mmap(2) specific and read a
past thread before resend
a patch.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: fix mmap overflow checking
On Tue, Sep 4, 2012 at 5:23 AM, Wanlong Gao gaowanl...@cn.fujitsu.com wrote:
POSIX said that if the file is a regular file and the value of off
plus len exceeds the offset maximum established in the open file
description associated with fildes, mmap should return EOVERFLOW.
The following test from LTP can reproduce this bug.
char tmpfname[256];
void *pa = NULL;
void *addr = NULL;
size_t len;
int flag;
int fd;
off_t off = 0;
int prot;
long page_size = sysconf(_SC_PAGE_SIZE);
snprintf(tmpfname, sizeof(tmpfname), /tmp/mmap_test_%d, getpid());
unlink(tmpfname);
fd = open(tmpfname, O_CREAT | O_RDWR | O_EXCL, S_IRUSR | S_IWUSR);
if (fd == -1) {
printf( Error at open(): %s\n, strerror(errno));
return 1;
}
unlink(tmpfname);
flag = MAP_SHARED;
prot = PROT_READ | PROT_WRITE;
/* len + off maximum offset */
len = ULONG_MAX;
if (len % page_size) {
/* Lower boundary */
len = ~(page_size - 1);
}
off = ULONG_MAX;
if (off % page_size) {
/* Lower boundary */
off = ~(page_size - 1);
}
printf(off: %lx, len: %lx\n, (unsigned long)off, (unsigned
long)len);
pa = mmap(addr, len, prot, flag, fd, off);
if (pa == MAP_FAILED errno == EOVERFLOW) {
printf(Test Pass: Error at mmap: %s\n, strerror(errno));
return 0;
}
if (pa == MAP_FAILED)
perror(Test FAIL: expect EOVERFLOW but get other error);
else
printf(Test FAIL : Expect EOVERFLOW but got no error\n);
close(fd);
munmap(pa, len);
return 1;
Cc: Andrew Morton a...@linux-foundation.org
Cc: KOSAKI Motohiro kosaki.motoh...@jp.fujitsu.com
Cc: linux...@kvack.org (open list:MEMORY MANAGEMENT)
Signed-off-by: Wanlong Gao gaowanl...@cn.fujitsu.com
---
mm/mmap.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/mm/mmap.c b/mm/mmap.c
index ae18a48..5380764 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -980,6 +980,7 @@ unsigned long do_mmap_pgoff(struct file *file, unsigned
long addr,
struct mm_struct * mm = current-mm;
struct inode *inode;
vm_flags_t vm_flags;
+ off_t off = pgoff PAGE_SHIFT;
I've seen the exactly same patch from another fujitsu guys several
month ago. and as I pointed
out at that time, this line don't work when 32bit kernel + mmap2 syscall case.
Please don't think do_mmap_pgoff() is for mmap(2) specific and read a
past thread before resend
a patch.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: fix mmap overflow checking
I've seen the exactly same patch from another fujitsu guys several month ago. and as I pointed out at that time, this line don't work when 32bit kernel + mmap2 syscall case. Please don't think do_mmap_pgoff() is for mmap(2) specific and read a past thread before resend a patch. So, what's your opinion about this bug? How to fix it in your mind? Fix glibc instead of kernel. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: fix mmap overflow checking
On 09/08/2012 09:58 AM, KOSAKI Motohiro wrote: I've seen the exactly same patch from another fujitsu guys several month ago. and as I pointed out at that time, this line don't work when 32bit kernel + mmap2 syscall case. Please don't think do_mmap_pgoff() is for mmap(2) specific and read a past thread before resend a patch. So, what's your opinion about this bug? How to fix it in your mind? Fix glibc instead of kernel. OK, I will send a patch to glibc community. Thank you. Wanlong Gao -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: fix mmap overflow checking
On Tue, 4 Sep 2012 17:23:00 +0800 Wanlong Gao wrote: > POSIX said that if the file is a regular file and the value of "off" > plus "len" exceeds the offset maximum established in the open file > description associated with fildes, mmap should return EOVERFLOW. That's what POSIX says, but what does Linux do? It is important that we precisely describe and understand the behaviour change, as there is potential here to break existing applications. I'm assuming that Linux presently permits the mmap() and then generates SIGBUS if an access is attempted beyond the max file size? > /* offset overflow? */ > - if ((pgoff + (len >> PAGE_SHIFT)) < pgoff) > - return -EOVERFLOW; > + if (off + len < off) > + return -EOVERFLOW; Well, this treats sizeof(off_t) as the "offset maximum established in the open file". But from my reading of the above excerpt, we should in fact be checking against the underlying fs's s_maxbytes? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: fix mmap overflow checking
On Tue, 4 Sep 2012 17:23:00 +0800 Wanlong Gao gaowanl...@cn.fujitsu.com wrote: POSIX said that if the file is a regular file and the value of off plus len exceeds the offset maximum established in the open file description associated with fildes, mmap should return EOVERFLOW. That's what POSIX says, but what does Linux do? It is important that we precisely describe and understand the behaviour change, as there is potential here to break existing applications. I'm assuming that Linux presently permits the mmap() and then generates SIGBUS if an access is attempted beyond the max file size? /* offset overflow? */ - if ((pgoff + (len PAGE_SHIFT)) pgoff) - return -EOVERFLOW; + if (off + len off) + return -EOVERFLOW; Well, this treats sizeof(off_t) as the offset maximum established in the open file. But from my reading of the above excerpt, we should in fact be checking against the underlying fs's s_maxbytes? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
