[uClinux-dev] [PATCH] : Avoid filename < TASK_SIZE test in do_getname() when no MMU

Jamie Lokier jamie at shareable.org
Mon May 24 13:26:08 EDT 2010


Philippe De Muyter wrote:
> On Mon, May 24, 2010 at 04:59:18PM +0100, David Howells wrote:
> > Philippe De Muyter <phdm at macqel.be> wrote:
> > 
> > > > +#else
> > > > +#define TASK_SIZE	(0xFFFFFFFFUL)
> > > > +#endif
> > > 
> > > Because of do_getname() :
> > > 
> > > 	len = TASK_SIZE - (unsigned long) filename;
> > > 
> > > we should rather have
> > > 
> > > 	#define TASK_SIZE (0x100000000ull)
> > 
> > Do you guarantee that will work everywhere on a 32-bit system, though?
> > 
> > Note that it also makes things slower as gcc has to start using 64-bit
> > arithmetic where it could otherwise use 32-bit arithmetic.
> 
> Except if gcc notices that this simplifies to
> 
> 	len = (unsigned long)(-filename);
> 
> I don't know if it does.

It did simplify on the x86 (gcc 4.4) and ARM (gcc 3.4.3) tests I just did.
Also the comparison "addr < TASK_SIZE" simplified to "1".

However I can imagine some logic like this going awry with that
definition:

    if (addr >= TASK_SIZE || len > TASK_SIZE - addr)
	return -EINVAL;

Think about the case addr == 0.

I would really rather all uses of TASK_SIZE in generic kernel code
were changed to a TASK_SIZE_CHECK macro or something like that.

There aren't all that many places where TASK_SIZE is used in generic
code (that isn't MMU specific).

TASK_SIZE is the wrong kind of check on no-MMU: A better check is that
the address is within the userspace mappable address range, whatever
that is, which may start at some value and end at some other value,
and may have holes.

-- Jamie



More information about the uClinux-dev mailing list