[PATCH 0/3] xtensa: clean up __user annotations in asm/uaccess.h

[PATCH 0/3] xtensa: clean up __user annotations in asm/uaccess.h

[Max Filippov]

Hello,

this series adds missing __user annotations to functions in
asm/uaccess.h. It fixes a bunch of sparse warnings for otherwise correct
code.

Max Filippov (3):
  xtensa: add missing __user annotations to __{get,put}_user_check
  xtensa: fix type conversion in __get_user_size
  xtensa: add missing __user annotations to asm/uaccess.h

 arch/xtensa/include/asm/uaccess.h | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

-- 
2.20.1

[PATCH 1/3] xtensa: add missing __user annotations to __{get,put}_user_check

[Max Filippov]

__get_user_check and __put_user_check use temporary pointer but don't
mark it as __user, resulting in sparse warnings:

  sparse: warning: incorrect type in initializer (different address spaces)
  sparse:    expected long *__pu_addr
  sparse:    got long [noderef] <asn:1> *ret

  sparse: warning: incorrect type in argument 1 (different address spaces)
  sparse:    expected void [noderef] <asn:1> *to
  sparse:    got long *__pu_addr

Add __user annotation to temporary pointer in __get_user_check and
__put_user_check.

Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 arch/xtensa/include/asm/uaccess.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/xtensa/include/asm/uaccess.h b/arch/xtensa/include/asm/uaccess.h
index 47b7702aaa40..754a7c96b9da 100644
--- a/arch/xtensa/include/asm/uaccess.h
+++ b/arch/xtensa/include/asm/uaccess.h
@@ -84,7 +84,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;							\
@@ -180,7 +180,7 @@ __asm__ __volatile__(					\
 #define __get_user_check(x, ptr, size)					\
 ({									\
 	long __gu_err = -EFAULT;					\
-	const __typeof__(*(ptr)) *__gu_addr = (ptr);			\
+	const __typeof__(*(ptr)) __user *__gu_addr = (ptr);		\
 	if (access_ok(__gu_addr, size))					\
 		__get_user_size((x), __gu_addr, (size), __gu_err);	\
 	else								\
-- 
2.20.1

[PATCH 2/3] xtensa: fix type conversion in __get_user_size

[Max Filippov]

8-byte access in __get_user_size converts pointer to temporary variable
to the type of original user pointer and then dereferences it, resulting
in the following sparse warning:

  sparse: warning: dereference of noderef expression

Instead dereference the original user pointer under the __typeof__ and
add indirection outside.

Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 arch/xtensa/include/asm/uaccess.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/xtensa/include/asm/uaccess.h b/arch/xtensa/include/asm/uaccess.h
index 754a7c96b9da..445bb4cf3c28 100644
--- a/arch/xtensa/include/asm/uaccess.h
+++ b/arch/xtensa/include/asm/uaccess.h
@@ -204,7 +204,7 @@ do {									\
 			retval = -EFAULT;				\
 			(x) = 0;					\
 		} else {						\
-			(x) = *(__force __typeof__((ptr)))&__x;		\
+			(x) = *(__force __typeof__(*(ptr)) *)&__x;	\
 		}							\
 		break;							\
 	}								\
-- 
2.20.1

[PATCH 3/3] xtensa: add missing __user annotations to asm/uaccess.h

[Max Filippov]

clear_user, strncpy_user, strnlen_user and their helpers operate on user
pointers, but don't have their arguments marked as __user.
Add __user annotation to userspace pointers of those functions.
Fix open-coded access check in the strnlen_user while at it.

Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 arch/xtensa/include/asm/uaccess.h | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/arch/xtensa/include/asm/uaccess.h b/arch/xtensa/include/asm/uaccess.h
index 445bb4cf3c28..e933ded0d07b 100644
--- a/arch/xtensa/include/asm/uaccess.h
+++ b/arch/xtensa/include/asm/uaccess.h
@@ -270,15 +270,15 @@ raw_copy_to_user(void __user *to, const void *from, unsigned long n)
  */
 
 static inline unsigned long
-__xtensa_clear_user(void *addr, unsigned long size)
+__xtensa_clear_user(void __user *addr, unsigned long size)
 {
-	if (!__memset(addr, 0, size))
+	if (!__memset((void __force *)addr, 0, size))
 		return size;
 	return 0;
 }
 
 static inline unsigned long
-clear_user(void *addr, unsigned long size)
+clear_user(void __user *addr, unsigned long size)
 {
 	if (access_ok(addr, size))
 		return __xtensa_clear_user(addr, size);
@@ -290,10 +290,10 @@ clear_user(void *addr, unsigned long size)
 
 #ifndef CONFIG_GENERIC_STRNCPY_FROM_USER
 
-extern long __strncpy_user(char *, const char *, long);
+extern long __strncpy_user(char *dst, const char __user *src, long count);
 
 static inline long
-strncpy_from_user(char *dst, const char *src, long count)
+strncpy_from_user(char *dst, const char __user *src, long count)
 {
 	if (access_ok(src, 1))
 		return __strncpy_user(dst, src, count);
@@ -306,13 +306,11 @@ long strncpy_from_user(char *dst, const char *src, long count);
 /*
  * Return the size of a string (including the ending 0!)
  */
-extern long __strnlen_user(const char *, long);
+extern long __strnlen_user(const char __user *str, long len);
 
-static inline long strnlen_user(const char *str, long len)
+static inline long strnlen_user(const char __user *str, long len)
 {
-	unsigned long top = __kernel_ok ? ~0UL : TASK_SIZE - 1;
-
-	if ((unsigned long)str > top)
+	if (!access_ok(str, 1))
 		return 0;
 	return __strnlen_user(str, len);
 }
-- 
2.20.1

Re: [PATCH 0/3] xtensa: clean up __user annotations in asm/uaccess.h

[Al Viro]

On Fri, May 22, 2020 at 02:41:50PM -0700, Max Filippov wrote:
> Hello,
> 
> this series adds missing __user annotations to functions in
> asm/uaccess.h. It fixes a bunch of sparse warnings for otherwise correct
> code.
> 
> Max Filippov (3):
>   xtensa: add missing __user annotations to __{get,put}_user_check
>   xtensa: fix type conversion in __get_user_size
>   xtensa: add missing __user annotations to asm/uaccess.h

Useful test:

void __user *f(void __user * __user *p)
{
	void __user *q;
	(void)get_user(q, p);
	return q;
}

Re: [PATCH 0/3] xtensa: clean up __user annotations in asm/uaccess.h

[Max Filippov]

On Fri, May 22, 2020 at 2:58 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Fri, May 22, 2020 at 02:41:50PM -0700, Max Filippov wrote:
> Useful test:
>
> void __user *f(void __user * __user *p)
> {
>         void __user *q;
>         (void)get_user(q, p);
>         return q;
> }

I think this change passes this test, i.e. originally reported warning
does not show up.
There's other kind of warning that it triggers:  'Using plain integer
as NULL pointer',
I'll post an updated version that fixes that as well.

-- 
Thanks.
-- Max