linux-kernel - Re: [PATCH v7 00/19] virtio: alignment issues

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Tue, 7 Apr 2020 05:48:55 -0400
From:   "Michael S. Tsirkin" <mst@...hat.com>
To:     Jason Wang <jasowang@...hat.com>
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 00/19] virtio: alignment issues

On Mon, Apr 06, 2020 at 11:41:32PM -0400, Jason Wang wrote:
> ----- Original Message -----
> > 
> > This is an alternative to
> > 	vhost: force spec specified alignment on types
> > which is a bit safer as it does not change UAPI.
> > I still think it's best to change the UAPI header as well,
> > we can do that as a follow-up cleanup.
> > 
> > changes from v6:
> > 	add missing header includes all over the place
> > changes from v5:
> > 	ack for mellanox patch
> > 	fixup to remoteproc
> > changes from v4:
> > 	fixup to issues reported by kbuild
> > changes from v3:
> > 	tools/virtio fixes
> > 	a bunch more cleanups that now become possible
> > 
> > Changes from v2:
> > 	don't change struct name, instead add ifndef
> > 	so kernel does not see the legacy UAPI version.
> > 
> > Jason, can you pls ack one of the approaches?
> 
> I prefer this approach but it looks a little bit risky for 5.7. Can we
> do this in 5.8?

It's not rc1 even yet, I think it's fine.

> Instead of using Kconfig, could we simply fail the initalization of
> vhost like:
> 
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 0395229486a9..e9f6a008ed12 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -2636,6 +2636,11 @@ EXPORT_SYMBOL_GPL(vhost_dequeue_msg);
>  
>  static int __init vhost_init(void)
>  {
> +	struct vhost_virtqueue *vq;
> +
> +	if (__alignof__ *vq->avail > VRING_AVAIL_ALIGN_SIZE)
> +		return -ENOTSUPP;
> +
>  	return 0;
>  }
> 
> Thanks

It's not just vhost, vringh is also broken.

It's not even rc1 yet, and I really like the resulting cleanup.
So I pushed this to next and it all seems to work pretty well,
I think I'll go with this.

> > 
> > 
> > Matej Genci (1):
> >   virtio: add VIRTIO_RING_NO_LEGACY
> > 
> > Michael S. Tsirkin (18):
> >   tools/virtio: define aligned attribute
> >   tools/virtio: make asm/barrier.h self contained
> >   tools/virtio: define __KERNEL__
> >   virtgpu: pull in uaccess.h
> >   virtio-rng: pull in slab.h
> >   remoteproc: pull in slab.h
> >   virtio_input: pull in slab.h
> >   virtio: stop using legacy struct vring in kernel
> >   vhost: force spec specified alignment on types
> >   virtio: add legacy init/size APIs
> >   virtio_ring: switch to virtio_legacy_init/size
> >   tools/virtio: switch to virtio_legacy_init/size
> >   vop: switch to virtio_legacy_init/size
> >   remoteproc: switch to virtio_legacy_init/size
> >   mellanox: switch to virtio_legacy_init/size
> >   vhost: option to fetch descriptors through an independent struct
> >   vhost: use batched version by default
> >   vhost: batching fetches
> > 
> >  drivers/block/virtio_blk.c               |   1 +
> >  drivers/char/hw_random/virtio-rng.c      |   1 +
> >  drivers/gpu/drm/virtio/virtgpu_ioctl.c   |   1 +
> >  drivers/misc/mic/vop/vop_main.c          |   5 +-
> >  drivers/misc/mic/vop/vop_vringh.c        |   8 +-
> >  drivers/platform/mellanox/mlxbf-tmfifo.c |   6 +-
> >  drivers/remoteproc/remoteproc_core.c     |   2 +-
> >  drivers/remoteproc/remoteproc_sysfs.c    |   1 +
> >  drivers/remoteproc/remoteproc_virtio.c   |   2 +-
> >  drivers/vhost/test.c                     |   2 +-
> >  drivers/vhost/vhost.c                    | 271 +++++++++++++++--------
> >  drivers/vhost/vhost.h                    |  23 +-
> >  drivers/virtio/virtio_input.c            |   1 +
> >  drivers/virtio/virtio_pci_modern.c       |   1 +
> >  drivers/virtio/virtio_ring.c             |  15 +-
> >  include/linux/virtio.h                   |   1 -
> >  include/linux/virtio_ring.h              |  46 ++++
> >  include/linux/vringh.h                   |   1 +
> >  include/uapi/linux/virtio_ring.h         |  30 ++-
> >  tools/virtio/Makefile                    |   2 +-
> >  tools/virtio/asm/barrier.h               |   1 +
> >  tools/virtio/linux/compiler.h            |   1 +
> >  tools/virtio/ringtest/virtio_ring_0_9.c  |   6 +-
> >  tools/virtio/virtio_test.c               |   6 +-
> >  tools/virtio/vringh_test.c               |  18 +-
> >  25 files changed, 311 insertions(+), 141 deletions(-)
> > 
> > --
> > MST
> > 
> > 
> 
> >