linux-kernel - Re: [PATCH RESEND] virtio: Fix typecast of pointer in vring

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 6 Jul 2015 11:24:42 +0200
From:	Thomas Huth <thuth@...hat.com>
To:	"Michael S. Tsirkin" <mst@...hat.com>
Cc:	virtualization@...ts.linux-foundation.org,
	linux-api@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND] virtio: Fix typecast of pointer in vring_init()

On Sun, 5 Jul 2015 14:59:54 +0200
"Michael S. Tsirkin" <mst@...hat.com> wrote:

> On Sun, Jul 05, 2015 at 12:58:53PM +0200, Michael S. Tsirkin wrote:
> > On Thu, Jul 02, 2015 at 09:21:22AM +0200, Thomas Huth wrote:
> > > The virtio_ring.h header is used in userspace programs (ie. QEMU),
> > > too. Here we can not assume that sizeof(pointer) is the same as
> > > sizeof(long), e.g. when compiling for Windows, so the typecast in
> > > vring_init() should be done with (uintptr_t) instead of (unsigned long).
> > > 
> > > Signed-off-by: Thomas Huth <thuth@...hat.com>
> > 
> > This seems to break some userspace too:
> > 
> >   INSTALL usr/include/linux/ (413 files)
> >   CHECK   usr/include/linux/ (413 files)
> >   HOSTCC  Documentation/accounting/getdelays
> >   HOSTCC  Documentation/connector/ucon
> >   HOSTCC  Documentation/mic/mpssd/mpssd.o
> > In file included from Documentation/mic/mpssd/mpssd.c:36:0:
> > ./usr/include/linux/virtio_ring.h: In function ‘vring_init’:
> > ./usr/include/linux/virtio_ring.h:146:24: error: ‘uintptr_t’ undeclared
> > (first use in this function)
> >   vr->used = (void *)(((uintptr_t)&vr->avail->ring[num] +
> > sizeof(__virtio16)
> >                         ^
> > ./usr/include/linux/virtio_ring.h:146:24: note: each undeclared
> > identifier is reported only once for each function it appears in
> > scripts/Makefile.host:108: recipe for target

D'oh, I only built the kernel for powerpc, that's why I did not notice
this breakage in the MIC code. Sorry!

> > E.g. fuse.h has this code:
> > #ifdef __KERNEL__
> > #include <linux/types.h>
> > #else
> > #include <stdint.h>
> > #endif
> > 
> > Maybe we need something similar.
> 
> The following seems to help.  Does it help the windows build?
...
> diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
> index 8682551..c072959 100644
> --- a/include/uapi/linux/virtio_ring.h
> +++ b/include/uapi/linux/virtio_ring.h
> @@ -31,6 +31,9 @@
>   * SUCH DAMAGE.
>   *
>   * Copyright Rusty Russell IBM Corporation 2007. */
> +#ifndef __KERNEL__
> +#include <stdint.h>
> +#endif
>  #include <linux/types.h>
>  #include <linux/virtio_types.h>

Since the update-linux-headers.sh script from QEMU replaces the
"#include <linux/types.h>" in virtio_ring.h to include a file that again
includes stdint.h already, your above patch should not do any
difference for compiling QEMU for Windows, I think. It's really just
about replacing the typecast there.

But you're right of course for other user space applications which
include this header by other means. So I'm fine if you change my patch
with your hunk above (or add it as an additional patch). Or shall I
rather resend my patch with your above hunk included?

 Thomas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/