linux-kernel - Re: [PATCH v2 2/2] tools/virtio: fix build caused by virtio

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, 11 Apr 2023 02:37:54 -0400
From:   "Michael S. Tsirkin" <mst@...hat.com>
To:     Shunsuke Mie <mie@...l.co.jp>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jason Wang <jasowang@...hat.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        "Matthew Wilcox (Oracle)" <willy@...radead.org>,
        Stefano Garzarella <sgarzare@...hat.com>,
        Eugenio Pérez <eperezma@...hat.com>,
        linux-kernel@...r.kernel.org,
        virtualization@...ts.linux-foundation.org
Subject: Re: [PATCH v2 2/2] tools/virtio: fix build caused by virtio_ring
 changes

On Tue, Apr 11, 2023 at 12:25:39PM +0900, Shunsuke Mie wrote:
> 
> On 2023/04/11 4:44, Michael S. Tsirkin wrote:
> > On Mon, Apr 10, 2023 at 08:40:34PM +0200, Greg Kroah-Hartman wrote:
> > > On Mon, Apr 10, 2023 at 08:00:33AM -0400, Michael S. Tsirkin wrote:
> > > > On Mon, Apr 10, 2023 at 08:28:45PM +0900, Shunsuke Mie wrote:
> > > > > Fix the build dependency for virtio_test. The virtio_ring that is used from
> > > > > the test requires container_of_const(). Change to use container_of.h kernel
> > > > > header directly and adapt related codes.
> > > > > 
> > > > > Signed-off-by: Shunsuke Mie <mie@...l.co.jp>
> > > > This is only for next right? That's where container_of_const
> > > > things are I think ...
> > > container_of_const() is in 6.2.
> > 
> > Absolutely but the patch switching virtio to that is not there.
> > 
> > 
> > > > > ---
> > > > >   tools/include/linux/types.h   |  1 -
> > > > >   tools/virtio/linux/compiler.h |  2 ++
> > > > >   tools/virtio/linux/kernel.h   |  5 +----
> > > > >   tools/virtio/linux/module.h   |  1 -
> > > > >   tools/virtio/linux/uaccess.h  | 11 ++---------
> > > > >   5 files changed, 5 insertions(+), 15 deletions(-)
> > > > > 
> > > > > diff --git a/tools/include/linux/types.h b/tools/include/linux/types.h
> > > > > index 051fdeaf2670..f1896b70a8e5 100644
> > > > > --- a/tools/include/linux/types.h
> > > > > +++ b/tools/include/linux/types.h
> > > > > @@ -49,7 +49,6 @@ typedef __s8  s8;
> > > > >   #endif
> > > > >   #define __force
> > > > > -#define __user
> > > Why is this needed?
> > > 
> > > > >   #define __must_check
> > > > >   #define __cold
> > > > > diff --git a/tools/virtio/linux/compiler.h b/tools/virtio/linux/compiler.h
> > > > > index 2c51bccb97bb..1f3a15b954b9 100644
> > > > > --- a/tools/virtio/linux/compiler.h
> > > > > +++ b/tools/virtio/linux/compiler.h
> > > > > @@ -2,6 +2,8 @@
> > > > >   #ifndef LINUX_COMPILER_H
> > > > >   #define LINUX_COMPILER_H
> > > > > +#include "../../../include/linux/compiler_types.h"
> > > While I understand your need to not want to duplicate code, what in the
> > > world is this doing?  Why not use the in-kernel compiler.h instead?  Why
> > > are you copying loads of .h files into tools/virtio/?  What is this for
> > > and why not just use the real files so you don't have to even attempt to
> > > try to keep things in sync (hint, they will always be out of sync.)
> > Because it's doing a very weird hack: building some kernel
> > code into a userspace binary, used for just for testing.
> > This is all not part of kernel build intentionally, no
> > one is supposed to use this binary in production, but
> > it was helpful in finding bugs in virtio core in the past
> > so I keep it around.
> Would it be possible to switch to in-kernel tests, such as KUnit? If it's
> possible, we could use the kernel headers and implementations as they are,
> and we could address the concerns that Greg raised I think.
> 
> 
> Best regards,
> 
> Shunsuke

I think your patch is fine as is. Using kunit is certainly possible,
but won't be a small project. tools/virtio was always a quick
hack to help experiment quickly, so I'm not worried by it being
broken often - whoever wants to play with it next, fixes it up.

-- 
MST