Re: Serious bug in uaccess.h

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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        +



[Index of Archives]     [Linux MIPS Home]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Linux]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux