linux-mips
[Top] [All Lists]

Re: Serious bug in uaccess.h

To: "Maciej W. Rozycki" <macro@ds2.pg.gda.pl>
Subject: Re: Serious bug in uaccess.h
From: Fabrice Bellard <bellard@email.enst.fr>
Date: Tue, 27 Feb 2001 16:33:17 +0100 (MET)
Cc: Fabrice Bellard <bellard@email.enst.fr>, linux-mips@oss.sgi.com
In-reply-to: <Pine.GSO.3.96.1010227155414.2952A-100000@delta.ds2.pg.gda.pl>
Sender: owner-linux-mips@oss.sgi.com
I read the gcc docs and you are right, so it seems to be a gcc bug. I am
using gcc 2.95.2. Which version of gcc is OK to compile a linux mips
kernel ?

BTW, the kernel would be smaller by moving all the asm around __copy_user
in __copy_user itself. I am currently doing that. The cost is an added
'jr' to jump to __memcpy. Do you think it is worthwhile to do that ?

Fabrice.

On Tue, 27 Feb 2001, Maciej W. Rozycki wrote:

> On Tue, 27 Feb 2001, Fabrice Bellard wrote:
> 
> > I found a serious bug in the assembler macros in asm-mips/uaccess.h. They
> > all do something like that:
> > 
> >             __asm__ __volatile__( \
> >                     "move\t$4, %1\n\t" \
> >                     "move\t$5, %2\n\t" \
> >                     "move\t$6, %3\n\t" \
> >                     ".set\tnoreorder\n\t" \
> >                     __MODULE_JAL(__copy_user) \
> > ...
> > 
> > The problem is that you cannot assume that gcc will not put %1, %2 or %3
> > in registers different from $4, $5 or $6. For example, if %2 is put in $4,
> > the code is incorrect. (With gcc-2.95.2 I got a bug in
> > generic_file_write!).
> 
>  Hmm, haven't looked through gcc sources, but docs state: "It is an error
> for a clobber description to overlap an input or output operand (for
> example, an operand describing a register class with one member, mentioned
> in the clobber list)."  I guess it implies clobbers are not used for input
> or output.  It's reasonable anyway and if gcc acts otherwise, you might
> just have caught a bug in gcc.
> 
> > A possible fix would be to use asm registers:
> > 
> > #define copy_from_user(to,from,n) ({ \
> >     register void *__cu_to asm("$4"); \
> >     register const void *__cu_from asm("$5"); \
> >     register long __cu_len asm("$6"); \
> >     \
> >     __cu_to = (to); \
> >     __cu_from = (from); \
> >     __cu_len = (n); \
> >     if (access_ok(VERIFY_READ, __cu_from, __cu_len)) \
> >             __asm__ __volatile__( \
> >                     ".set\tnoreorder\n\t" \
> >                     __MODULE_JAL(__copy_user) \
> > ...
> > 
> > But I am not sure that it is always correct. Any idea ?
> 
>  This is fine and saves us three instructions.  Go on, make a patch (I'd
> suggest using "__asm__" for consistency, though)! 
> 
> -- 
> +  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
> +--------------------------------------------------------------+
> +        e-mail: macro@ds2.pg.gda.pl, PGP key available        +
> 


<Prev in Thread] Current Thread [Next in Thread>