Re: [RFC/PATCH mmap2: better determine overflow

[Randy Dunlap <rdunlap@xenotime.net>]

On Tue, 26 Sep 2006 19:10:41 +0100 (BST) Hugh Dickins wrote:

> On Tue, 26 Sep 2006, Randy Dunlap wrote:
> > From: Randy Dunlap <rdunlap@xenotime.net>
> > 
> > mm/mmap.c::do_mmap_pgoff() checks for overflow like:
> > 
> > 	/* offset overflow? */
> > 	if ((pgoff + (len >> PAGE_SHIFT)) < pgoff)
> >                return -EOVERFLOW;
> > 
> > However, using pgoff (page indexes) to determine address range
> > overflow doesn't overflow.  Change to use byte offsets instead,
> > so that overflow can actually happen and be noticed.
> 
> I think you're mistaken there.  Thinking in particular of 32-bit
> arches, isn't the check precisely about allowing an mmap at a high
> offset of a file >4GB in length; but not at so high an offset that
> pgoff (page index) wraps back to 0?  Whereas you're changing it
> now to fail at 4GB.

OK, I think I see.  I'll check/test/verify more.

> > Also return EOVERFLOW instead of ENOMEM when PAGE_ALIGN(len)
> > is 0.
> 
> Which standard mandates that change?

It was an interpretation.  Perhaps a mis-interpretation.
This comes after:

	if (!len)
		return -EINVAL;
...then

 	len = PAGE_ALIGN(len);
b:
-	if (!len || len > TASK_SIZE)
-		return -ENOMEM;
+	if (!len)
+		return -EOVERFLOW;

so if len is 0 at b:, then it was a very large unsigned long number
(larger than 0 - PAGE_SIZE, i.e., >= 0xfffff001 on 32-bit or
>= 0xffffffff_fffff001 on 64-bit), and PAGE_ALIGN() rounded it "up"
to 0.  That seems more like an overflow than a NOMEM to me.
That's all.


So, I'm interested in the EOVERFLOW case(s).
Would you attempt to translate this return value case for me?
(from http://www.opengroup.org/onlinepubs/009695399/functions/mmap.html:)

[EOVERFLOW]
    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.

I'm not concerned about the "off plus len" since I am looking at
mmap2() [using pgoff's instead].  I'm more concerned about the
"offset maximum established in the open file description associated
with fildes."

Does mmap2() on Linux use the actual filesize as a limit for the
mmap() area [not that I can see] or does it just use (effectively)
ULONG_MAX, without regard file actual filesize?

Thanks for looking/helping.

> Hugh
> 
> > 
> > Tested on i686 and x86_64.
> > 
> > Test program is at:  http://www.xenotime.net/linux/src/mmap-test.c
> > 
> > Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
> > ---
> >  mm/mmap.c |    9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > --- linux-2618-work.orig/mm/mmap.c
> > +++ linux-2618-work/mm/mmap.c
> > @@ -923,13 +923,16 @@ unsigned long do_mmap_pgoff(struct file 
> >  
> >  	/* Careful about overflows.. */
> >  	len = PAGE_ALIGN(len);
> > -	if (!len || len > TASK_SIZE)
> > -		return -ENOMEM;
> > +	if (!len)
> > +		return -EOVERFLOW;
> >  
> >  	/* offset overflow? */
> > -	if ((pgoff + (len >> PAGE_SHIFT)) < pgoff)
> > +	if (((pgoff << PAGE_SHIFT) + len) < (pgoff << PAGE_SHIFT))
> >                 return -EOVERFLOW;
> >  
> > +	if (len > TASK_SIZE)
> > +		return -ENOMEM;
> > +
> >  	/* Too many mappings? */
> >  	if (mm->map_count > sysctl_max_map_count)
> >  		return -ENOMEM;
> > 
> > ---

---
~Randy

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>