Subject: Serious bug in uaccess.h

From: Fabrice Bellard <[email-masked]>
Date: Tue, 27 Feb 2001 15:40:11 +0100 (MET)

Hi!

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!).

Did someone already fixed this bug ?

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 ?

Fabrice.

---

From: Fabrice Bellard <[email-masked]>
Date: Tue, 27 Feb 2001 16:33:17 +0100 (MET)

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:
> 
>  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.
> 
>  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: [email-masked], PGP key available        +
>

---

From: "Maciej W. Rozycki" <[email-masked]>
Date: Tue, 27 Feb 2001 16:13:33 +0100 (MET)

---

From: Ralf Baechle <[email-masked]>
Date: Tue, 27 Feb 2001 18:50:04 +0100

> But I am not sure that it is always correct. Any idea ?

While this should be correct - it was my original attempt also - it was
misscompiled by whatever C compiler I was using at that time.  If
egcs 1.1.2 and gcc 2.95 both compile this construct right now then I'd
say go for it.

  Ralf

---

From: "Maciej W. Rozycki" <[email-masked]>
Date: Tue, 27 Feb 2001 18:54:22 +0100 (MET)

> using gcc 2.95.2. Which version of gcc is OK to compile a linux mips

 Patched 2.95.2 should be fine; RPM packages of what I use are available
at 'ftp://ftp.ds2.pg.gda.pl/pub/macro/'. 

> 'jr' to jump to __memcpy. Do you think it is worthwhile to do that ?

 What asm do you mean?

---

From: Fabrice Bellard <[email-masked]>
Date: Tue, 27 Feb 2001 23:22:27 +0100

> > 'jr' to jump to __memcpy. Do you think it is worthwhile to do that ?

I mean the code in arch/mips/lib/memcpy.S. It is possible to modify
__copy_user so that it has exactly the same calling convention of a C
function. Then, no asm is necessary in uaccess.h. It costs us a
supplementary jump.

Fabrice.

---

From: "Maciej W. Rozycki" <[email-masked]>
Date: Wed, 28 Feb 2001 13:47:27 +0100 (MET)

> function. Then, no asm is necessary in uaccess.h. It costs us a

 You mean the supplementary return value in a2?  Hmm, it is always set to
zero!  Also "addu $1, %2, %3" makes no sense.

 Ralf, the code is weird.  The header implies you are the author.  Could
you please elaborate what you meant in copy_*_user()?

---

From: Ralf Baechle <[email-masked]>
Date: Wed, 28 Feb 2001 13:56:28 +0100

On Wed, Feb 28, 2001 at 01:47:27PM +0100, Maciej W. Rozycki wrote:
> Date: Wed, 28 Feb 2001 13:47:27 +0100 (MET)
> From: "Maciej W. Rozycki" <[email-masked]>
> To: Fabrice Bellard <[email-masked]>,
>         Ralf Baechle <[email-masked]>
> cc: [email-masked]
> Subject: Re: Serious bug in uaccess.h
> 

I'll look at it and try to recall :-)

  Ralf