linux-kernel - Re: [PATCH] x86/rwsem: Save and restore all callee-clobbered regs in 32-bit ____down

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-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 13 May 2016 19:19:15 +0200
From:	Borislav Petkov <bp@...e.de>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Guenter Roeck <linux@...ck-us.net>, Ingo Molnar <mingo@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	"linux-next@...r.kernel.org" <linux-next@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] x86/rwsem: Save and restore all callee-clobbered regs in
 32-bit ____down_write()

On Fri, May 13, 2016 at 10:03:26AM -0700, Linus Torvalds wrote:
> I think your fix is wrong. Your fix adds the pointless push/pop that
> doesn't help any, since you might as well just force the temporary
> back to %edx.

But if we do this, then everything using the slow path
call_rwsem_down_write_failed et al, which then calls
{save,restore}_common_regs, would have to remember to use %edx as
temporary because {save,restore}_common_regs won't protect it and gcc
might clobber it.

OTOH, the 64-bit versions {save,restore}_common_regs don't stash away
%rdx either so I guess that mechanism was supposed to not save the ABI
return registers rAX and rDX.

The only thing that needs to be corrected then is the misleading comment
above the 32-bit version "... Save the C-clobbered registers (%eax, %edx
and %ecx) .." - the 64-bit version comment is correct AFAICT.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--