From: Borislav Petkov <[email protected]>
This was caught while staring at the whole {set,get}_fs() machinery.
No functional changes.
Signed-off-by: Borislav Petkov <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Jann Horn <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: the arch/x86 maintainers <[email protected]>
Cc: "Tobin C. Harding" <[email protected]>
---
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
On Mon, Feb 25, 2019 at 11:11 AM Borislav Petkov <[email protected]> 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
On Mon, Feb 25, 2019 at 11:20:42AM -0800, Linus Torvalds wrote:
> On Mon, Feb 25, 2019 at 11:11 AM Borislav Petkov <[email protected]> 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.
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 <[email protected]> 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) \
Commit-ID: 2e7614c0736de93c8796bb2d58debb8871a59db8
Gitweb: https://git.kernel.org/tip/2e7614c0736de93c8796bb2d58debb8871a59db8
Author: Borislav Petkov <[email protected]>
AuthorDate: Mon, 25 Feb 2019 20:08:27 +0100
Committer: Borislav Petkov <[email protected]>
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 <[email protected]>
Acked-by: Linus Torvalds <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Jann Horn <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: the arch/x86 maintainers <[email protected]>
Cc: "Tobin C. Harding" <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
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.
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 <[email protected]>
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 <[email protected]>
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.
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.