linux-kernel - Re: [PATCH] x86: Make variable_test

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]

Message-ID: <990b8719-066b-4e1d-892f-6a3ea4241fb2@email.android.com>
Date:	Thu, 06 Oct 2011 18:37:27 -0700
From:	"hpanvin@...il.com" <hpa@...or.com>
To:	Josh Stone <jistone@...hat.com>, linux-kernel@...r.kernel.org
CC:	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, x86@...nel.org,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	Srikar Dronamraju <srikar@...ux.vnet.ibm.com>,
	Jakub Jelinek <jakub@...hat.com>
Subject: Re: [PATCH] x86: Make variable_test_bit reference all of *addr

This is concerning... the kernel relies heavily on asm volatile being a universal memory consumer.  If that is suddenly broken, we are f*** in many, many, MANY places in the kernel all of a sudden!

Josh Stone <jistone@...hat.com> wrote:

>This casts the addr that's fed to asm into a struct-array pointer, so
>gcc knows that more than just the first long is needed.  Since there's
>no fixed size for all callers, an arbitrary size is chosen just to
>ensure that it's probably good enough.
>
>I noticed this warning on i686, with gcc-4.6.1-9.fc15:
>
>  CC      arch/x86/kernel/kprobes.o
>In file included from include/linux/bitops.h:22:0,
>                 from include/linux/kernel.h:17,
>                 from [...]/arch/x86/include/asm/percpu.h:44,
>                 from [...]/arch/x86/include/asm/current.h:5,
>                 from [...]/arch/x86/include/asm/processor.h:15,
>                 from [...]/arch/x86/include/asm/atomic.h:6,
>                 from include/linux/atomic.h:4,
>                 from include/linux/mutex.h:18,
>                 from include/linux/notifier.h:13,
>                 from include/linux/kprobes.h:34,
>                 from arch/x86/kernel/kprobes.c:43:
>[...]/arch/x86/include/asm/bitops.h: In function ‘can_boost.part.1’:
>[...]/arch/x86/include/asm/bitops.h:319:2: warning: use of memory input
>without lvalue in asm operand 1 is deprecated [enabled by default]
>
>In investigating the impact of this warning, I discovered that only the
>first long of the 32-byte twobyte_is_boostable[] was making into the
>object file.
>
>Jakub advised that variable_test_bit is incorrectly telling gcc that
>its
>asm only uses a single long from the addr pointer, and he suggested the
>struct-array cast to broaden the memory reference.
>
>Signed-off-by: Josh Stone <jistone@...hat.com>
>Cc: Jakub Jelinek <jakub@...hat.com>
>
>---
>
>An alternate fix would be to make kprobes' twobyte_is_boostable[]
>volatile, which forces gcc to keep it around.  I feel that's treating
>the symptom though, rather than the cause in variable_test_bit().
>
>IMO this is also a good candidate for -stable, for fixing the obviously
>bad
>data behavior, but I'll let others judge...
>
>---
> arch/x86/include/asm/bitops.h |    3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
>diff --git a/arch/x86/include/asm/bitops.h
>b/arch/x86/include/asm/bitops.h
>index 1775d6e..0565371 100644
>--- a/arch/x86/include/asm/bitops.h
>+++ b/arch/x86/include/asm/bitops.h
>@@ -319,7 +319,8 @@ static inline int variable_test_bit(int nr,
>volatile const unsigned long *addr)
> 	asm volatile("bt %2,%1\n\t"
> 		     "sbb %0,%0"
> 		     : "=r" (oldbit)
>-		     : "m" (*(unsigned long *)addr), "Ir" (nr));
>+		     : "m" (*(struct { unsigned long _[0x10000]; } *)addr),
>+		       "Ir" (nr));
> 
> 	return oldbit;
> }
>-- 
>1.7.6.4

-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
--
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/