[PATCH] x86/uaccess: Remove unused __addr_ok() macro

[PATCH] x86/uaccess: Remove unused __addr_ok() macro

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

This was caught while staring at the whole {set,get}_fs() machinery.

No functional changes.

Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Jann Horn <jannh@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: the arch/x86 maintainers <x86@kernel.org>
Cc: "Tobin C. Harding" <tobin@kernel.org>
---
 arch/x86/include/asm/uaccess.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index 780f2b42c8ef..e2ffedd51fd8 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -35,10 +35,7 @@ static inline void set_fs(mm_segment_t fs)
 }
 
 #define segment_eq(a, b)	((a).seg == (b).seg)
-
 #define user_addr_max() (current->thread.addr_limit.seg)
-#define __addr_ok(addr) 	\
-	((unsigned long __force)(addr) < user_addr_max())
 
 /*
  * Test whether a block of memory is a valid user space address.
-- 
2.21.0

Re: [PATCH] x86/uaccess: Remove unused __addr_ok() macro

[Linus Torvalds]

On Mon, Feb 25, 2019 at 11:11 AM Borislav Petkov <bp@alien8.de> wrote:
>
> This was caught while staring at the whole {set,get}_fs() machinery.

Heh.

You should probably have researched _when_ it became unused.

That seems to have happened in commit 5723aa993d83 ("x86: use the new
generic strnlen_user() function") which removed the single user from
the x86-32 version of strnlen_user(), which used to have

        unsigned long mask = -__addr_ok(s);

in it.

Way back in 2012.

Ack.

                 Linus

Re: [PATCH] x86/uaccess: Remove unused __addr_ok() macro

[Borislav Petkov]

On Mon, Feb 25, 2019 at 11:20:42AM -0800, Linus Torvalds wrote:
> On Mon, Feb 25, 2019 at 11:11 AM Borislav Petkov <bp@alien8.de> wrote:
> >
> > This was caught while staring at the whole {set,get}_fs() machinery.
> 
> Heh.
> 
> You should probably have researched _when_ it became unused.
> 
> That seems to have happened in commit 5723aa993d83 ("x86: use the new
> generic strnlen_user() function") which removed the single user from
> the x86-32 version of strnlen_user(), which used to have
> 
>         unsigned long mask = -__addr_ok(s);

Yap, found it. I still have

$ git log -p -G__addr_ok --pickaxe-all

in one of the shells' history here.

I'll add that to the commit message.

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH] x86/uaccess: Remove unused __addr_ok() macro

[Joe Perches]

On Mon, 2019-02-25 at 20:42 +0100, Borislav Petkov wrote:
> On Mon, Feb 25, 2019 at 11:20:42AM -0800, Linus Torvalds wrote:
> > On Mon, Feb 25, 2019 at 11:11 AM Borislav Petkov <bp@alien8.de> wrote:
> > > This was caught while staring at the whole {set,get}_fs() machinery.
> > 
> > Heh.
> > 
> > You should probably have researched _when_ it became unused.
> > 
> > That seems to have happened in commit 5723aa993d83 ("x86: use the new
> > generic strnlen_user() function") which removed the single user from
> > the x86-32 version of strnlen_user(), which used to have
> > 
> >         unsigned long mask = -__addr_ok(s);
> 
> Yap, found it. I still have
> 
> $ git log -p -G__addr_ok --pickaxe-all
> 
> in one of the shells' history here.
> 
> I'll add that to the commit message.
> 
> Thx.

Looks like it's not used in several arches

$ git grep -w __addr_ok
arch/arm/include/asm/uaccess.h:#define __addr_ok(addr)          ((void)(addr), 1)
arch/csky/include/asm/uaccess.h:#define __addr_ok(addr) (access_ok(addr, 0))
arch/openrisc/include/asm/uaccess.h:#define __addr_ok(addr) ((unsigned long) addr < get_fs())
arch/sh/include/asm/uaccess.h:#define __addr_ok(addr) \
arch/sh/include/asm/uaccess.h:  __ao_end >= __ao_a && __addr_ok(__ao_end); })
arch/x86/include/asm/uaccess.h:#define __addr_ok(addr)  \

Re: [PATCH] x86/uaccess: Remove unused __addr_ok() macro

[Christian Kujau]

On Mon, 25 Feb 2019, Joe Perches wrote:
> Looks like it's not used in several arches
> 
> $ git grep -w __addr_ok
> arch/arm/include/asm/uaccess.h:#define __addr_ok(addr)          ((void)(addr), 1)
> arch/csky/include/asm/uaccess.h:#define __addr_ok(addr) (access_ok(addr, 0))
> arch/openrisc/include/asm/uaccess.h:#define __addr_ok(addr) ((unsigned long) addr < get_fs())
> arch/sh/include/asm/uaccess.h:#define __addr_ok(addr) \
> arch/sh/include/asm/uaccess.h:  __ao_end >= __ao_a && __addr_ok(__ao_end); })
> arch/x86/include/asm/uaccess.h:#define __addr_ok(addr)  \

If so, would simly removing it do the trick or is there more magic 
involved? I don't have that many cross-compilers though and it's not even 
build-tested:


commit f899653c64cce05fde426d0298cd67670f8ab8e2
Author: Christian Kujau <lists@nerdbynature.de>
Date:   Sun Mar 3 22:43:09 2019 -0800

    Remove unused __addr_ok() macro.
    
     arch/arm/include/asm/uaccess.h      | 1 -
     arch/csky/include/asm/uaccess.h     | 2 --
     arch/openrisc/include/asm/uaccess.h | 3 ---
     arch/sh/include/asm/uaccess.h       | 5 +----
     arch/x86/include/asm/uaccess.h      | 2 --
     5 files changed, 1 insertion(+), 12 deletions(-)
    
    Signed-off-by: Christian Kujau <lists@nerdbynature.de>

diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
index 42aa4a22803c..16411c76076d 100644
--- a/arch/arm/include/asm/uaccess.h
+++ b/arch/arm/include/asm/uaccess.h
@@ -266,7 +266,6 @@ extern int __put_user_8(void *, unsigned long long);
 #define USER_DS			KERNEL_DS
 
 #define segment_eq(a, b)		(1)
-#define __addr_ok(addr)		((void)(addr), 1)
 #define __range_ok(addr, size)	((void)(addr), 0)
 #define get_fs()		(KERNEL_DS)
 
diff --git a/arch/csky/include/asm/uaccess.h b/arch/csky/include/asm/uaccess.h
index eaa1c3403a42..c02b243fecaa 100644
--- a/arch/csky/include/asm/uaccess.h
+++ b/arch/csky/include/asm/uaccess.h
@@ -24,8 +24,6 @@ static inline int access_ok(const void *addr, unsigned long size)
 		((unsigned long)(addr + size) < limit));
 }
 
-#define __addr_ok(addr) (access_ok(addr, 0))
-
 extern int __put_user_bad(void);
 
 /*
diff --git a/arch/openrisc/include/asm/uaccess.h b/arch/openrisc/include/asm/uaccess.h
index a44682c8adc3..9198371e30c2 100644
--- a/arch/openrisc/include/asm/uaccess.h
+++ b/arch/openrisc/include/asm/uaccess.h
@@ -55,9 +55,6 @@
  */
 #define __range_ok(addr, size) (size <= get_fs() && addr <= (get_fs()-size))
 
-/* Ensure that addr is below task's addr_limit */
-#define __addr_ok(addr) ((unsigned long) addr < get_fs())
-
 #define access_ok(addr, size)						\
 ({ 									\
 	unsigned long __ao_addr = (unsigned long)(addr);		\
diff --git a/arch/sh/include/asm/uaccess.h b/arch/sh/include/asm/uaccess.h
index 5fe751ad7582..b41f6a011474 100644
--- a/arch/sh/include/asm/uaccess.h
+++ b/arch/sh/include/asm/uaccess.h
@@ -5,9 +5,6 @@
 #include <asm/segment.h>
 #include <asm/extable.h>
 
-#define __addr_ok(addr) \
-	((unsigned long __force)(addr) < current_thread_info()->addr_limit.seg)
-
 /*
  * __access_ok: Check if address with size is OK or not.
  *
@@ -19,7 +16,7 @@
 #define __access_ok(addr, size)	({				\
 	unsigned long __ao_a = (addr), __ao_b = (size);		\
 	unsigned long __ao_end = __ao_a + __ao_b - !!__ao_b;	\
-	__ao_end >= __ao_a && __addr_ok(__ao_end); })
+	__ao_end >= __ao_a; })
 
 #define access_ok(addr, size)	\
 	(__chk_user_ptr(addr),		\
diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index c1334aaaa78d..d630978738dc 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -37,8 +37,6 @@ static inline void set_fs(mm_segment_t fs)
 #define segment_eq(a, b)	((a).seg == (b).seg)
 
 #define user_addr_max() (current->thread.addr_limit.seg)
-#define __addr_ok(addr) 	\
-	((unsigned long __force)(addr) < user_addr_max())
 
 /*
  * Test whether a block of memory is a valid user space address.


-- 
BOFH excuse #123:

user to computer ratio too high.

Re: [PATCH] x86/uaccess: Remove unused __addr_ok() macro

[Borislav Petkov]

On Sun, Mar 03, 2019 at 10:47:00PM -0800, Christian Kujau wrote:
> If so, would simly removing it do the trick or is there more magic 
> involved? I don't have that many cross-compilers though and it's not even 
> build-tested:

There are cross compilers here:

https://www.kernel.org/pub/tools/crosstool/

which you can use.

And perhaps splitting that patch per architecture and letting arch
maintainers apply each, would be easier. I think.

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[tip:x86/cleanups] x86/uaccess: Remove unused __addr_ok() macro

[tip-bot for Borislav Petkov]

Commit-ID:  2e7614c0736de93c8796bb2d58debb8871a59db8
Gitweb:     https://git.kernel.org/tip/2e7614c0736de93c8796bb2d58debb8871a59db8
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Mon, 25 Feb 2019 20:08:27 +0100
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Mon, 25 Feb 2019 23:13:05 +0100

x86/uaccess: Remove unused __addr_ok() macro

This was caught while staring at the whole {set,get}_fs() machinery.

It's last user, the 32-bit version of strnlen_user() went away with

  5723aa993d83 ("x86: use the new generic strnlen_user() function")

so drop it.

No functional changes.

Signed-off-by: Borislav Petkov <bp@suse.de>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Jann Horn <jannh@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: the arch/x86 maintainers <x86@kernel.org>
Cc: "Tobin C. Harding" <tobin@kernel.org>
Link: https://lkml.kernel.org/r/20190225191109.7671-1-bp@alien8.de
---
 arch/x86/include/asm/uaccess.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index a77445d1b034..ec8d36f04786 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -35,10 +35,7 @@ static inline void set_fs(mm_segment_t fs)
 }
 
 #define segment_eq(a, b)	((a).seg == (b).seg)
-
 #define user_addr_max() (current->thread.addr_limit.seg)
-#define __addr_ok(addr) 	\
-	((unsigned long __force)(addr) < user_addr_max())
 
 /*
  * Test whether a block of memory is a valid user space address.