| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <54A6BC0E.50008@imgtec.com>
Date: Fri, 2 Jan 2015 15:41:02 +0000
From: James Hogan <james.hogan@...tec.com>
To: "Michael S. Tsirkin" <mst@...hat.com>,
<linux-kernel@...r.kernel.org>
CC: Arnd Bergmann <arnd@...db.de>, <linux-arch@...r.kernel.org>,
<linux-metag@...r.kernel.org>
Subject: Re: [PATCH repost 09/16] metag/uaccess: fix sparse errors
Hi,
On 25/12/14 09:29, Michael S. Tsirkin wrote:
> virtio wants to read bitwise types from userspace using get_user. At the
> moment this triggers sparse errors, since the value is passed through an
> integer.
>
> Fix that up using __force.
I still see these sparse warnings with metag even with your patch:
CHECK drivers/vhost/vhost.c
drivers/vhost/vhost.c:1004:13: warning: cast from restricted __virtio16
drivers/vhost/vhost.c:1004:13: warning: cast from restricted __virtio16
drivers/vhost/vhost.c:1004:13: warning: cast from restricted __virtio16
drivers/vhost/vhost.c:1004:13: warning: cast from restricted __virtio16
drivers/vhost/vhost.c:1022:13: warning: cast from restricted __virtio16
drivers/vhost/vhost.c:1022:13: warning: cast from restricted __virtio16
drivers/vhost/vhost.c:1022:13: warning: cast from restricted __virtio16
drivers/vhost/vhost.c:1022:13: warning: cast from restricted __virtio16
drivers/vhost/vhost.c:1373:21: warning: cast from restricted __virtio32
drivers/vhost/vhost.c:1373:21: warning: cast from restricted __virtio32
drivers/vhost/vhost.c:1373:21: warning: cast from restricted __virtio32
drivers/vhost/vhost.c:1373:21: warning: cast from restricted __virtio32
drivers/vhost/vhost.c:1377:21: warning: cast from restricted __virtio32
drivers/vhost/vhost.c:1377:21: warning: cast from restricted __virtio32
drivers/vhost/vhost.c:1377:21: warning: cast from restricted __virtio32
drivers/vhost/vhost.c:1377:21: warning: cast from restricted __virtio32
drivers/vhost/vhost.c:1425:13: warning: cast from restricted __virtio16
drivers/vhost/vhost.c:1425:13: warning: cast from restricted __virtio16
drivers/vhost/vhost.c:1425:13: warning: cast from restricted __virtio16
drivers/vhost/vhost.c:1425:13: warning: cast from restricted __virtio16
Which something like the following hunk fixes in a similar way to yours:
diff --git a/arch/metag/include/asm/uaccess.h b/arch/metag/include/asm/uaccess.h
index 0748b0a97986..594497053748 100644
--- a/arch/metag/include/asm/uaccess.h
+++ b/arch/metag/include/asm/uaccess.h
@@ -112,13 +112,17 @@ do { \
retval = 0; \
switch (size) { \
case 1: \
- retval = __put_user_asm_b((unsigned int)x, ptr); break; \
+ retval = __put_user_asm_b((__force unsigned int)x, ptr); \
+ break; \
case 2: \
- retval = __put_user_asm_w((unsigned int)x, ptr); break; \
+ retval = __put_user_asm_w((__force unsigned int)x, ptr); \
+ break; \
case 4: \
- retval = __put_user_asm_d((unsigned int)x, ptr); break; \
+ retval = __put_user_asm_d((__force unsigned int)x, ptr); \
+ break; \
case 8: \
- retval = __put_user_asm_l((unsigned long long)x, ptr); break; \
+ retval = __put_user_asm_l((__force unsigned long long)x, ptr); \
+ break; \
default: \
__put_user_bad(); \
}
As far as I understand it, using __force on the value (as opposed to the
pointer) is safe here, in the sense of not masking any genuine defects.
Do you agree? Do you want to apply that hunk with your patch too?
Note, this change also suppresses warnings for writing a pointer to
userland due to the casts to unsigned int / unsigned long long, such as
the following (each 4 times due to 4 cases above):
kernel/signal.c:2740:25: warning: cast removes address space of expression
kernel/signal.c:2747:24: warning: cast removes address space of expression
kernel/signal.c:2760:24: warning: cast removes address space of expression
kernel/signal.c:2761:24: warning: cast removes address space of expression
kernel/signal.c:2775:24: warning: cast removes address space of expression
kernel/signal.c:2779:24: warning: cast removes address space of expression
kernel/signal.c:3202:25: warning: cast removes address space of expression
kernel/signal.c:3225:17: warning: cast removes address space of expression
kernel/futex.c:2769:16: warning: cast removes address space of expression
>
> Signed-off-by: Michael S. Tsirkin <mst@...hat.com>
> ---
> arch/metag/include/asm/uaccess.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/metag/include/asm/uaccess.h b/arch/metag/include/asm/uaccess.h
> index 0748b0a..c314b45 100644
> --- a/arch/metag/include/asm/uaccess.h
> +++ b/arch/metag/include/asm/uaccess.h
> @@ -135,7 +135,7 @@ extern long __get_user_bad(void);
> ({ \
> long __gu_err, __gu_val; \
> __get_user_size(__gu_val, (ptr), (size), __gu_err); \
> - (x) = (__typeof__(*(ptr)))__gu_val; \
> + (x) = (__force __typeof__(*(ptr)))__gu_val; \
Can you adjust the position of the \ to line up please
> __gu_err; \
> })
>
> @@ -145,7 +145,7 @@ extern long __get_user_bad(void);
> const __typeof__(*(ptr)) __user *__gu_addr = (ptr); \
> if (access_ok(VERIFY_READ, __gu_addr, size)) \
> __get_user_size(__gu_val, __gu_addr, (size), __gu_err); \
> - (x) = (__typeof__(*(ptr)))__gu_val; \
> + (x) = (__force __typeof__(*(ptr)))__gu_val; \
same here (this one causes a checkpatch error due to 80 column limit)
> __gu_err; \
> })
>
>
With those changes,
Acked-by: James Hogan <james.hogan@...tec.com>
Cheers
James
Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)
Powered by blists - more mailing lists