linux-kernel - Re: [PATCH 0/2] fix vma->anon_vma check for per-VMA locking; fix anon

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

Date:   Fri, 28 Jul 2023 02:18:20 -0700
From:   Nadav Amit <nadav.amit@...il.com>
To:     Linus Torvalds <torvalds@...uxfoundation.org>
Cc:     Will Deacon <will@...nel.org>, Jann Horn <jannh@...gle.com>,
        "Paul E. McKenney" <paulmck@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Suren Baghdasaryan <surenb@...gle.com>,
        Matthew Wilcox <willy@...radead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-mm <linux-mm@...ck.org>,
        Alan Stern <stern@...land.harvard.edu>,
        Andrea Parri <parri.andrea@...il.com>,
        Boqun Feng <boqun.feng@...il.com>,
        Nicholas Piggin <npiggin@...il.com>,
        David Howells <dhowells@...hat.com>,
        Jade Alglave <j.alglave@....ac.uk>,
        Luc Maranget <luc.maranget@...ia.fr>,
        Akira Yokosawa <akiyks@...il.com>,
        Daniel Lustig <dlustig@...dia.com>,
        Joel Fernandes <joel@...lfernandes.org>
Subject: Re: [PATCH 0/2] fix vma->anon_vma check for per-VMA locking; fix
 anon_vma memory ordering



> On Jul 27, 2023, at 1:11 PM, Nadav Amit <nadav.amit@...il.com> wrote:
> 
> 
> 
>> On Jul 27, 2023, at 12:39 PM, Linus Torvalds <torvalds@...uxfoundation.org> wrote:
>> 
>> On Thu, 27 Jul 2023 at 12:10, Nadav Amit <nadav.amit@...il.com> wrote:
>>> 
>>> Interesting. I wonder if you considered adding to READ_ONCE() something
>>> like:
>>> 
>>>       asm volatile("" : "+g" (x) );
>>> 
>>> So later loads (such as baz = *ptr) would reload the updated value.
>> 
>> Not necessarily a bad idea.  Although I suspect you'd want to add
>> *two* of them - on either side - to make sure any previous loads
>> wouldn't be moved around it either.
> 
> You are right, two are needed.
> 
> I’ll give it a shot and see if I see changes to the binary.

Just for the record (so nobody will make my mistakes):

I implemented it, but it works poorly. It appears to communicate the
constraints but the generated code is much worse.

The problem is that if the GCC inline asm decides to use a memory operand
(e.g. “+m”), it computes the address (uses LEA first before the MOV) and
allocates a register for the address. Using “+g” also causes the compiler
to allocate a register.

-- >8 --

---
 include/asm-generic/rwonce.h | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/include/asm-generic/rwonce.h b/include/asm-generic/rwonce.h
index 8d0a6280e982..c6a2f8e3013c 100644
--- a/include/asm-generic/rwonce.h
+++ b/include/asm-generic/rwonce.h
@@ -44,10 +44,34 @@
 #define __READ_ONCE(x)	(*(const volatile __unqual_scalar_typeof(x) *)&(x))
 #endif
 
-#define READ_ONCE(x)							\
-({									\
-	compiletime_assert_rwonce_type(x);				\
-	__READ_ONCE(x);							\
+#define as_scalar(x)							\
+    __builtin_choose_expr(sizeof(x) == sizeof(char),  			\
+				(__force char *)&(x), 			\
+    __builtin_choose_expr(sizeof(x) == sizeof(short),			\
+				(__force short *)&(x),			\
+    __builtin_choose_expr(sizeof(x) == sizeof(int),			\
+				(__force int *)&(x), 			\
+    __builtin_choose_expr(sizeof(x) == sizeof(long),			\
+				(__force long *)&(x),			\
+    __builtin_choose_expr(sizeof(x) == sizeof(long long),		\
+				(__force long long *)&(x),		\
+    (void*)0)))))
+
+#define READ_ONCE(x)                                                    \
+({                                                                      \
+    typeof(x) * px = &(x);						\
+    union {								\
+        typeof(x) __orig;						\
+        typeof(*as_scalar(x)) __alias;					\
+    } __u;								\
+									\
+    compiletime_assert_rwonce_type(x);					\
+    asm volatile ("" :							\
+               "+g" (*(__force typeof(*as_scalar(x)) *)(px)));		\
+    __u.__alias = __READ_ONCE(*as_scalar(*px));				\
+    asm volatile ("" :							\
+               "+g" (*(__force typeof(*as_scalar(x)) *)(px)));		\
+    __u.__orig;								\
 })
 
 #define __WRITE_ONCE(x, val)						\
-- 
2.34.1