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
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 <[email protected]>
---
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
__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 <[email protected]>
Reported-by: Arnd Bergmann <[email protected]>
Signed-off-by: Max Filippov <[email protected]>
---
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
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 <[email protected]>
---
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
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;
}
On Fri, May 22, 2020 at 2:58 PM Al Viro <[email protected]> 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