linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/6] OpenRISC header and sparse warning fixes for 5.9
@ 2020-08-05 21:07 Stafford Horne
  2020-08-05 21:07 ` [PATCH v2 1/6] openrisc: io: Fixup defines and move include to the end Stafford Horne
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Stafford Horne @ 2020-08-05 21:07 UTC (permalink / raw)
  To: LKML; +Cc: Stafford Horne

Hello,

Changes since v1:
 - in "io: Fixup defines and move include to the end" added a linux/types.h
   include as there were compiler failurs pointed out by kbuild.

This a series of fixes for OpenRISC sparse warnings.  The kbuild robots report
many issues related to issues with OpenRISC headers having missing or incorrect
sparse annotations.

Example kdbuild-all report:

  net/ipv4/ip_sockglue.c:1489:13: sparse: sparse: incorrect type in initializer (different address spaces)

  https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org/thread/MB6SE7BX425ENFTSIL6KAOB3CVS4WJLH/

Also this includes a few cleanups which I noticed while working on the warning
fixups.

-Stafford

Stafford Horne (6):
  openrisc: io: Fixup defines and move include to the end
  openrisc: uaccess: Fix sparse address space warnings
  openrisc: uaccess: Use static inline function in access_ok
  openrisc: uaccess: Remove unused macro __addr_ok
  openrisc: signal: Fix sparse address space warnings
  openrisc: uaccess: Add user address space check to access_ok

 arch/openrisc/include/asm/io.h      |  9 +++++++--
 arch/openrisc/include/asm/uaccess.h | 21 +++++++++++----------
 arch/openrisc/kernel/signal.c       | 14 +++++++-------
 3 files changed, 25 insertions(+), 19 deletions(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/6] openrisc: io: Fixup defines and move include to the end
  2020-08-05 21:07 [PATCH v2 0/6] OpenRISC header and sparse warning fixes for 5.9 Stafford Horne
@ 2020-08-05 21:07 ` Stafford Horne
  2020-08-05 21:07 ` [PATCH v2 2/6] openrisc: uaccess: Fix sparse address space warnings Stafford Horne
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Stafford Horne @ 2020-08-05 21:07 UTC (permalink / raw)
  To: LKML
  Cc: Stafford Horne, Jonas Bonn, Stefan Kristiansson, Mike Rapoport,
	Andrew Morton, Arnd Bergmann, Palmer Dabbelt, openrisc

This didn't seem to cause any issues, but while working on fixing up
sparse annotations for OpenRISC I noticed this.  This patch moves the
include of asm-generic/io.h to the end of the file.  Also, we add
defines of ioremap and iounmap, that way we don't get duplicate
definitions from asm-generic/io.h.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
Changes since v1:
 - Add linux/types.h include following report from kbuild

 arch/openrisc/include/asm/io.h | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/openrisc/include/asm/io.h b/arch/openrisc/include/asm/io.h
index db02fb2077d9..7d6b4a77b379 100644
--- a/arch/openrisc/include/asm/io.h
+++ b/arch/openrisc/include/asm/io.h
@@ -14,6 +14,8 @@
 #ifndef __ASM_OPENRISC_IO_H
 #define __ASM_OPENRISC_IO_H
 
+#include <linux/types.h>
+
 /*
  * PCI: can we really do 0 here if we have no port IO?
  */
@@ -25,9 +27,12 @@
 #define PIO_OFFSET		0
 #define PIO_MASK		0
 
-#include <asm-generic/io.h>
-
+#define ioremap ioremap
 void __iomem *ioremap(phys_addr_t offset, unsigned long size);
+
+#define iounmap iounmap
 extern void iounmap(void *addr);
 
+#include <asm-generic/io.h>
+
 #endif
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 2/6] openrisc: uaccess: Fix sparse address space warnings
  2020-08-05 21:07 [PATCH v2 0/6] OpenRISC header and sparse warning fixes for 5.9 Stafford Horne
  2020-08-05 21:07 ` [PATCH v2 1/6] openrisc: io: Fixup defines and move include to the end Stafford Horne
@ 2020-08-05 21:07 ` Stafford Horne
  2020-08-05 21:07 ` [PATCH v2 3/6] openrisc: uaccess: Use static inline function in access_ok Stafford Horne
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Stafford Horne @ 2020-08-05 21:07 UTC (permalink / raw)
  To: LKML
  Cc: Stafford Horne, Jonas Bonn, Stefan Kristiansson,
	Luc Van Oostenryck, openrisc

The OpenRISC user access functions put_user(), get_user() and
clear_user() were missing proper sparse annotations.  This generated
warnings like the below.

This patch adds the annotations to fix the warnings.

Example warnings:

net/ipv4/ip_sockglue.c:759:29: warning: incorrect type in argument 1 (different address spaces)
net/ipv4/ip_sockglue.c:759:29:    expected void const volatile [noderef] __user *
net/ipv4/ip_sockglue.c:759:29:    got int const *__gu_addr
net/ipv4/ip_sockglue.c:764:29: warning: incorrect type in initializer (different address spaces)
net/ipv4/ip_sockglue.c:764:29:    expected unsigned char const *__gu_addr
net/ipv4/ip_sockglue.c:764:29:    got unsigned char [noderef] __user *

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 arch/openrisc/include/asm/uaccess.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/openrisc/include/asm/uaccess.h b/arch/openrisc/include/asm/uaccess.h
index 46e31bb4a9ad..f2fc5c4b88c3 100644
--- a/arch/openrisc/include/asm/uaccess.h
+++ b/arch/openrisc/include/asm/uaccess.h
@@ -100,7 +100,7 @@ extern long __put_user_bad(void);
 #define __put_user_check(x, ptr, size)					\
 ({									\
 	long __pu_err = -EFAULT;					\
-	__typeof__(*(ptr)) *__pu_addr = (ptr);				\
+	__typeof__(*(ptr)) __user *__pu_addr = (ptr);			\
 	if (access_ok(__pu_addr, size))			\
 		__put_user_size((x), __pu_addr, (size), __pu_err);	\
 	__pu_err;							\
@@ -173,7 +173,7 @@ struct __large_struct {
 #define __get_user_check(x, ptr, size)					\
 ({									\
 	long __gu_err = -EFAULT, __gu_val = 0;				\
-	const __typeof__(*(ptr)) * __gu_addr = (ptr);			\
+	const __typeof__(*(ptr)) __user *__gu_addr = (ptr);		\
 	if (access_ok(__gu_addr, size))			\
 		__get_user_size(__gu_val, __gu_addr, (size), __gu_err);	\
 	(x) = (__force __typeof__(*(ptr)))__gu_val;			\
@@ -248,10 +248,10 @@ raw_copy_to_user(void __user *to, const void *from, unsigned long size)
 #define INLINE_COPY_FROM_USER
 #define INLINE_COPY_TO_USER
 
-extern unsigned long __clear_user(void *addr, unsigned long size);
+extern unsigned long __clear_user(void __user *addr, unsigned long size);
 
 static inline __must_check unsigned long
-clear_user(void *addr, unsigned long size)
+clear_user(void __user *addr, unsigned long size)
 {
 	if (likely(access_ok(addr, size)))
 		size = __clear_user(addr, size);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 3/6] openrisc: uaccess: Use static inline function in access_ok
  2020-08-05 21:07 [PATCH v2 0/6] OpenRISC header and sparse warning fixes for 5.9 Stafford Horne
  2020-08-05 21:07 ` [PATCH v2 1/6] openrisc: io: Fixup defines and move include to the end Stafford Horne
  2020-08-05 21:07 ` [PATCH v2 2/6] openrisc: uaccess: Fix sparse address space warnings Stafford Horne
@ 2020-08-05 21:07 ` Stafford Horne
  2020-08-05 21:07 ` [PATCH v2 4/6] openrisc: uaccess: Remove unused macro __addr_ok Stafford Horne
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Stafford Horne @ 2020-08-05 21:07 UTC (permalink / raw)
  To: LKML
  Cc: Stafford Horne, Linus Torvalds, Jonas Bonn, Stefan Kristiansson,
	Luc Van Oostenryck, openrisc

As suggested by Linus when reviewing commit 9cb2feb4d21d
("arch/openrisc: Fix issues with access_ok()") last year; making
__range_ok an inline function also fixes the used twice issue that the
commit was fixing.  I agree it's a good cleanup.  This patch addresses
that as I am currently working on the access_ok macro to fixup sparse
annotations in OpenRISC.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 arch/openrisc/include/asm/uaccess.h | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/openrisc/include/asm/uaccess.h b/arch/openrisc/include/asm/uaccess.h
index f2fc5c4b88c3..4b59dc9ad300 100644
--- a/arch/openrisc/include/asm/uaccess.h
+++ b/arch/openrisc/include/asm/uaccess.h
@@ -48,16 +48,19 @@
 /* Ensure that the range from addr to addr+size is all within the process'
  * address space
  */
-#define __range_ok(addr, size) (size <= get_fs() && addr <= (get_fs()-size))
+static inline int __range_ok(unsigned long addr, unsigned long size)
+{
+	const mm_segment_t fs = get_fs();
+
+	return size <= fs && addr <= (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);		\
-	unsigned long __ao_size = (unsigned long)(size);		\
-	__range_ok(__ao_addr, __ao_size);				\
+	__range_ok((unsigned long)(addr), (size));			\
 })
 
 /*
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 4/6] openrisc: uaccess: Remove unused macro __addr_ok
  2020-08-05 21:07 [PATCH v2 0/6] OpenRISC header and sparse warning fixes for 5.9 Stafford Horne
                   ` (2 preceding siblings ...)
  2020-08-05 21:07 ` [PATCH v2 3/6] openrisc: uaccess: Use static inline function in access_ok Stafford Horne
@ 2020-08-05 21:07 ` Stafford Horne
  2020-08-05 21:07 ` [PATCH v2 5/6] openrisc: signal: Fix sparse address space warnings Stafford Horne
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Stafford Horne @ 2020-08-05 21:07 UTC (permalink / raw)
  To: LKML
  Cc: Stafford Horne, Jonas Bonn, Stefan Kristiansson,
	Luc Van Oostenryck, openrisc

Since commit b48b2c3e5043 ("openrisc: use generic strnlen_user()
function") the macro __addr_ok is no longer used.  It is safe to remove
so this patch removes it.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 arch/openrisc/include/asm/uaccess.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/openrisc/include/asm/uaccess.h b/arch/openrisc/include/asm/uaccess.h
index 4b59dc9ad300..85a55359b244 100644
--- a/arch/openrisc/include/asm/uaccess.h
+++ b/arch/openrisc/include/asm/uaccess.h
@@ -55,9 +55,6 @@ static inline int __range_ok(unsigned long addr, unsigned long size)
 	return size <= fs && addr <= (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)						\
 ({ 									\
 	__range_ok((unsigned long)(addr), (size));			\
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 5/6] openrisc: signal: Fix sparse address space warnings
  2020-08-05 21:07 [PATCH v2 0/6] OpenRISC header and sparse warning fixes for 5.9 Stafford Horne
                   ` (3 preceding siblings ...)
  2020-08-05 21:07 ` [PATCH v2 4/6] openrisc: uaccess: Remove unused macro __addr_ok Stafford Horne
@ 2020-08-05 21:07 ` Stafford Horne
  2020-08-06 19:04   ` Luc Van Oostenryck
  2020-08-05 21:07 ` [PATCH v2 6/6] openrisc: uaccess: Add user address space check to access_ok Stafford Horne
  2020-08-06 19:11 ` [PATCH v2 0/6] OpenRISC header and sparse warning fixes for 5.9 Luc Van Oostenryck
  6 siblings, 1 reply; 15+ messages in thread
From: Stafford Horne @ 2020-08-05 21:07 UTC (permalink / raw)
  To: LKML; +Cc: Stafford Horne, Jonas Bonn, Stefan Kristiansson, openrisc

The __user annotations in signal.c were mostly missing.  The missing
annotations caused the warnings listed below.  This patch fixes them up
by adding the __user annotations.

arch/openrisc/kernel/signal.c:71:38: warning: incorrect type in initializer (different address spaces)
arch/openrisc/kernel/signal.c:71:38:    expected struct rt_sigframe *frame
arch/openrisc/kernel/signal.c:71:38:    got struct rt_sigframe [noderef] __user *
arch/openrisc/kernel/signal.c:82:14: warning: incorrect type in argument 1 (different address spaces)
arch/openrisc/kernel/signal.c:82:14:    expected void const volatile [noderef] __user *
arch/openrisc/kernel/signal.c:82:14:    got struct rt_sigframe *frame
arch/openrisc/kernel/signal.c:84:37: warning: incorrect type in argument 2 (different address spaces)
arch/openrisc/kernel/signal.c:84:37:    expected void const [noderef] __user *from
arch/openrisc/kernel/signal.c:84:37:    got struct sigset_t *
arch/openrisc/kernel/signal.c:89:39: warning: incorrect type in argument 2 (different address spaces)
arch/openrisc/kernel/signal.c:89:39:    expected struct sigcontext [noderef] __user *sc
arch/openrisc/kernel/signal.c:89:39:    got struct sigcontext *
arch/openrisc/kernel/signal.c:92:31: warning: incorrect type in argument 1 (different address spaces)
arch/openrisc/kernel/signal.c:92:31:    expected struct sigaltstack const [noderef] [usertype] __user *
arch/openrisc/kernel/signal.c:92:31:    got struct sigaltstack *
arch/openrisc/kernel/signal.c:158:15: warning: incorrect type in assignment (different address spaces)
arch/openrisc/kernel/signal.c:158:15:    expected struct rt_sigframe *frame
arch/openrisc/kernel/signal.c:158:15:    got void [noderef] __user *
arch/openrisc/kernel/signal.c:160:14: warning: incorrect type in argument 1 (different address spaces)
arch/openrisc/kernel/signal.c:160:14:    expected void const volatile [noderef] __user *
arch/openrisc/kernel/signal.c:160:14:    got struct rt_sigframe *frame
arch/openrisc/kernel/signal.c:165:46: warning: incorrect type in argument 1 (different address spaces)
arch/openrisc/kernel/signal.c:165:46:    expected struct siginfo [noderef] [usertype] __user *to
arch/openrisc/kernel/signal.c:165:46:    got struct siginfo *
arch/openrisc/kernel/signal.c:170:33: warning: incorrect type in argument 1 (different address spaces)
arch/openrisc/kernel/signal.c:170:33:    expected struct sigaltstack [noderef] [usertype] __user *
arch/openrisc/kernel/signal.c:170:33:    got struct sigaltstack *
arch/openrisc/kernel/signal.c:171:40: warning: incorrect type in argument 2 (different address spaces)
arch/openrisc/kernel/signal.c:171:40:    expected struct sigcontext [noderef] __user *sc
arch/openrisc/kernel/signal.c:171:40:    got struct sigcontext *
arch/openrisc/kernel/signal.c:173:32: warning: incorrect type in argument 1 (different address spaces)
arch/openrisc/kernel/signal.c:173:32:    expected void [noderef] __user *to
arch/openrisc/kernel/signal.c:173:32:    got struct sigset_t *

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 arch/openrisc/kernel/signal.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/openrisc/kernel/signal.c b/arch/openrisc/kernel/signal.c
index 4f0754874d78..7ce0728412f6 100644
--- a/arch/openrisc/kernel/signal.c
+++ b/arch/openrisc/kernel/signal.c
@@ -68,7 +68,7 @@ static int restore_sigcontext(struct pt_regs *regs,
 
 asmlinkage long _sys_rt_sigreturn(struct pt_regs *regs)
 {
-	struct rt_sigframe *frame = (struct rt_sigframe __user *)regs->sp;
+	struct rt_sigframe __user *frame = (struct rt_sigframe __user *)regs->sp;
 	sigset_t set;
 
 	/*
@@ -76,7 +76,7 @@ asmlinkage long _sys_rt_sigreturn(struct pt_regs *regs)
 	 * then frame should be dword aligned here.  If it's
 	 * not, then the user is trying to mess with us.
 	 */
-	if (((long)frame) & 3)
+	if (((__force unsigned long)frame) & 3)
 		goto badframe;
 
 	if (!access_ok(frame, sizeof(*frame)))
@@ -151,7 +151,7 @@ static inline void __user *get_sigframe(struct ksignal *ksig,
 static int setup_rt_frame(struct ksignal *ksig, sigset_t *set,
 			  struct pt_regs *regs)
 {
-	struct rt_sigframe *frame;
+	struct rt_sigframe __user *frame;
 	unsigned long return_ip;
 	int err = 0;
 
@@ -181,10 +181,10 @@ static int setup_rt_frame(struct ksignal *ksig, sigset_t *set,
 		l.ori r11,r0,__NR_sigreturn
 		l.sys 1
 	 */
-	err |= __put_user(0xa960,             (short *)(frame->retcode + 0));
-	err |= __put_user(__NR_rt_sigreturn,  (short *)(frame->retcode + 2));
-	err |= __put_user(0x20000001, (unsigned long *)(frame->retcode + 4));
-	err |= __put_user(0x15000000, (unsigned long *)(frame->retcode + 8));
+	err |= __put_user(0xa960,             (short __user *)(frame->retcode + 0));
+	err |= __put_user(__NR_rt_sigreturn,  (short __user *)(frame->retcode + 2));
+	err |= __put_user(0x20000001, (unsigned long __user *)(frame->retcode + 4));
+	err |= __put_user(0x15000000, (unsigned long __user *)(frame->retcode + 8));
 
 	if (err)
 		return -EFAULT;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 6/6] openrisc: uaccess: Add user address space check to access_ok
  2020-08-05 21:07 [PATCH v2 0/6] OpenRISC header and sparse warning fixes for 5.9 Stafford Horne
                   ` (4 preceding siblings ...)
  2020-08-05 21:07 ` [PATCH v2 5/6] openrisc: signal: Fix sparse address space warnings Stafford Horne
@ 2020-08-05 21:07 ` Stafford Horne
  2020-08-06 19:02   ` Luc Van Oostenryck
  2020-08-06 19:11 ` [PATCH v2 0/6] OpenRISC header and sparse warning fixes for 5.9 Luc Van Oostenryck
  6 siblings, 1 reply; 15+ messages in thread
From: Stafford Horne @ 2020-08-05 21:07 UTC (permalink / raw)
  To: LKML
  Cc: Stafford Horne, Jonas Bonn, Stefan Kristiansson,
	Luc Van Oostenryck, openrisc

Now that __user annotations are fixed for openrisc uaccess api's we can
add checking to the access_ok macro.  This patch adds the __chk_user_ptr
check, on normal builds the added check is a nop.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 arch/openrisc/include/asm/uaccess.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/openrisc/include/asm/uaccess.h b/arch/openrisc/include/asm/uaccess.h
index 85a55359b244..53ddc66abb3f 100644
--- a/arch/openrisc/include/asm/uaccess.h
+++ b/arch/openrisc/include/asm/uaccess.h
@@ -57,7 +57,8 @@ static inline int __range_ok(unsigned long addr, unsigned long size)
 
 #define access_ok(addr, size)						\
 ({ 									\
-	__range_ok((unsigned long)(addr), (size));			\
+	__chk_user_ptr(addr);						\
+	__range_ok((__force unsigned long)(addr), (size));		\
 })
 
 /*
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 6/6] openrisc: uaccess: Add user address space check to access_ok
  2020-08-05 21:07 ` [PATCH v2 6/6] openrisc: uaccess: Add user address space check to access_ok Stafford Horne
@ 2020-08-06 19:02   ` Luc Van Oostenryck
  2020-08-08 22:35     ` Stafford Horne
  0 siblings, 1 reply; 15+ messages in thread
From: Luc Van Oostenryck @ 2020-08-06 19:02 UTC (permalink / raw)
  To: Stafford Horne; +Cc: LKML, Jonas Bonn, Stefan Kristiansson, openrisc

On Thu, Aug 06, 2020 at 06:07:25AM +0900, Stafford Horne wrote:
> Now that __user annotations are fixed for openrisc uaccess api's we can
> add checking to the access_ok macro.  This patch adds the __chk_user_ptr
> check, on normal builds the added check is a nop.
> 
> Signed-off-by: Stafford Horne <shorne@gmail.com>
> ---
>  arch/openrisc/include/asm/uaccess.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/openrisc/include/asm/uaccess.h b/arch/openrisc/include/asm/uaccess.h
> index 85a55359b244..53ddc66abb3f 100644
> --- a/arch/openrisc/include/asm/uaccess.h
> +++ b/arch/openrisc/include/asm/uaccess.h
> @@ -57,7 +57,8 @@ static inline int __range_ok(unsigned long addr, unsigned long size)
>  
>  #define access_ok(addr, size)						\
>  ({ 									\
> -	__range_ok((unsigned long)(addr), (size));			\
> +	__chk_user_ptr(addr);						\
> +	__range_ok((__force unsigned long)(addr), (size));		\
>  })

Just for info, __force is not needed when casting a pointer to
unsigned long (or uintptr_t). It's not incorrect to use one
but I think it's to avoid __force as much as possible.

-- Luc

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 5/6] openrisc: signal: Fix sparse address space warnings
  2020-08-05 21:07 ` [PATCH v2 5/6] openrisc: signal: Fix sparse address space warnings Stafford Horne
@ 2020-08-06 19:04   ` Luc Van Oostenryck
  2020-08-08 22:48     ` Stafford Horne
  0 siblings, 1 reply; 15+ messages in thread
From: Luc Van Oostenryck @ 2020-08-06 19:04 UTC (permalink / raw)
  To: Stafford Horne; +Cc: LKML, Jonas Bonn, Stefan Kristiansson, openrisc

On Thu, Aug 06, 2020 at 06:07:24AM +0900, Stafford Horne wrote:
> ---
>  arch/openrisc/kernel/signal.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/openrisc/kernel/signal.c b/arch/openrisc/kernel/signal.c
> index 4f0754874d78..7ce0728412f6 100644
> --- a/arch/openrisc/kernel/signal.c
> +++ b/arch/openrisc/kernel/signal.c
> @@ -76,7 +76,7 @@ asmlinkage long _sys_rt_sigreturn(struct pt_regs *regs)
>  	 * then frame should be dword aligned here.  If it's
>  	 * not, then the user is trying to mess with us.
>  	 */
> -	if (((long)frame) & 3)
> +	if (((__force unsigned long)frame) & 3)
>  		goto badframe;

Same as patch 6, the __force is not needed.
  
-- Luc

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/6] OpenRISC header and sparse warning fixes for 5.9
  2020-08-05 21:07 [PATCH v2 0/6] OpenRISC header and sparse warning fixes for 5.9 Stafford Horne
                   ` (5 preceding siblings ...)
  2020-08-05 21:07 ` [PATCH v2 6/6] openrisc: uaccess: Add user address space check to access_ok Stafford Horne
@ 2020-08-06 19:11 ` Luc Van Oostenryck
  2020-08-08 22:49   ` Stafford Horne
  6 siblings, 1 reply; 15+ messages in thread
From: Luc Van Oostenryck @ 2020-08-06 19:11 UTC (permalink / raw)
  To: Stafford Horne; +Cc: LKML

On Thu, Aug 06, 2020 at 06:07:19AM +0900, Stafford Horne wrote:
> Hello,
> 
> Changes since v1:
>  - in "io: Fixup defines and move include to the end" added a linux/types.h
>    include as there were compiler failurs pointed out by kbuild.
> 
> This a series of fixes for OpenRISC sparse warnings.  The kbuild robots report
> many issues related to issues with OpenRISC headers having missing or incorrect
> sparse annotations.

The changes look quite good to me (I just add 2 nits for patches 5 & 6).

Fell free to add my
Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 6/6] openrisc: uaccess: Add user address space check to access_ok
  2020-08-06 19:02   ` Luc Van Oostenryck
@ 2020-08-08 22:35     ` Stafford Horne
  0 siblings, 0 replies; 15+ messages in thread
From: Stafford Horne @ 2020-08-08 22:35 UTC (permalink / raw)
  To: Luc Van Oostenryck; +Cc: LKML, Jonas Bonn, Stefan Kristiansson, openrisc

On Thu, Aug 06, 2020 at 09:02:29PM +0200, Luc Van Oostenryck wrote:
> On Thu, Aug 06, 2020 at 06:07:25AM +0900, Stafford Horne wrote:
> > Now that __user annotations are fixed for openrisc uaccess api's we can
> > add checking to the access_ok macro.  This patch adds the __chk_user_ptr
> > check, on normal builds the added check is a nop.
> > 
> > Signed-off-by: Stafford Horne <shorne@gmail.com>
> > ---
> >  arch/openrisc/include/asm/uaccess.h | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/openrisc/include/asm/uaccess.h b/arch/openrisc/include/asm/uaccess.h
> > index 85a55359b244..53ddc66abb3f 100644
> > --- a/arch/openrisc/include/asm/uaccess.h
> > +++ b/arch/openrisc/include/asm/uaccess.h
> > @@ -57,7 +57,8 @@ static inline int __range_ok(unsigned long addr, unsigned long size)
> >  
> >  #define access_ok(addr, size)						\
> >  ({ 									\
> > -	__range_ok((unsigned long)(addr), (size));			\
> > +	__chk_user_ptr(addr);						\
> > +	__range_ok((__force unsigned long)(addr), (size));		\
> >  })
> 
> Just for info, __force is not needed when casting a pointer to
> unsigned long (or uintptr_t). It's not incorrect to use one
> but I think it's to avoid __force as much as possible.

Thanks, I didn't realize that.  I will fix.

-Stafford

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 5/6] openrisc: signal: Fix sparse address space warnings
  2020-08-06 19:04   ` Luc Van Oostenryck
@ 2020-08-08 22:48     ` Stafford Horne
  2020-08-08 23:08       ` Luc Van Oostenryck
  0 siblings, 1 reply; 15+ messages in thread
From: Stafford Horne @ 2020-08-08 22:48 UTC (permalink / raw)
  To: Luc Van Oostenryck; +Cc: LKML, Jonas Bonn, Stefan Kristiansson, openrisc

On Thu, Aug 06, 2020 at 09:04:49PM +0200, Luc Van Oostenryck wrote:
> On Thu, Aug 06, 2020 at 06:07:24AM +0900, Stafford Horne wrote:
> > ---
> >  arch/openrisc/kernel/signal.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/arch/openrisc/kernel/signal.c b/arch/openrisc/kernel/signal.c
> > index 4f0754874d78..7ce0728412f6 100644
> > --- a/arch/openrisc/kernel/signal.c
> > +++ b/arch/openrisc/kernel/signal.c
> > @@ -76,7 +76,7 @@ asmlinkage long _sys_rt_sigreturn(struct pt_regs *regs)
> >  	 * then frame should be dword aligned here.  If it's
> >  	 * not, then the user is trying to mess with us.
> >  	 */
> > -	if (((long)frame) & 3)
> > +	if (((__force unsigned long)frame) & 3)
> >  		goto badframe;
> 
> Same as patch 6, the __force is not needed.

Thanks,  I thought this was complaining before, I tested now and there is no
problem so I must have been mixed up with something else.

-Stafford

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/6] OpenRISC header and sparse warning fixes for 5.9
  2020-08-06 19:11 ` [PATCH v2 0/6] OpenRISC header and sparse warning fixes for 5.9 Luc Van Oostenryck
@ 2020-08-08 22:49   ` Stafford Horne
  0 siblings, 0 replies; 15+ messages in thread
From: Stafford Horne @ 2020-08-08 22:49 UTC (permalink / raw)
  To: Luc Van Oostenryck; +Cc: LKML

On Thu, Aug 06, 2020 at 09:11:46PM +0200, Luc Van Oostenryck wrote:
> On Thu, Aug 06, 2020 at 06:07:19AM +0900, Stafford Horne wrote:
> > Hello,
> > 
> > Changes since v1:
> >  - in "io: Fixup defines and move include to the end" added a linux/types.h
> >    include as there were compiler failurs pointed out by kbuild.
> > 
> > This a series of fixes for OpenRISC sparse warnings.  The kbuild robots report
> > many issues related to issues with OpenRISC headers having missing or incorrect
> > sparse annotations.
> 
> The changes look quite good to me (I just add 2 nits for patches 5 & 6).
> 
> Fell free to add my
> Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

Thank you.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 5/6] openrisc: signal: Fix sparse address space warnings
  2020-08-08 22:48     ` Stafford Horne
@ 2020-08-08 23:08       ` Luc Van Oostenryck
  2020-08-09  8:37         ` Stafford Horne
  0 siblings, 1 reply; 15+ messages in thread
From: Luc Van Oostenryck @ 2020-08-08 23:08 UTC (permalink / raw)
  To: Stafford Horne; +Cc: LKML, Jonas Bonn, Stefan Kristiansson, openrisc

On Sun, Aug 09, 2020 at 07:48:22AM +0900, Stafford Horne wrote:
> On Thu, Aug 06, 2020 at 09:04:49PM +0200, Luc Van Oostenryck wrote:
> > On Thu, Aug 06, 2020 at 06:07:24AM +0900, Stafford Horne wrote:
> > > ---
> > >  arch/openrisc/kernel/signal.c | 14 +++++++-------
> > >  1 file changed, 7 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/arch/openrisc/kernel/signal.c b/arch/openrisc/kernel/signal.c
> > > index 4f0754874d78..7ce0728412f6 100644
> > > --- a/arch/openrisc/kernel/signal.c
> > > +++ b/arch/openrisc/kernel/signal.c
> > > @@ -76,7 +76,7 @@ asmlinkage long _sys_rt_sigreturn(struct pt_regs *regs)
> > >  	 * then frame should be dword aligned here.  If it's
> > >  	 * not, then the user is trying to mess with us.
> > >  	 */
> > > -	if (((long)frame) & 3)
> > > +	if (((__force unsigned long)frame) & 3)
> > >  		goto badframe;
> > 
> > Same as patch 6, the __force is not needed.
> 
> Thanks,  I thought this was complaining before, I tested now and there is no
> problem so I must have been mixed up with something else.

Sparse should have complained with with the cast to long but
purposely doesn't with unsigned long (or uintptr_t).
So, no mix up, I think.
 
Cheers,
-- Luc

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 5/6] openrisc: signal: Fix sparse address space warnings
  2020-08-08 23:08       ` Luc Van Oostenryck
@ 2020-08-09  8:37         ` Stafford Horne
  0 siblings, 0 replies; 15+ messages in thread
From: Stafford Horne @ 2020-08-09  8:37 UTC (permalink / raw)
  To: Luc Van Oostenryck; +Cc: LKML, Jonas Bonn, Stefan Kristiansson, openrisc

On Sun, Aug 09, 2020 at 01:08:42AM +0200, Luc Van Oostenryck wrote:
> On Sun, Aug 09, 2020 at 07:48:22AM +0900, Stafford Horne wrote:
> > On Thu, Aug 06, 2020 at 09:04:49PM +0200, Luc Van Oostenryck wrote:
> > > On Thu, Aug 06, 2020 at 06:07:24AM +0900, Stafford Horne wrote:
> > > > ---
> > > >  arch/openrisc/kernel/signal.c | 14 +++++++-------
> > > >  1 file changed, 7 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/arch/openrisc/kernel/signal.c b/arch/openrisc/kernel/signal.c
> > > > index 4f0754874d78..7ce0728412f6 100644
> > > > --- a/arch/openrisc/kernel/signal.c
> > > > +++ b/arch/openrisc/kernel/signal.c
> > > > @@ -76,7 +76,7 @@ asmlinkage long _sys_rt_sigreturn(struct pt_regs *regs)
> > > >  	 * then frame should be dword aligned here.  If it's
> > > >  	 * not, then the user is trying to mess with us.
> > > >  	 */
> > > > -	if (((long)frame) & 3)
> > > > +	if (((__force unsigned long)frame) & 3)
> > > >  		goto badframe;
> > > 
> > > Same as patch 6, the __force is not needed.
> > 
> > Thanks,  I thought this was complaining before, I tested now and there is no
> > problem so I must have been mixed up with something else.
> 
> Sparse should have complained with with the cast to long but
> purposely doesn't with unsigned long (or uintptr_t).
> So, no mix up, I think.

You are right, I noticed that right after I sent that email.  Thanks a lot.

-Stafford

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-08-09  8:37 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-05 21:07 [PATCH v2 0/6] OpenRISC header and sparse warning fixes for 5.9 Stafford Horne
2020-08-05 21:07 ` [PATCH v2 1/6] openrisc: io: Fixup defines and move include to the end Stafford Horne
2020-08-05 21:07 ` [PATCH v2 2/6] openrisc: uaccess: Fix sparse address space warnings Stafford Horne
2020-08-05 21:07 ` [PATCH v2 3/6] openrisc: uaccess: Use static inline function in access_ok Stafford Horne
2020-08-05 21:07 ` [PATCH v2 4/6] openrisc: uaccess: Remove unused macro __addr_ok Stafford Horne
2020-08-05 21:07 ` [PATCH v2 5/6] openrisc: signal: Fix sparse address space warnings Stafford Horne
2020-08-06 19:04   ` Luc Van Oostenryck
2020-08-08 22:48     ` Stafford Horne
2020-08-08 23:08       ` Luc Van Oostenryck
2020-08-09  8:37         ` Stafford Horne
2020-08-05 21:07 ` [PATCH v2 6/6] openrisc: uaccess: Add user address space check to access_ok Stafford Horne
2020-08-06 19:02   ` Luc Van Oostenryck
2020-08-08 22:35     ` Stafford Horne
2020-08-06 19:11 ` [PATCH v2 0/6] OpenRISC header and sparse warning fixes for 5.9 Luc Van Oostenryck
2020-08-08 22:49   ` Stafford Horne

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).