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

Philippe De Muyter phdm at macqel.be
Tue May 25 04:36:12 EDT 2010


On Tue, May 25, 2010 at 12:20:47AM +0100, Jamie Lokier wrote:
> Philippe De Muyter wrote:
> > On Mon, May 24, 2010 at 06:26:08PM +0100, Jamie Lokier wrote:
> > > 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.
> > 
> > It would give ( 0 || len > (unsigned long)(-addr))

Sorry.  I should not have answered that late.
Of course, for (addr == 0), it would give

		( 0 || len > TASK_SIZE)

and hence
		( 0 || 0)

> > I don't see a problem there.
> 
> The problem is it would return -EINVAL for any non-zero value of len,
> which isn't what the code looks like it's meant to do.

No, see above.

Of course, if one computes the maximum len (TASK_SIZE - addr) separately
and stores it in a unsigned 32bit, then there is a design error in the
program.

Philippe

-- 
Philippe De Muyter  phdm at macqel dot be  Tel +32 27029044
Macq Electronique SA  rue de l'Aeronef 2  B-1140 Bruxelles  Fax +32 27029077



More information about the uClinux-dev mailing list