linux-kernel - Re: x86-32: clean up rwsem inline asm statements

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	13 Jan 2010 14:58:28 -0500
From:	"George Spelvin" <linux@...izon.com>
To:	hpa@...or.com
Cc:	linux@...izon.com, linux-kernel@...r.kernel.org,
	schwab@...ux-m68k.org, torvalds@...ux-foundation.org
Subject: Re: x86-32: clean up rwsem inline asm statements

> As far as I can tell, very few of these assembly statements actually
> need a size at all -- the very first inc statement is purely to memory,
> and as such it needs a size marker, but almost any operation which is
> register-register or register-memory will simply take its size from the
> register operand.  For those, it seems cleaner to simply drop the size
> suffix, and in fact this is the style we have been pushing people
> towards (use the suffix where mandatory or where the size is fixed
> anyway, to help catch bugs; use no suffix where the size can vary and is
> implied by the operands.)

The one thing is that for a register-memory operation, using the
size of the memory operand can catch bugs where it doesn't match
the size of the register operand.

GCC's inline asm doesn't make operand size very implicit, and it's
awkward to cast output operands, so there's a potential for bugs.
I especially get nervous when the operand itself is an immediate
constant, as I can't remember the ANSI rules for the type very well.
(Quick: is 0x80000000 an unsigned 32-bit int or a signed 64-bit one?
What about 2147483648 or 1<<31?)

Since this is mostly inline functions, it's not so big a problem, but
I'd consider something like:

static inline void __up_write(struct rw_semaphore *sem)
{
       unsigned long tmp;
	asm volatile("# beginning __up_write\n\t"
		     LOCK_PREFIX "  xadd%z0	%1,(%2)\n\t"
		     /* tries to transition
			0xffff0001 -> 0x00000000 */
		     "  jz	1f\n"
		     "  call call_rwsem_wake\n"
		     "1:\n\t"
		     "# ending __up_write\n"
       	     : "+m" (sem->count), "=d" (tmp)
       	     : "a" (sem), "1" (-RWSEM_ACTIVE_WRITE_BIAS)
       	     : "memory", "cc");
}

Just in case the size of -RWSEM_ACTIVE_WRITE_BIAS doesn't match that
of sem->count.  It'll explode when you try to run it, of course, but
there's something to be said for compile-time errors.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/