[PATCH repost 00/16] uaccess: fix sparse warning on get_user for bitwise types

[PATCH repost 00/16] uaccess: fix sparse warning on get_user for bitwise types

[Michael S. Tsirkin]

At Arnd's request, reposting now that 3.19-rc1 is out.
No changes from the original post, except that xtensa
and powerpc patches have been merged by maintainers.
Please review, and consider for 3.20.

At the moment, if p and x are both tagged as bitwise types,
get_user(x, p) produces a sparse warning on many architectures.
This is because *p on these architectures is loaded into long
(typically using asm), then cast back to typeof(*p).

When typeof(*p) is a bitwise type (which is uncommon), such a cast needs
__force, otherwise sparse produces a warning.

Some architectures already have the __force tag, add it
where it's missing.

Specificlly, vhost 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.

I tested this on x86 only. Since it's just adding __force, should be
trivially safe everywhere?


Michael S. Tsirkin (16):
  x86/uaccess: fix sparse errors
  alpha/uaccess: fix sparse errors
  arm64/uaccess: fix sparse errors
  avr32/uaccess: fix sparse errors
  blackfin/uaccess: fix sparse errors
  cris/uaccess: fix sparse errors
  ia64/uaccess: fix sparse errors
  m32r/uaccess: fix sparse errors
  metag/uaccess: fix sparse errors
  microblaze/uaccess: fix sparse errors
  openrisc/uaccess: fix sparse errors
  parisc/uaccess: fix sparse errors
  sh/uaccess: fix sparse errors
  sparc/uaccess: fix sparse errors
  sparc/uaccess: fix sparse errors
  m68k/uaccess: fix sparse errors

 arch/alpha/include/asm/uaccess.h      | 4 ++--
 arch/arm64/include/asm/uaccess.h      | 2 +-
 arch/avr32/include/asm/uaccess.h      | 4 ++--
 arch/blackfin/include/asm/uaccess.h   | 2 +-
 arch/cris/include/asm/uaccess.h       | 4 ++--
 arch/ia64/include/asm/uaccess.h       | 2 +-
 arch/m32r/include/asm/uaccess.h       | 4 ++--
 arch/m68k/include/asm/uaccess_mm.h    | 4 ++--
 arch/metag/include/asm/uaccess.h      | 4 ++--
 arch/microblaze/include/asm/uaccess.h | 4 ++--
 arch/openrisc/include/asm/uaccess.h   | 4 ++--
 arch/parisc/include/asm/uaccess.h     | 2 +-
 arch/sh/include/asm/uaccess.h         | 4 ++--
 arch/sparc/include/asm/uaccess_32.h   | 8 ++++----
 arch/sparc/include/asm/uaccess_64.h   | 4 ++--
 arch/x86/include/asm/uaccess.h        | 2 +-
 16 files changed, 29 insertions(+), 29 deletions(-)

-- 
MST

[PATCH repost 01/16] x86/uaccess: fix sparse errors

[Michael S. Tsirkin]

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.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/x86/include/asm/uaccess.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index 0d592e0..8ce1af8 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -179,7 +179,7 @@ __typeof__(__builtin_choose_expr(sizeof(x) > sizeof(0UL), 0ULL, 0UL))
 	asm volatile("call __get_user_%P3"				\
 		     : "=a" (__ret_gu), "=r" (__val_gu)			\
 		     : "0" (ptr), "i" (sizeof(*(ptr))));		\
-	(x) = (__typeof__(*(ptr))) __val_gu;				\
+	(x) = (__force __typeof__(*(ptr))) __val_gu;				\
 	__ret_gu;							\
 })
 
-- 
MST

[PATCH repost 02/16] alpha/uaccess: fix sparse errors

[Michael S. Tsirkin]

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.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/alpha/include/asm/uaccess.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/alpha/include/asm/uaccess.h b/arch/alpha/include/asm/uaccess.h
index 766fdfd..fbe1d1e 100644
--- a/arch/alpha/include/asm/uaccess.h
+++ b/arch/alpha/include/asm/uaccess.h
@@ -96,7 +96,7 @@ extern void __get_user_unknown(void);
 	  case 8: __get_user_64(ptr); break;			\
 	  default: __get_user_unknown(); break;			\
 	}							\
-	(x) = (__typeof__(*(ptr))) __gu_val;			\
+	(x) = (__force __typeof__(*(ptr))) __gu_val;			\
 	__gu_err;						\
 })
 
@@ -115,7 +115,7 @@ extern void __get_user_unknown(void);
 		  default: __get_user_unknown(); break;			\
 		}							\
 	}								\
-	(x) = (__typeof__(*(ptr))) __gu_val;				\
+	(x) = (__force __typeof__(*(ptr))) __gu_val;				\
 	__gu_err;							\
 })
 
-- 
MST

[PATCH repost 03/16] arm64/uaccess: fix sparse errors

[Michael S. Tsirkin]

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.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Acked-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/uaccess.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 3bf8f4e..8d66bcf 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -147,7 +147,7 @@ do {									\
 	default:							\
 		BUILD_BUG();						\
 	}								\
-	(x) = (__typeof__(*(ptr)))__gu_val;				\
+	(x) = (__force __typeof__(*(ptr)))__gu_val;				\
 } while (0)
 
 #define __get_user(x, ptr)						\
-- 
MST

[PATCH repost 03/16] arm64/uaccess: fix sparse errors

[Michael S. Tsirkin]

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.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Acked-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/uaccess.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 3bf8f4e..8d66bcf 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -147,7 +147,7 @@ do {									\
 	default:							\
 		BUILD_BUG();						\
 	}								\
-	(x) = (__typeof__(*(ptr)))__gu_val;				\
+	(x) = (__force __typeof__(*(ptr)))__gu_val;				\
 } while (0)
 
 #define __get_user(x, ptr)						\
-- 
MST

Re: [PATCH repost 03/16] arm64/uaccess: fix sparse errors

[Catalin Marinas]

On Thu, Dec 25, 2014 at 09:28:41AM +0000, 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.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Acked-by: Will Deacon <will.deacon@arm.com>

I'm not sure who's going to merge this series. In the meantime, I
cherry-picked the arm64 patch. Thanks.

-- 
Catalin

[PATCH repost 03/16] arm64/uaccess: fix sparse errors

[Catalin Marinas]

On Thu, Dec 25, 2014 at 09:28:41AM +0000, 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.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Acked-by: Will Deacon <will.deacon@arm.com>

I'm not sure who's going to merge this series. In the meantime, I
cherry-picked the arm64 patch. Thanks.

-- 
Catalin

Re: [PATCH repost 03/16] arm64/uaccess: fix sparse errors

[Catalin Marinas]

On Thu, Dec 25, 2014 at 09:28:41AM +0000, 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.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Acked-by: Will Deacon <will.deacon@arm.com>

I'm not sure who's going to merge this series. In the meantime, I
cherry-picked the arm64 patch. Thanks.

-- 
Catalin

Re: [PATCH repost 03/16] arm64/uaccess: fix sparse errors

[Will Deacon]

On Fri, Jan 23, 2015 at 03:33:16PM +0000, Catalin Marinas wrote:
> On Thu, Dec 25, 2014 at 09:28:41AM +0000, 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.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > Acked-by: Will Deacon <will.deacon@arm.com>
> 
> I'm not sure who's going to merge this series. In the meantime, I
> cherry-picked the arm64 patch. Thanks.

I think there was a pull request for the whole series that went to Arnd, so
I don't think we need to do anything on the arm64 side.

Will

[PATCH repost 03/16] arm64/uaccess: fix sparse errors

[Will Deacon]

On Fri, Jan 23, 2015 at 03:33:16PM +0000, Catalin Marinas wrote:
> On Thu, Dec 25, 2014 at 09:28:41AM +0000, 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.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > Acked-by: Will Deacon <will.deacon@arm.com>
> 
> I'm not sure who's going to merge this series. In the meantime, I
> cherry-picked the arm64 patch. Thanks.

I think there was a pull request for the whole series that went to Arnd, so
I don't think we need to do anything on the arm64 side.

Will

Re: [PATCH repost 03/16] arm64/uaccess: fix sparse errors

[Will Deacon]

On Fri, Jan 23, 2015 at 03:33:16PM +0000, Catalin Marinas wrote:
> On Thu, Dec 25, 2014 at 09:28:41AM +0000, 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.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > Acked-by: Will Deacon <will.deacon@arm.com>
> 
> I'm not sure who's going to merge this series. In the meantime, I
> cherry-picked the arm64 patch. Thanks.

I think there was a pull request for the whole series that went to Arnd, so
I don't think we need to do anything on the arm64 side.

Will

Re: [PATCH repost 03/16] arm64/uaccess: fix sparse errors

[Catalin Marinas]

On Fri, Jan 23, 2015 at 03:42:14PM +0000, Will Deacon wrote:
> On Fri, Jan 23, 2015 at 03:33:16PM +0000, Catalin Marinas wrote:
> > On Thu, Dec 25, 2014 at 09:28:41AM +0000, 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.
> > > 
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > 
> > > Acked-by: Will Deacon <will.deacon@arm.com>
> > 
> > I'm not sure who's going to merge this series. In the meantime, I
> > cherry-picked the arm64 patch. Thanks.
> 
> I think there was a pull request for the whole series that went to Arnd, so
> I don't think we need to do anything on the arm64 side.

OK. I dropped the patch.

-- 
Catalin

[PATCH repost 03/16] arm64/uaccess: fix sparse errors

[Catalin Marinas]

On Fri, Jan 23, 2015 at 03:42:14PM +0000, Will Deacon wrote:
> On Fri, Jan 23, 2015 at 03:33:16PM +0000, Catalin Marinas wrote:
> > On Thu, Dec 25, 2014 at 09:28:41AM +0000, 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.
> > > 
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > 
> > > Acked-by: Will Deacon <will.deacon@arm.com>
> > 
> > I'm not sure who's going to merge this series. In the meantime, I
> > cherry-picked the arm64 patch. Thanks.
> 
> I think there was a pull request for the whole series that went to Arnd, so
> I don't think we need to do anything on the arm64 side.

OK. I dropped the patch.

-- 
Catalin

Re: [PATCH repost 03/16] arm64/uaccess: fix sparse errors

[Catalin Marinas]

On Fri, Jan 23, 2015 at 03:42:14PM +0000, Will Deacon wrote:
> On Fri, Jan 23, 2015 at 03:33:16PM +0000, Catalin Marinas wrote:
> > On Thu, Dec 25, 2014 at 09:28:41AM +0000, 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.
> > > 
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > 
> > > Acked-by: Will Deacon <will.deacon@arm.com>
> > 
> > I'm not sure who's going to merge this series. In the meantime, I
> > cherry-picked the arm64 patch. Thanks.
> 
> I think there was a pull request for the whole series that went to Arnd, so
> I don't think we need to do anything on the arm64 side.

OK. I dropped the patch.

-- 
Catalin

[PATCH repost 04/16] avr32/uaccess: fix sparse errors

[Michael S. Tsirkin]

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.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Hans-Christian Egtvedt <egtvedt@samfundet.no>
---
 arch/avr32/include/asm/uaccess.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/avr32/include/asm/uaccess.h b/arch/avr32/include/asm/uaccess.h
index 245b2ee..ec6ce63 100644
--- a/arch/avr32/include/asm/uaccess.h
+++ b/arch/avr32/include/asm/uaccess.h
@@ -191,7 +191,7 @@ extern int __put_user_bad(void);
 	default: __gu_err = __get_user_bad(); break;			\
 	}								\
 									\
-	x = (typeof(*(ptr)))__gu_val;					\
+	x = (__force typeof(*(ptr)))__gu_val;					\
 	__gu_err;							\
 })
 
@@ -222,7 +222,7 @@ extern int __put_user_bad(void);
 	} else {							\
 		__gu_err = -EFAULT;					\
 	}								\
-	x = (typeof(*(ptr)))__gu_val;					\
+	x = (__force typeof(*(ptr)))__gu_val;					\
 	__gu_err;							\
 })
 
-- 
MST

[PATCH repost 05/16] blackfin/uaccess: fix sparse errors

[Michael S. Tsirkin]

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.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Steven Miao <realmz6@gmail.com>
---
 arch/blackfin/include/asm/uaccess.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/blackfin/include/asm/uaccess.h b/arch/blackfin/include/asm/uaccess.h
index 57701c3..6a46e7d 100644
--- a/arch/blackfin/include/asm/uaccess.h
+++ b/arch/blackfin/include/asm/uaccess.h
@@ -147,7 +147,7 @@ static inline int bad_user_access_length(void)
 		}						\
 	} else							\
 		_err = -EFAULT;					\
-	x = (typeof(*(ptr)))_val;				\
+	x = (__force typeof(*(ptr)))_val;				\
 	_err;							\
 })
 
-- 
MST

[PATCH repost 06/16] cris/uaccess: fix sparse errors

[Michael S. Tsirkin]

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.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/cris/include/asm/uaccess.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/cris/include/asm/uaccess.h b/arch/cris/include/asm/uaccess.h
index 9145408..958a350 100644
--- a/arch/cris/include/asm/uaccess.h
+++ b/arch/cris/include/asm/uaccess.h
@@ -153,7 +153,7 @@ struct __large_struct { unsigned long buf[100]; };
 ({								\
 	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;			\
 	__gu_err;						\
 })
 
@@ -163,7 +163,7 @@ struct __large_struct { unsigned long buf[100]; };
 	const __typeof__(*(ptr)) *__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;				\
 	__gu_err;							\
 })
 
-- 
MST

[PATCH repost 07/16] ia64/uaccess: fix sparse errors

[Michael S. Tsirkin]

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.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/ia64/include/asm/uaccess.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/ia64/include/asm/uaccess.h b/arch/ia64/include/asm/uaccess.h
index 103bedc..a540705 100644
--- a/arch/ia64/include/asm/uaccess.h
+++ b/arch/ia64/include/asm/uaccess.h
@@ -197,7 +197,7 @@ extern void __get_user_unknown (void);
 		      case 8: __get_user_size(__gu_val, __gu_ptr, 8, __gu_err); break;	\
 		      default: __get_user_unknown(); break;				\
 		}									\
-	(x) = (__typeof__(*(__gu_ptr))) __gu_val;					\
+	(x) = (__force __typeof__(*(__gu_ptr))) __gu_val;					\
 	__gu_err;									\
 })
 
-- 
MST

[PATCH repost 07/16] ia64/uaccess: fix sparse errors

[Michael S. Tsirkin]

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.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/ia64/include/asm/uaccess.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/ia64/include/asm/uaccess.h b/arch/ia64/include/asm/uaccess.h
index 103bedc..a540705 100644
--- a/arch/ia64/include/asm/uaccess.h
+++ b/arch/ia64/include/asm/uaccess.h
@@ -197,7 +197,7 @@ extern void __get_user_unknown (void);
 		      case 8: __get_user_size(__gu_val, __gu_ptr, 8, __gu_err); break;	\
 		      default: __get_user_unknown(); break;				\
 		}									\
-	(x) = (__typeof__(*(__gu_ptr))) __gu_val;					\
+	(x) = (__force __typeof__(*(__gu_ptr))) __gu_val;					\
 	__gu_err;									\
 })
 
-- 
MST

[PATCH repost 08/16] m32r/uaccess: fix sparse errors

[Michael S. Tsirkin]

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.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/m32r/include/asm/uaccess.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/m32r/include/asm/uaccess.h b/arch/m32r/include/asm/uaccess.h
index 84fe7ba..ee4a212 100644
--- a/arch/m32r/include/asm/uaccess.h
+++ b/arch/m32r/include/asm/uaccess.h
@@ -218,7 +218,7 @@ extern int fixup_exception(struct pt_regs *regs);
 	unsigned long __gu_val;						\
 	might_fault();							\
 	__get_user_size(__gu_val,(ptr),(size),__gu_err);		\
-	(x) = (__typeof__(*(ptr)))__gu_val;				\
+	(x) = (__force __typeof__(*(ptr)))__gu_val;				\
 	__gu_err;							\
 })
 
@@ -230,7 +230,7 @@ extern int fixup_exception(struct pt_regs *regs);
 	might_fault();							\
 	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;				\
 	__gu_err;							\
 })
 
-- 
MST

[PATCH repost 09/16] metag/uaccess: fix sparse errors

[Michael S. Tsirkin]

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.

Signed-off-by: Michael S. Tsirkin <mst@redhat.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;                     \
 	__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;                             \
 	__gu_err;                                                       \
 })
 
-- 
MST

Re: [PATCH repost 09/16] metag/uaccess: fix sparse errors

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 5653 bytes --]

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@redhat.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@imgtec.com>

Cheers
James


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH repost 09/16] metag/uaccess: fix sparse errors

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 5653 bytes --]

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@redhat.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@imgtec.com>

Cheers
James


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH repost 09/16] metag/uaccess: fix sparse errors

[Michael S. Tsirkin]

On Fri, Jan 02, 2015 at 03:41:02PM +0000, James Hogan wrote:
> 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?

what about this code:
             u16 v = 0;
             int rc = get_user(v, (__force __le16 __user *)p);

it should trigger a warning, does it?



> 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@redhat.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@imgtec.com>
> 
> Cheers
> James
> 

Re: [PATCH repost 09/16] metag/uaccess: fix sparse errors

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 6518 bytes --]

On 04/01/15 10:52, Michael S. Tsirkin wrote:
> On Fri, Jan 02, 2015 at 03:41:02PM +0000, James Hogan wrote:
>> 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?
> 
> what about this code:
>              u16 v = 0;
>              int rc = get_user(v, (__force __le16 __user *)p);
> 
> it should trigger a warning, does it?

No, but it didn't previously either, and doesn't seem to with x86_64 either.

I tried __be16 too, since I presume you're referring to a discrepancy
between the native byte order of v (u16) and the foreign byte order of
*p (__be16)?

Cheers
James

> 
> 
> 
>> 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@redhat.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@imgtec.com>
>>
>> Cheers
>> James
>>
> 
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH repost 09/16] metag/uaccess: fix sparse errors

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 6518 bytes --]

On 04/01/15 10:52, Michael S. Tsirkin wrote:
> On Fri, Jan 02, 2015 at 03:41:02PM +0000, James Hogan wrote:
>> 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?
> 
> what about this code:
>              u16 v = 0;
>              int rc = get_user(v, (__force __le16 __user *)p);
> 
> it should trigger a warning, does it?

No, but it didn't previously either, and doesn't seem to with x86_64 either.

I tried __be16 too, since I presume you're referring to a discrepancy
between the native byte order of v (u16) and the foreign byte order of
*p (__be16)?

Cheers
James

> 
> 
> 
>> 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@redhat.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@imgtec.com>
>>
>> Cheers
>> James
>>
> 
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH repost 09/16] metag/uaccess: fix sparse errors

[Michael S. Tsirkin]

On Mon, Jan 05, 2015 at 09:44:14AM +0000, James Hogan wrote:
> On 04/01/15 10:52, Michael S. Tsirkin wrote:
> > On Fri, Jan 02, 2015 at 03:41:02PM +0000, James Hogan wrote:
> >> 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?
> > 
> > what about this code:
> >              u16 v = 0;
> >              int rc = get_user(v, (__force __le16 __user *)p);
> > 
> > it should trigger a warning, does it?
> 
> No, but it didn't previously either, and doesn't seem to with x86_64 either.
> 
> I tried __be16 too, since I presume you're referring to a discrepancy
> between the native byte order of v (u16) and the foreign byte order of
> *p (__be16)?
> 
> Cheers
> James

Yes.
It certainly does for me.
Did you define __CHECK_ENDIAN__?

Documentation/sparse.txt says:

To perform endianness
checks, you may define __CHECK_ENDIAN__:

        make C=2 CF="-D__CHECK_ENDIAN__"

These checks are disabled by default as they generate a host of warnings.


Or just try with __virtio16 instead of __le16 - I think these are
enabled unconditionally.


> > 
> > 
> > 
> >> 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@redhat.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@imgtec.com>
> >>
> >> Cheers
> >> James
> >>
> > 
> > 
> 

Re: [PATCH repost 09/16] metag/uaccess: fix sparse errors

[James Hogan]

On 5 January 2015 at 13:00, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Mon, Jan 05, 2015 at 09:44:14AM +0000, James Hogan wrote:
>> On 04/01/15 10:52, Michael S. Tsirkin wrote:
>> > On Fri, Jan 02, 2015 at 03:41:02PM +0000, James Hogan wrote:
>> >> 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?
>> >
>> > what about this code:
>> >              u16 v = 0;
>> >              int rc = get_user(v, (__force __le16 __user *)p);
>> >
>> > it should trigger a warning, does it?
>>
>> No, but it didn't previously either, and doesn't seem to with x86_64 either.
>>
>> I tried __be16 too, since I presume you're referring to a discrepancy
>> between the native byte order of v (u16) and the foreign byte order of
>> *p (__be16)?
>>
>> Cheers
>> James
>
> Yes.
> It certainly does for me.
> Did you define __CHECK_ENDIAN__?
>
> Documentation/sparse.txt says:
>
> To perform endianness
> checks, you may define __CHECK_ENDIAN__:
>
>         make C=2 CF="-D__CHECK_ENDIAN__"
>
> These checks are disabled by default as they generate a host of warnings.
>
>
> Or just try with __virtio16 instead of __le16 - I think these are
> enabled unconditionally.

right, thanks,

So before your patch:
arch/metag/kernel/setup.c:153:18: warning: cast to restricted __le16
arch/metag/kernel/setup.c:153:18: warning: incorrect type in
assignment (different base types)
arch/metag/kernel/setup.c:153:18:    expected unsigned short
[unsigned] [usertype] v
arch/metag/kernel/setup.c:153:18:    got restricted __le16 <noident>

after your patch:
arch/metag/kernel/setup.c:153:18: warning: incorrect type in
assignment (different base types)
arch/metag/kernel/setup.c:153:18:    expected unsigned short
[unsigned] [usertype] v
arch/metag/kernel/setup.c:153:18:    got restricted __le16 <noident>

Changing to put_user, before my hunk above:
arch/metag/kernel/setup.c:154:18: warning: cast to restricted __le16
arch/metag/kernel/setup.c:154:18: warning: cast from restricted __le16
arch/metag/kernel/setup.c:154:18: warning: cast to restricted __le16
arch/metag/kernel/setup.c:154:18: warning: cast from restricted __le16
arch/metag/kernel/setup.c:154:18: warning: cast to restricted __le16
arch/metag/kernel/setup.c:154:18: warning: cast from restricted __le16
arch/metag/kernel/setup.c:154:18: warning: cast to restricted __le16
arch/metag/kernel/setup.c:154:18: warning: cast from restricted __le16

after my hunk above:
arch/metag/kernel/setup.c:154:18: warning: cast to restricted __le16
arch/metag/kernel/setup.c:154:18: warning: cast to restricted __le16
arch/metag/kernel/setup.c:154:18: warning: cast to restricted __le16
arch/metag/kernel/setup.c:154:18: warning: cast to restricted __le16

It seems to still serve its purpose of warning (in one way or
another), due to the implementation of put_user which does an explicit
cast to the type of *ptr:

/*
 * These are the main single-value transfer routines.  They automatically
 * use the right size if we just have the right pointer type.
 */
#define put_user(x, ptr) \
    __put_user_check((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))

Cheers
James

>
>
>> >
>> >
>> >
>> >> 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@redhat.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@imgtec.com>
>> >>
>> >> Cheers
>> >> James
>> >>
>> >
>> >
>>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-metag" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH repost 09/16] metag/uaccess: fix sparse errors

[Michael S. Tsirkin]

On Mon, Jan 05, 2015 at 02:47:19PM +0000, James Hogan wrote:
> On 5 January 2015 at 13:00, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Mon, Jan 05, 2015 at 09:44:14AM +0000, James Hogan wrote:
> >> On 04/01/15 10:52, Michael S. Tsirkin wrote:
> >> > On Fri, Jan 02, 2015 at 03:41:02PM +0000, James Hogan wrote:
> >> >> 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?
> >> >
> >> > what about this code:
> >> >              u16 v = 0;
> >> >              int rc = get_user(v, (__force __le16 __user *)p);
> >> >
> >> > it should trigger a warning, does it?
> >>
> >> No, but it didn't previously either, and doesn't seem to with x86_64 either.
> >>
> >> I tried __be16 too, since I presume you're referring to a discrepancy
> >> between the native byte order of v (u16) and the foreign byte order of
> >> *p (__be16)?
> >>
> >> Cheers
> >> James
> >
> > Yes.
> > It certainly does for me.
> > Did you define __CHECK_ENDIAN__?
> >
> > Documentation/sparse.txt says:
> >
> > To perform endianness
> > checks, you may define __CHECK_ENDIAN__:
> >
> >         make C=2 CF="-D__CHECK_ENDIAN__"
> >
> > These checks are disabled by default as they generate a host of warnings.
> >
> >
> > Or just try with __virtio16 instead of __le16 - I think these are
> > enabled unconditionally.
> 
> right, thanks,
> 
> So before your patch:
> arch/metag/kernel/setup.c:153:18: warning: cast to restricted __le16
> arch/metag/kernel/setup.c:153:18: warning: incorrect type in
> assignment (different base types)
> arch/metag/kernel/setup.c:153:18:    expected unsigned short
> [unsigned] [usertype] v
> arch/metag/kernel/setup.c:153:18:    got restricted __le16 <noident>
> 
> after your patch:
> arch/metag/kernel/setup.c:153:18: warning: incorrect type in
> assignment (different base types)
> arch/metag/kernel/setup.c:153:18:    expected unsigned short
> [unsigned] [usertype] v
> arch/metag/kernel/setup.c:153:18:    got restricted __le16 <noident>
> 
> Changing to put_user, before my hunk above:
> arch/metag/kernel/setup.c:154:18: warning: cast to restricted __le16
> arch/metag/kernel/setup.c:154:18: warning: cast from restricted __le16
> arch/metag/kernel/setup.c:154:18: warning: cast to restricted __le16
> arch/metag/kernel/setup.c:154:18: warning: cast from restricted __le16
> arch/metag/kernel/setup.c:154:18: warning: cast to restricted __le16
> arch/metag/kernel/setup.c:154:18: warning: cast from restricted __le16
> arch/metag/kernel/setup.c:154:18: warning: cast to restricted __le16
> arch/metag/kernel/setup.c:154:18: warning: cast from restricted __le16
> 
> after my hunk above:
> arch/metag/kernel/setup.c:154:18: warning: cast to restricted __le16
> arch/metag/kernel/setup.c:154:18: warning: cast to restricted __le16
> arch/metag/kernel/setup.c:154:18: warning: cast to restricted __le16
> arch/metag/kernel/setup.c:154:18: warning: cast to restricted __le16
> 
> It seems to still serve its purpose of warning (in one way or
> another), due to the implementation of put_user which does an explicit
> cast to the type of *ptr:
> 
> /*
>  * These are the main single-value transfer routines.  They automatically
>  * use the right size if we just have the right pointer type.
>  */
> #define put_user(x, ptr) \
>     __put_user_check((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
> 
> Cheers
> James


Right. I would probably stick an explicit
	(x) = (ptr)
there.

Let me send a patch.


> >
> >
> >> >
> >> >
> >> >
> >> >> 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@redhat.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@imgtec.com>
> >> >>
> >> >> Cheers
> >> >> James
> >> >>
> >> >
> >> >
> >>
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-metag" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH repost 10/16] microblaze/uaccess: fix sparse errors

[Michael S. Tsirkin]

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.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/microblaze/include/asm/uaccess.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/microblaze/include/asm/uaccess.h b/arch/microblaze/include/asm/uaccess.h
index 59a89a6..57acf04 100644
--- a/arch/microblaze/include/asm/uaccess.h
+++ b/arch/microblaze/include/asm/uaccess.h
@@ -220,7 +220,7 @@ extern long __user_bad(void);
 	} else {							\
 		__gu_err = -EFAULT;					\
 	}								\
-	x = (typeof(*(ptr)))__gu_val;					\
+	x = (__force typeof(*(ptr)))__gu_val;					\
 	__gu_err;							\
 })
 
@@ -242,7 +242,7 @@ extern long __user_bad(void);
 	default:							\
 		/* __gu_val = 0; __gu_err = -EINVAL;*/ __gu_err = __user_bad();\
 	}								\
-	x = (__typeof__(*(ptr))) __gu_val;				\
+	x = (__force __typeof__(*(ptr))) __gu_val;				\
 	__gu_err;							\
 })
 
-- 
MST

[PATCH repost 11/16] openrisc/uaccess: fix sparse errors

[Michael S. Tsirkin]

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.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/openrisc/include/asm/uaccess.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/openrisc/include/asm/uaccess.h b/arch/openrisc/include/asm/uaccess.h
index ab2e7a1..7be8d35 100644
--- a/arch/openrisc/include/asm/uaccess.h
+++ b/arch/openrisc/include/asm/uaccess.h
@@ -192,7 +192,7 @@ struct __large_struct {
 ({								\
 	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;			\
 	__gu_err;						\
 })
 
@@ -202,7 +202,7 @@ struct __large_struct {
 	const __typeof__(*(ptr)) * __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;				\
 	__gu_err;							\
 })
 
-- 
MST

[PATCH repost 11/16] openrisc/uaccess: fix sparse errors

[Michael S. Tsirkin]

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.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/openrisc/include/asm/uaccess.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/openrisc/include/asm/uaccess.h b/arch/openrisc/include/asm/uaccess.h
index ab2e7a1..7be8d35 100644
--- a/arch/openrisc/include/asm/uaccess.h
+++ b/arch/openrisc/include/asm/uaccess.h
@@ -192,7 +192,7 @@ struct __large_struct {
 ({								\
 	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;			\
 	__gu_err;						\
 })
 
@@ -202,7 +202,7 @@ struct __large_struct {
 	const __typeof__(*(ptr)) * __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;				\
 	__gu_err;							\
 })
 
-- 
MST

[PATCH repost 12/16] parisc/uaccess: fix sparse errors

[Michael S. Tsirkin]

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.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/parisc/include/asm/uaccess.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/parisc/include/asm/uaccess.h b/arch/parisc/include/asm/uaccess.h
index a5cb070..3a20da6 100644
--- a/arch/parisc/include/asm/uaccess.h
+++ b/arch/parisc/include/asm/uaccess.h
@@ -104,7 +104,7 @@ struct exception_data {
 	    }                                           \
 	}                                               \
 							\
-	(x) = (__typeof__(*(ptr))) __gu_val;            \
+	(x) = (__force __typeof__(*(ptr))) __gu_val;            \
 	__gu_err;                                       \
 })
 
-- 
MST

Re: [PATCH repost 12/16] parisc/uaccess: fix sparse errors

[Helge Deller]

Hi Michael,

On 12/25/2014 10:29 AM, Michael S. Tsirkin wrote:
> virtio wants to read bitwise types from userspace using get_user.  At the

I don't know the virtio code much yet, but does it makes sense to read bitwise types?
Will virtio then get possible troubles because of endianess correct as well?

Do you have a code example, or the sparse error message ?

Helge

> moment this triggers sparse errors, since the value is passed through an
> integer.
>
> Fix that up using __force.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>   arch/parisc/include/asm/uaccess.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/parisc/include/asm/uaccess.h b/arch/parisc/include/asm/uaccess.h
> index a5cb070..3a20da6 100644
> --- a/arch/parisc/include/asm/uaccess.h
> +++ b/arch/parisc/include/asm/uaccess.h
> @@ -104,7 +104,7 @@ struct exception_data {
>   	    }                                           \
>   	}                                               \
>   							\
> -	(x) = (__typeof__(*(ptr))) __gu_val;            \
> +	(x) = (__force __typeof__(*(ptr))) __gu_val;            \
>   	__gu_err;                                       \
>   })
>
>

Re: [PATCH repost 12/16] parisc/uaccess: fix sparse errors

[Michael S. Tsirkin]

On Thu, Dec 25, 2014 at 11:37:45PM +0100, Helge Deller wrote:
> Hi Michael,
> 
> On 12/25/2014 10:29 AM, Michael S. Tsirkin wrote:
> >virtio wants to read bitwise types from userspace using get_user.  At the
> 
> I don't know the virtio code much yet, but does it makes sense to read bitwise types?
> Will virtio then get possible troubles because of endianess correct as well?

There's no conversion: we are reading from __virtio16 __user *
pointer into __virtio16 v value.

> Do you have a code example, or the sparse error message ?
> 
> Helge

Sure. the code is upstream now.
The warning is below.

sparse warnings: (new ones prefixed by >>)

>> drivers/vhost/vringh.c:554:18: sparse: cast to restricted __virtio16

vim +554 drivers/vhost/vringh.c

   538                                                           __virtio16 *p, u16 val))
   539  {
   540          if (!vrh->event_indices) {
   541                  /* Old-school; update flags. */
   542                  if (putu16(vrh, &vrh->vring.used->flags,
   543                             VRING_USED_F_NO_NOTIFY)) {
   544                          vringh_bad("Setting used flags %p",
   545                                     &vrh->vring.used->flags);
   546                  }
   547          }
   548  }
   549
   550  /* Userspace access helpers: in this case, addresses are really userspace. */
   551  static inline int getu16_user(const struct vringh *vrh, u16 *val, const __virtio16 *p)
   552  {
   553          __virtio16 v = 0;
 > 554          int rc = get_user(v, (__force __virtio16 __user *)p);
   555          *val = vringh16_to_cpu(vrh, v);
   556          return rc;
   557  }
   558
   559  static inline int putu16_user(const struct vringh *vrh, __virtio16 *p, u16 val)
   560  {
   561          __virtio16 v = cpu_to_vringh16(vrh, val);
   562          return put_user(v, (__force __virtio16 __user *)p);



> 
> >moment this triggers sparse errors, since the value is passed through an
> >integer.
> >
> >Fix that up using __force.
> >
> >Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >---
> >  arch/parisc/include/asm/uaccess.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/arch/parisc/include/asm/uaccess.h b/arch/parisc/include/asm/uaccess.h
> >index a5cb070..3a20da6 100644
> >--- a/arch/parisc/include/asm/uaccess.h
> >+++ b/arch/parisc/include/asm/uaccess.h
> >@@ -104,7 +104,7 @@ struct exception_data {
> >  	    }                                           \
> >  	}                                               \
> >  							\
> >-	(x) = (__typeof__(*(ptr))) __gu_val;            \
> >+	(x) = (__force __typeof__(*(ptr))) __gu_val;            \
> >  	__gu_err;                                       \
> >  })
> >
> >

Re: [PATCH repost 12/16] parisc/uaccess: fix sparse errors

[James Bottomley]

On Sat, 2014-12-27 at 18:14 +0200, Michael S. Tsirkin wrote:
> On Thu, Dec 25, 2014 at 11:37:45PM +0100, Helge Deller wrote:
> > Hi Michael,
> > 
> > On 12/25/2014 10:29 AM, Michael S. Tsirkin wrote:
> > >virtio wants to read bitwise types from userspace using get_user.  At the
> > 
> > I don't know the virtio code much yet, but does it makes sense to read bitwise types?
> > Will virtio then get possible troubles because of endianess correct as well?
> 
> There's no conversion: we are reading from __virtio16 __user *
> pointer into __virtio16 v value.
> 
> > Do you have a code example, or the sparse error message ?
> > 
> > Helge
> 
> Sure. the code is upstream now.
> The warning is below.
> 
> sparse warnings: (new ones prefixed by >>)
> 
> >> drivers/vhost/vringh.c:554:18: sparse: cast to restricted __virtio16
> 
> vim +554 drivers/vhost/vringh.c
> 
>    538                                                           __virtio16 *p, u16 val))
>    539  {
>    540          if (!vrh->event_indices) {
>    541                  /* Old-school; update flags. */
>    542                  if (putu16(vrh, &vrh->vring.used->flags,
>    543                             VRING_USED_F_NO_NOTIFY)) {
>    544                          vringh_bad("Setting used flags %p",
>    545                                     &vrh->vring.used->flags);
>    546                  }
>    547          }
>    548  }
>    549
>    550  /* Userspace access helpers: in this case, addresses are really userspace. */
>    551  static inline int getu16_user(const struct vringh *vrh, u16 *val, const __virtio16 *p)
>    552  {
>    553          __virtio16 v = 0;
>  > 554          int rc = get_user(v, (__force __virtio16 __user *)p);
>    555          *val = vringh16_to_cpu(vrh, v);
>    556          return rc;
>    557  }
>    558
>    559  static inline int putu16_user(const struct vringh *vrh, __virtio16 *p, u16 val)
>    560  {
>    561          __virtio16 v = cpu_to_vringh16(vrh, val);
>    562          return put_user(v, (__force __virtio16 __user *)p);

OK, parisc developers still being dense, but this does look like an
abuse of the bitwise type.  bitwise is supposed to be consumed by endian
specific accessors.  get/put_user have no endian tags because they
really can't do this ... the potential for width mismatch between the
user and kernel address spaces could cause havoc if people get this
wrong, so the warning looks correct to me.

If we take your proposed patch we lose the type checking on all
accessors because of the __force.  Why not, instead, alter your code to
tell the kernel you know what you're doing:

        __u16 v = 0;
        int rc = get_user(v, (__force __u16 __user *)p);
        *val = vringh16_to_cpu(vrh, (__force __virtio16)v);
        return rc;

That way the accessors still warn if anyone else tries this but your
warning is gone and the code basically says you knew the u16 was really
an endianness specific virtio quantity?

James

Re: [PATCH repost 12/16] parisc/uaccess: fix sparse errors

[Michael S. Tsirkin]

On Wed, Dec 31, 2014 at 09:17:20AM -0800, James Bottomley wrote:
> On Sat, 2014-12-27 at 18:14 +0200, Michael S. Tsirkin wrote:
> > On Thu, Dec 25, 2014 at 11:37:45PM +0100, Helge Deller wrote:
> > > Hi Michael,
> > > 
> > > On 12/25/2014 10:29 AM, Michael S. Tsirkin wrote:
> > > >virtio wants to read bitwise types from userspace using get_user.  At the
> > > 
> > > I don't know the virtio code much yet, but does it makes sense to read bitwise types?
> > > Will virtio then get possible troubles because of endianess correct as well?
> > 
> > There's no conversion: we are reading from __virtio16 __user *
> > pointer into __virtio16 v value.
> > 
> > > Do you have a code example, or the sparse error message ?
> > > 
> > > Helge
> > 
> > Sure. the code is upstream now.
> > The warning is below.
> > 
> > sparse warnings: (new ones prefixed by >>)
> > 
> > >> drivers/vhost/vringh.c:554:18: sparse: cast to restricted __virtio16
> > 
> > vim +554 drivers/vhost/vringh.c
> > 
> >    538                                                           __virtio16 *p, u16 val))
> >    539  {
> >    540          if (!vrh->event_indices) {
> >    541                  /* Old-school; update flags. */
> >    542                  if (putu16(vrh, &vrh->vring.used->flags,
> >    543                             VRING_USED_F_NO_NOTIFY)) {
> >    544                          vringh_bad("Setting used flags %p",
> >    545                                     &vrh->vring.used->flags);
> >    546                  }
> >    547          }
> >    548  }
> >    549
> >    550  /* Userspace access helpers: in this case, addresses are really userspace. */
> >    551  static inline int getu16_user(const struct vringh *vrh, u16 *val, const __virtio16 *p)
> >    552  {
> >    553          __virtio16 v = 0;
> >  > 554          int rc = get_user(v, (__force __virtio16 __user *)p);
> >    555          *val = vringh16_to_cpu(vrh, v);
> >    556          return rc;
> >    557  }
> >    558
> >    559  static inline int putu16_user(const struct vringh *vrh, __virtio16 *p, u16 val)
> >    560  {
> >    561          __virtio16 v = cpu_to_vringh16(vrh, val);
> >    562          return put_user(v, (__force __virtio16 __user *)p);
> 
> OK, parisc developers still being dense, but this does look like an
> abuse of the bitwise type.

To give you another example:

	__le16 __user *p;
	__le16 foo;
	int rc = get_user(v, p);

really should be fine, ATM this gives a warning.


>  bitwise is supposed to be consumed by endian
> specific accessors.

Surely, assignment is OK too? get_user is exactly that.

vringh16_to_cpu is an endian specific accessor.
Look up it's definition please. The reason for that __force is
because we are adding __user.
It's a decision Rusty made to reduce code duplication:
we have some code that handles both kernel and userspace pointers.

>  get/put_user have no endian tags because they
> really can't do this ... the potential for width mismatch between the
> user and kernel address spaces could cause havoc if people get this
> wrong, so the warning looks correct to me.


I'm sorry I don't understand.

Why is

	access_ok
	__get_user

safer than

	get_user

?

It does not trigger the warning, because
__get_user does not have the cast to long internally.

Also, on some architectures get_user does not cast to long
internally so there's no warning.

> If we take your proposed patch we lose the type checking on all
> accessors because of the __force.


Did you try?  In my testing, this is not at all true.

For example with my patch:


             u16 v = 0;
             int rc = get_user(v, (__force __virtio16 __user *)p);

correctly triggers a warning.



>  Why not, instead, alter your code to
> tell the kernel you know what you're doing:
> 
>         __u16 v = 0;
>         int rc = get_user(v, (__force __u16 __user *)p);
>         *val = vringh16_to_cpu(vrh, (__force __virtio16)v);
>         return rc;
> 
> That way the accessors still warn if anyone else tries this

Hmm I don't understand, sorry. Tries what?
Can you please show me an invalid use of get_user that
produces a warning currently but won't with my patch?

> but your
> warning is gone and the code basically says you knew the u16 was really
> an endianness specific virtio quantity?
> 
> James
> 


(__force __virtio16 __user *)
tells get_user exactly that pointer is to type __virtio16.
It does not get any more explicit.

What you are proposing is really discarding type
information by a bunch of __force calls.

I am very reluctant to do this.
In fact, because of the static checking I added,
conversion to virtio 1.0 went so smoothly:
most drivers worked right away after the conversion.
I'm very sure without static checking, or with
__force thrown around liberally, I would have



vringh specifically has one __force cast anyway because
it's mixing userspace and kernel pointers.

But, I also have an out of tree patch that use structures
like this:

	struct foo {
		__virtio16 bar;
	};


Now with my patches I can do:

       __virtio16 v = 0;
	struct foo __user *p;
       int rc = get_user(v, &p->bar);


-- 
MST

Re: [PATCH repost 12/16] parisc/uaccess: fix sparse errors

[James Bottomley]

On Wed, 2014-12-31 at 20:38 +0200, Michael S. Tsirkin wrote:
> On Wed, Dec 31, 2014 at 09:17:20AM -0800, James Bottomley wrote:
[...]
> > OK, parisc developers still being dense, but this does look like an
> > abuse of the bitwise type.
> 
> To give you another example:
> 
> 	__le16 __user *p;
> 	__le16 foo;
> 	int rc = get_user(v, p);
> 
> really should be fine, ATM this gives a warning.

OK, I think I've figured it out.  You're saying that casting __gu_val to
a bitwise annotated type is an automatic sparse failure because it has
to be a long in our assembly code to receive the load/store as a
register.  However, this is required for sparse to do the correct lvalue
type = rvalue type check in the assignment to x.  We were all thinking
the __force just killed these sparse type checks.

In that case, I think parisc is fine with this.

James

[PATCH repost 13/16] sh/uaccess: fix sparse errors

[Michael S. Tsirkin]

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.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/sh/include/asm/uaccess.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/sh/include/asm/uaccess.h b/arch/sh/include/asm/uaccess.h
index 9486376..484b1f9 100644
--- a/arch/sh/include/asm/uaccess.h
+++ b/arch/sh/include/asm/uaccess.h
@@ -60,7 +60,7 @@ struct __large_struct { unsigned long buf[100]; };
 	const __typeof__(*(ptr)) __user *__gu_addr = (ptr);	\
 	__chk_user_ptr(ptr);					\
 	__get_user_size(__gu_val, __gu_addr, (size), __gu_err);	\
-	(x) = (__typeof__(*(ptr)))__gu_val;			\
+	(x) = (__force __typeof__(*(ptr)))__gu_val;			\
 	__gu_err;						\
 })
 
@@ -71,7 +71,7 @@ struct __large_struct { unsigned long buf[100]; };
 	const __typeof__(*(ptr)) *__gu_addr = (ptr);			\
 	if (likely(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;				\
 	__gu_err;							\
 })
 
-- 
MST

[PATCH repost 13/16] sh/uaccess: fix sparse errors

[Michael S. Tsirkin]

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.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/sh/include/asm/uaccess.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/sh/include/asm/uaccess.h b/arch/sh/include/asm/uaccess.h
index 9486376..484b1f9 100644
--- a/arch/sh/include/asm/uaccess.h
+++ b/arch/sh/include/asm/uaccess.h
@@ -60,7 +60,7 @@ struct __large_struct { unsigned long buf[100]; };
 	const __typeof__(*(ptr)) __user *__gu_addr = (ptr);	\
 	__chk_user_ptr(ptr);					\
 	__get_user_size(__gu_val, __gu_addr, (size), __gu_err);	\
-	(x) = (__typeof__(*(ptr)))__gu_val;			\
+	(x) = (__force __typeof__(*(ptr)))__gu_val;			\
 	__gu_err;						\
 })
 
@@ -71,7 +71,7 @@ struct __large_struct { unsigned long buf[100]; };
 	const __typeof__(*(ptr)) *__gu_addr = (ptr);			\
 	if (likely(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;				\
 	__gu_err;							\
 })
 
-- 
MST

[PATCH repost 14/16] sparc/uaccess: fix sparse errors

[Michael S. Tsirkin]

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.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/sparc/include/asm/uaccess_32.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/sparc/include/asm/uaccess_32.h b/arch/sparc/include/asm/uaccess_32.h
index 9634d08..8b571a0 100644
--- a/arch/sparc/include/asm/uaccess_32.h
+++ b/arch/sparc/include/asm/uaccess_32.h
@@ -164,7 +164,7 @@ case 2: __get_user_asm(__gu_val,uh,addr,__gu_ret); break; \
 case 4: __get_user_asm(__gu_val,,addr,__gu_ret); break; \
 case 8: __get_user_asm(__gu_val,d,addr,__gu_ret); break; \
 default: __gu_val = 0; __gu_ret = __get_user_bad(); break; \
-} } else { __gu_val = 0; __gu_ret = -EFAULT; } x = (type) __gu_val; __gu_ret; })
+} } else { __gu_val = 0; __gu_ret = -EFAULT; } x = (__force type) __gu_val; __gu_ret; })
 
 #define __get_user_check_ret(x,addr,size,type,retval) ({ \
 register unsigned long __gu_val __asm__ ("l1"); \
@@ -175,7 +175,7 @@ case 2: __get_user_asm_ret(__gu_val,uh,addr,retval); break; \
 case 4: __get_user_asm_ret(__gu_val,,addr,retval); break; \
 case 8: __get_user_asm_ret(__gu_val,d,addr,retval); break; \
 default: if (__get_user_bad()) return retval; \
-} x = (type) __gu_val; } else return retval; })
+} x = (__force type) __gu_val; } else return retval; })
 
 #define __get_user_nocheck(x,addr,size,type) ({ \
 register int __gu_ret; \
@@ -186,7 +186,7 @@ case 2: __get_user_asm(__gu_val,uh,addr,__gu_ret); break; \
 case 4: __get_user_asm(__gu_val,,addr,__gu_ret); break; \
 case 8: __get_user_asm(__gu_val,d,addr,__gu_ret); break; \
 default: __gu_val = 0; __gu_ret = __get_user_bad(); break; \
-} x = (type) __gu_val; __gu_ret; })
+} x = (__force type) __gu_val; __gu_ret; })
 
 #define __get_user_nocheck_ret(x,addr,size,type,retval) ({ \
 register unsigned long __gu_val __asm__ ("l1"); \
@@ -196,7 +196,7 @@ case 2: __get_user_asm_ret(__gu_val,uh,addr,retval); break; \
 case 4: __get_user_asm_ret(__gu_val,,addr,retval); break; \
 case 8: __get_user_asm_ret(__gu_val,d,addr,retval); break; \
 default: if (__get_user_bad()) return retval; \
-} x = (type) __gu_val; })
+} x = (__force type) __gu_val; })
 
 #define __get_user_asm(x,size,addr,ret)					\
 __asm__ __volatile__(							\
-- 
MST

[PATCH repost 14/16] sparc/uaccess: fix sparse errors

[Michael S. Tsirkin]

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.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/sparc/include/asm/uaccess_32.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/sparc/include/asm/uaccess_32.h b/arch/sparc/include/asm/uaccess_32.h
index 9634d08..8b571a0 100644
--- a/arch/sparc/include/asm/uaccess_32.h
+++ b/arch/sparc/include/asm/uaccess_32.h
@@ -164,7 +164,7 @@ case 2: __get_user_asm(__gu_val,uh,addr,__gu_ret); break; \
 case 4: __get_user_asm(__gu_val,,addr,__gu_ret); break; \
 case 8: __get_user_asm(__gu_val,d,addr,__gu_ret); break; \
 default: __gu_val = 0; __gu_ret = __get_user_bad(); break; \
-} } else { __gu_val = 0; __gu_ret = -EFAULT; } x = (type) __gu_val; __gu_ret; })
+} } else { __gu_val = 0; __gu_ret = -EFAULT; } x = (__force type) __gu_val; __gu_ret; })
 
 #define __get_user_check_ret(x,addr,size,type,retval) ({ \
 register unsigned long __gu_val __asm__ ("l1"); \
@@ -175,7 +175,7 @@ case 2: __get_user_asm_ret(__gu_val,uh,addr,retval); break; \
 case 4: __get_user_asm_ret(__gu_val,,addr,retval); break; \
 case 8: __get_user_asm_ret(__gu_val,d,addr,retval); break; \
 default: if (__get_user_bad()) return retval; \
-} x = (type) __gu_val; } else return retval; })
+} x = (__force type) __gu_val; } else return retval; })
 
 #define __get_user_nocheck(x,addr,size,type) ({ \
 register int __gu_ret; \
@@ -186,7 +186,7 @@ case 2: __get_user_asm(__gu_val,uh,addr,__gu_ret); break; \
 case 4: __get_user_asm(__gu_val,,addr,__gu_ret); break; \
 case 8: __get_user_asm(__gu_val,d,addr,__gu_ret); break; \
 default: __gu_val = 0; __gu_ret = __get_user_bad(); break; \
-} x = (type) __gu_val; __gu_ret; })
+} x = (__force type) __gu_val; __gu_ret; })
 
 #define __get_user_nocheck_ret(x,addr,size,type,retval) ({ \
 register unsigned long __gu_val __asm__ ("l1"); \
@@ -196,7 +196,7 @@ case 2: __get_user_asm_ret(__gu_val,uh,addr,retval); break; \
 case 4: __get_user_asm_ret(__gu_val,,addr,retval); break; \
 case 8: __get_user_asm_ret(__gu_val,d,addr,retval); break; \
 default: if (__get_user_bad()) return retval; \
-} x = (type) __gu_val; })
+} x = (__force type) __gu_val; })
 
 #define __get_user_asm(x,size,addr,ret)					\
 __asm__ __volatile__(							\
-- 
MST

[PATCH repost 15/16] sparc/uaccess: fix sparse errors

[Michael S. Tsirkin]

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.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/sparc/include/asm/uaccess_64.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/sparc/include/asm/uaccess_64.h b/arch/sparc/include/asm/uaccess_64.h
index c990a5e..b80866d 100644
--- a/arch/sparc/include/asm/uaccess_64.h
+++ b/arch/sparc/include/asm/uaccess_64.h
@@ -145,7 +145,7 @@ case 2: __get_user_asm(__gu_val,uh,addr,__gu_ret); break; \
 case 4: __get_user_asm(__gu_val,uw,addr,__gu_ret); break; \
 case 8: __get_user_asm(__gu_val,x,addr,__gu_ret); break; \
 default: __gu_val = 0; __gu_ret = __get_user_bad(); break; \
-} data = (type) __gu_val; __gu_ret; })
+} data = (__force type) __gu_val; __gu_ret; })
 
 #define __get_user_nocheck_ret(data,addr,size,type,retval) ({ \
 register unsigned long __gu_val __asm__ ("l1"); \
@@ -155,7 +155,7 @@ case 2: __get_user_asm_ret(__gu_val,uh,addr,retval); break; \
 case 4: __get_user_asm_ret(__gu_val,uw,addr,retval); break; \
 case 8: __get_user_asm_ret(__gu_val,x,addr,retval); break; \
 default: if (__get_user_bad()) return retval; \
-} data = (type) __gu_val; })
+} data = (__force type) __gu_val; })
 
 #define __get_user_asm(x,size,addr,ret)					\
 __asm__ __volatile__(							\
-- 
MST

[PATCH repost 15/16] sparc/uaccess: fix sparse errors

[Michael S. Tsirkin]

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.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/sparc/include/asm/uaccess_64.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/sparc/include/asm/uaccess_64.h b/arch/sparc/include/asm/uaccess_64.h
index c990a5e..b80866d 100644
--- a/arch/sparc/include/asm/uaccess_64.h
+++ b/arch/sparc/include/asm/uaccess_64.h
@@ -145,7 +145,7 @@ case 2: __get_user_asm(__gu_val,uh,addr,__gu_ret); break; \
 case 4: __get_user_asm(__gu_val,uw,addr,__gu_ret); break; \
 case 8: __get_user_asm(__gu_val,x,addr,__gu_ret); break; \
 default: __gu_val = 0; __gu_ret = __get_user_bad(); break; \
-} data = (type) __gu_val; __gu_ret; })
+} data = (__force type) __gu_val; __gu_ret; })
 
 #define __get_user_nocheck_ret(data,addr,size,type,retval) ({ \
 register unsigned long __gu_val __asm__ ("l1"); \
@@ -155,7 +155,7 @@ case 2: __get_user_asm_ret(__gu_val,uh,addr,retval); break; \
 case 4: __get_user_asm_ret(__gu_val,uw,addr,retval); break; \
 case 8: __get_user_asm_ret(__gu_val,x,addr,retval); break; \
 default: if (__get_user_bad()) return retval; \
-} data = (type) __gu_val; })
+} data = (__force type) __gu_val; })
 
 #define __get_user_asm(x,size,addr,ret)					\
 __asm__ __volatile__(							\
-- 
MST

Re: [PATCH repost 15/16] sparc/uaccess: fix sparse errors

[David Miller]

From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Thu, 25 Dec 2014 11:30:22 +0200

> 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.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Patches #14 and #15 have identical commit message header lines, you
need to say "sparc32: " or "sparc64: " so that shortlog scanners can
distinguish them.

Thanks.

Re: [PATCH repost 15/16] sparc/uaccess: fix sparse errors

[David Miller]

From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Thu, 25 Dec 2014 11:30:22 +0200

> 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.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Patches #14 and #15 have identical commit message header lines, you
need to say "sparc32: " or "sparc64: " so that shortlog scanners can
distinguish them.

Thanks.

Re: [PATCH repost 15/16] sparc/uaccess: fix sparse errors

[Michael S. Tsirkin]

On Thu, Dec 25, 2014 at 06:39:30PM -0500, David Miller wrote:
> From: "Michael S. Tsirkin" <mst@redhat.com>
> Date: Thu, 25 Dec 2014 11:30:22 +0200
> 
> > 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.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Patches #14 and #15 have identical commit message header lines, you
> need to say "sparc32: " or "sparc64: " so that shortlog scanners can
> distinguish them.
> 
> Thanks.

Got it.
Arnd, you are queueing this - could you tweak the subjects please?
Or would you prefer me to repost with tweaked subjects?

-- 
MST

Re: [PATCH repost 15/16] sparc/uaccess: fix sparse errors

[Michael S. Tsirkin]

On Thu, Dec 25, 2014 at 06:39:30PM -0500, David Miller wrote:
> From: "Michael S. Tsirkin" <mst@redhat.com>
> Date: Thu, 25 Dec 2014 11:30:22 +0200
> 
> > 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.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Patches #14 and #15 have identical commit message header lines, you
> need to say "sparc32: " or "sparc64: " so that shortlog scanners can
> distinguish them.
> 
> Thanks.

Got it.
Arnd, you are queueing this - could you tweak the subjects please?
Or would you prefer me to repost with tweaked subjects?

-- 
MST

[PATCH repost 16/16] m68k/uaccess: fix sparse errors

[Michael S. Tsirkin]

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.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/m68k/include/asm/uaccess_mm.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/m68k/include/asm/uaccess_mm.h b/arch/m68k/include/asm/uaccess_mm.h
index 15901db..c31f53f 100644
--- a/arch/m68k/include/asm/uaccess_mm.h
+++ b/arch/m68k/include/asm/uaccess_mm.h
@@ -146,7 +146,7 @@ asm volatile ("\n"					\
 		"	.previous"				\
 		: "+d" (res), "=&" #reg (__gu_val)		\
 		: "m" (*(ptr)), "i" (err));			\
-	(x) = (typeof(*(ptr)))(unsigned long)__gu_val;		\
+	(x) = (__force typeof(*(ptr)))(__force unsigned long)__gu_val;		\
 })
 
 #define __get_user(x, ptr)						\
@@ -188,7 +188,7 @@ asm volatile ("\n"					\
 			  "+a" (__gu_ptr)				\
 			: "i" (-EFAULT)					\
 			: "memory");					\
-		(x) = (typeof(*(ptr)))__gu_val;				\
+		(x) = (__force typeof(*(ptr)))__gu_val;				\
 		break;							\
 	    }	*/							\
 	default:							\
-- 
MST