linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Chris Metcalf <cmetcalf@ezchip.com>
Cc: linux-kernel@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>,
	linux-arch@vger.kernel.org
Subject: Re: [PATCH v2 22/40] tile: fix put_user sparse errors
Date: Tue, 13 Jan 2015 11:35:35 +0200	[thread overview]
Message-ID: <20150113093535.GA4434@redhat.com> (raw)
In-Reply-To: <54B44326.6010501@ezchip.com>

On Mon, Jan 12, 2015 at 04:56:54PM -0500, Chris Metcalf wrote:
> Nack for this patch as-is.
> 
> On 1/6/2015 10:44 AM, Michael S. Tsirkin wrote:
> >virtio wants to write bitwise types to userspace using put_user.
> >At the moment this triggers sparse errors, since the value is passed
> >through an integer.
> >
> >For example:
> >
> >	__le32 __user *p;
> >	__le32 x;
> >	put_user(x, p);
> >
> >is safe, but currently triggers a sparse warning on tile.
> >
> >The reason has to do with this code:
> >	__typeof((x)-(x))
> >which seems to be a way to force check for an integer type.
> 
> No, it's purely a way to avoid
> 
>   warning: cast from pointer to integer of different size
> 
> at every place we invoke put_user() with a pointer - which is
> in fact pretty frequent throughout the kernel.
>
>  The idiom of
> casting to the difference of the type converts it to a type
> of the same size as the input (whether integral or pointer),
> but guaranteed to be an integral type.  Then from there it's safe
> to cast it on to a u64 without generating a warning.

Thanks for the comments.  OK, I see, though I wonder why didn't
kbuild notify me about new warnings. Doesn't it build tile?

So if you want to merge your patch, please let me know.

But I think the fix can be much simpler: unsigned long has the same property
without any of the complexity, or problems with sparse.  So how about this:

--->

tile: fix put_user sparse errors

virtio wants to write bitwise types to userspace using put_user.
At the moment this triggers sparse errors, since the value is passed
through an integer.

For example:

__le32 __user *p;
__le32 x;
put_user(x, p);

is safe, but currently triggers a sparse warning on tile.

The reason has to do with this code:
__typeof((x)-(x))
which is a way to avoid cast from pointer to integer of different size
warnings.

Fix that up using __force unsigned long cast instead:
this is similar to what many other architectures do.

Note: this does not suppress any useful sparse checks since
the original merely casted x to typeof(x-x).

Tile currently does not trigger sparse warnings when get_user
causes an illegal assignment across bitwise types.
This patch does not attempt to fix this.

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

---->

diff --git a/arch/tile/include/asm/uaccess.h b/arch/tile/include/asm/uaccess.h
index b6cde32..9fcbe6f 100644
--- a/arch/tile/include/asm/uaccess.h
+++ b/arch/tile/include/asm/uaccess.h
@@ -246,7 +246,7 @@ extern int __get_user_bad(void)
 #define __put_user_4(x, ptr, ret) __put_user_asm(sw, x, ptr, ret)
 #define __put_user_8(x, ptr, ret)					\
 	({								\
-		u64 __x = (__typeof((x)-(x)))(x);			\
+		u64 __x = (u64)(__force unsigned long)(x);		\
 		int __lo = (int) __x, __hi = (int) (__x >> 32);		\
 		asm volatile("1: { sw %1, %2; addi %0, %1, 4 }\n"	\
 			     "2: { sw %0, %3; movei %0, 0 }\n"		\

  parent reply	other threads:[~2015-01-13  9:35 UTC|newest]

Thread overview: 134+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-06 15:43 [PATCH v2 00/40] uaccess: fix sparse warning on get/put_user for bitwise types Michael S. Tsirkin
2015-01-06 15:43 ` Michael S. Tsirkin
2015-01-06 15:43 ` [PATCH v2 01/40] x86/uaccess: fix sparse errors Michael S. Tsirkin
2015-01-06 15:43   ` Michael S. Tsirkin
2015-01-13 12:33   ` Thomas Gleixner
2015-01-13 12:33     ` Thomas Gleixner
2015-01-06 15:43 ` [PATCH v2 02/40] alpha/uaccess: " Michael S. Tsirkin
2015-01-06 15:43   ` Michael S. Tsirkin
2015-01-06 15:43 ` [PATCH v2 03/40] arm64/uaccess: " Michael S. Tsirkin
2015-01-06 15:43   ` Michael S. Tsirkin
2015-01-06 19:14   ` Will Deacon
2015-01-06 19:14     ` Will Deacon
2015-01-06 21:48     ` Michael S. Tsirkin
2015-01-06 21:48       ` Michael S. Tsirkin
2015-01-06 21:51       ` Michael S. Tsirkin
2015-01-06 21:51         ` Michael S. Tsirkin
2015-01-07 10:09         ` Will Deacon
2015-01-06 15:43 ` [PATCH v2 04/40] avr32/uaccess: " Michael S. Tsirkin
2015-01-06 15:43   ` Michael S. Tsirkin
2015-01-06 15:43 ` [PATCH v2 05/40] blackfin/uaccess: " Michael S. Tsirkin
2015-01-06 15:43 ` [PATCH v2 06/40] cris/uaccess: " Michael S. Tsirkin
2015-01-06 15:43   ` Michael S. Tsirkin
2015-01-27 16:27   ` Jesper Nilsson
2015-01-06 15:43 ` [PATCH v2 07/40] ia64/uaccess: " Michael S. Tsirkin
2015-01-06 15:43   ` Michael S. Tsirkin
2015-01-06 15:43 ` [PATCH v2 08/40] m32r/uaccess: " Michael S. Tsirkin
2015-01-06 15:43 ` [PATCH v2 09/40] metag/uaccess: " Michael S. Tsirkin
2015-01-06 15:43   ` Michael S. Tsirkin
     [not found]   ` <1420558883-10131-10-git-send-email-mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-01-07  9:47     ` James Hogan
2015-01-07  9:47       ` James Hogan
     [not found]       ` <54AD00AC.2030801-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
2015-01-07  9:55         ` Michael S. Tsirkin
2015-01-07  9:55           ` Michael S. Tsirkin
2015-01-06 15:44 ` [PATCH v2 10/40] microblaze/uaccess: " Michael S. Tsirkin
2015-01-07 10:14   ` Michal Simek
2015-01-07 10:14     ` Michal Simek
2015-01-06 15:44 ` [PATCH v2 11/40] openrisc/uaccess: " Michael S. Tsirkin
2015-01-06 15:44   ` Michael S. Tsirkin
2015-01-06 15:44 ` [PATCH v2 12/40] parisc/uaccess: " Michael S. Tsirkin
2015-01-06 15:44 ` [PATCH v2 13/40] sh/uaccess: " Michael S. Tsirkin
2015-01-06 15:44   ` Michael S. Tsirkin
2015-01-06 15:44 ` [PATCH v2 14/40] sparc32/uaccess: " Michael S. Tsirkin
2015-01-06 15:44   ` Michael S. Tsirkin
2015-01-06 18:21   ` David Miller
2015-01-06 18:21     ` David Miller
2015-01-06 15:44 ` [PATCH v2 15/40] sparc64/uaccess: " Michael S. Tsirkin
2015-01-06 18:21   ` David Miller
2015-01-06 18:21     ` David Miller
2015-01-06 15:44 ` [PATCH v2 16/40] m68k/uaccess: " Michael S. Tsirkin
2015-01-06 15:44   ` Michael S. Tsirkin
2015-01-07 10:38   ` Geert Uytterhoeven
2015-01-07 10:38     ` Geert Uytterhoeven
2015-01-06 15:44 ` [PATCH v2 17/40] arm: fix put_user " Michael S. Tsirkin
2015-01-06 15:44   ` Michael S. Tsirkin
2015-01-06 15:44 ` [PATCH v2 18/40] blackfin: " Michael S. Tsirkin
2015-01-06 15:44   ` Michael S. Tsirkin
2015-01-06 15:44 ` [PATCH v2 19/40] ia64: " Michael S. Tsirkin
2015-01-06 15:44   ` Michael S. Tsirkin
2015-01-06 15:44 ` [PATCH v2 20/40] metag: " Michael S. Tsirkin
2015-01-07  9:55   ` James Hogan
2015-01-07  9:55     ` James Hogan
2015-01-06 15:44 ` [PATCH v2 21/40] sh: " Michael S. Tsirkin
2015-01-06 15:44   ` Michael S. Tsirkin
2015-01-06 15:44 ` [PATCH v2 22/40] tile: " Michael S. Tsirkin
2015-01-06 15:44   ` Michael S. Tsirkin
2015-01-12 21:56   ` Chris Metcalf
2015-01-12 21:56     ` Chris Metcalf
2015-01-13  9:35     ` Michael S. Tsirkin [this message]
2015-01-13  9:40       ` Michael S. Tsirkin
2015-01-13 13:30       ` Chris Metcalf
2015-01-13  9:50     ` Michael S. Tsirkin
2015-01-06 15:44 ` [PATCH v2 23/40] tile: enable sparse checks for get/put_user Michael S. Tsirkin
2015-01-06 15:44   ` Michael S. Tsirkin
2015-01-13  0:08   ` Chris Metcalf
2015-01-13  9:45     ` Michael S. Tsirkin
2015-01-13 15:55       ` Chris Metcalf
2015-01-13 16:05         ` Michael S. Tsirkin
2015-01-06 15:44 ` [PATCH v2 24/40] avr32: whitespace fix Michael S. Tsirkin
2015-01-06 15:44   ` Michael S. Tsirkin
2015-01-07  6:50   ` Hans-Christian Egtvedt
2015-01-06 15:44 ` [PATCH v2 25/40] arch/sparc: uaccess_32 macro whitespace fixes Michael S. Tsirkin
2015-01-06 15:44   ` Michael S. Tsirkin
2015-01-06 18:22   ` David Miller
2015-01-06 18:22     ` David Miller
2015-01-06 15:44 ` [PATCH v2 26/40] arch/sparc: uaccess_64 " Michael S. Tsirkin
2015-01-06 15:44   ` Michael S. Tsirkin
2015-01-06 16:53   ` Sam Ravnborg
2015-01-06 17:19     ` Michael S. Tsirkin
2015-01-06 18:27       ` Sam Ravnborg
2015-01-06 18:27         ` Sam Ravnborg
2015-01-06 20:23         ` Michael S. Tsirkin
2015-01-06 20:46           ` Sam Ravnborg
2015-01-06 20:46             ` Sam Ravnborg
2015-01-06 21:44             ` Michael S. Tsirkin
2015-01-06 21:44               ` Michael S. Tsirkin
2015-01-06 18:22   ` David Miller
2015-01-06 18:22     ` David Miller
2015-01-06 15:45 ` [PATCH v2 27/40] blackfin: " Michael S. Tsirkin
2015-01-06 15:45 ` [PATCH v2 28/40] microblaze: whitespace fix Michael S. Tsirkin
2015-01-06 15:45   ` Michael S. Tsirkin
2015-01-07 10:14   ` Michal Simek
2015-01-06 15:45 ` [PATCH v2 29/40] alpha: macro whitespace fixes Michael S. Tsirkin
2015-01-06 15:45   ` Michael S. Tsirkin
2015-01-06 15:45 ` [PATCH v2 30/40] arm: " Michael S. Tsirkin
2015-01-06 15:45   ` Michael S. Tsirkin
2015-01-06 15:45 ` [PATCH v2 31/40] arm64: " Michael S. Tsirkin
2015-01-07 10:10   ` Will Deacon
2015-01-06 15:45 ` [PATCH v2 32/40] avr32: " Michael S. Tsirkin
2015-01-06 15:45   ` Michael S. Tsirkin
2015-01-07  6:52   ` Hans-Christian Egtvedt
2015-01-07  6:52     ` Hans-Christian Egtvedt
2015-01-06 15:45 ` [PATCH v2 33/40] cris: " Michael S. Tsirkin
2015-01-27 16:28   ` Jesper Nilsson
2015-01-06 15:45 ` [PATCH v2 34/40] frv: " Michael S. Tsirkin
2015-01-06 15:45   ` Michael S. Tsirkin
2015-01-06 15:45 ` [PATCH v2 35/40] m32r: " Michael S. Tsirkin
2015-01-06 15:45 ` [PATCH v2 36/40] m68k: " Michael S. Tsirkin
2015-01-06 15:45   ` Michael S. Tsirkin
2015-01-07  9:56   ` Geert Uytterhoeven
2015-01-07  9:56     ` Geert Uytterhoeven
2015-01-06 15:45 ` [PATCH v2 37/40] parisc: " Michael S. Tsirkin
2015-01-06 15:45   ` Michael S. Tsirkin
2015-01-06 15:45 ` [PATCH v2 38/40] s390: " Michael S. Tsirkin
2015-01-07  8:33   ` Heiko Carstens
2015-01-07  9:07     ` Michael S. Tsirkin
2015-01-06 15:45 ` [PATCH v2 39/40] sh: " Michael S. Tsirkin
2015-01-06 15:45 ` [PATCH v2 40/40] xtensa: " Michael S. Tsirkin
2015-01-06 15:45   ` Michael S. Tsirkin
2015-01-06 23:12   ` Max Filippov
2015-01-06 19:08 ` [PATCH v2 00/40] uaccess: fix sparse warning on get/put_user for bitwise types Arnd Bergmann
2015-01-06 19:08   ` Arnd Bergmann
2015-01-06 21:54   ` Michael S. Tsirkin
2015-01-06 21:54     ` Michael S. Tsirkin
2015-01-11 11:55   ` Michael S. Tsirkin
2015-01-13 10:19     ` Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150113093535.GA4434@redhat.com \
    --to=mst@redhat.com \
    --cc=arnd@arndb.de \
    --cc=cmetcalf@ezchip.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).