linux-kernel - Re: [RFC PATCH 1/7] vfio: Include optional device match in vfio_device

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]

Message-ID: <20200207103334.25b17267.cohuck@redhat.com>
Date:   Fri, 7 Feb 2020 10:33:34 +0100
From:   Cornelia Huck <cohuck@...hat.com>
To:     Alex Williamson <alex.williamson@...hat.com>
Cc:     kvm@...r.kernel.org, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org, dev@...k.org, mtosatti@...hat.com,
        thomas@...jalon.net, bluca@...ian.org, jerinjacobk@...il.com,
        bruce.richardson@...el.com
Subject: Re: [RFC PATCH 1/7] vfio: Include optional device match in
 vfio_device_ops callbacks

On Thu, 6 Feb 2020 11:18:42 -0700
Alex Williamson <alex.williamson@...hat.com> wrote:

> On Thu, 6 Feb 2020 12:14:19 +0100
> Cornelia Huck <cohuck@...hat.com> wrote:
> 
> > On Tue, 04 Feb 2020 16:05:43 -0700
> > Alex Williamson <alex.williamson@...hat.com> wrote:
> >   
> > > Allow bus drivers to provide their own callback to match a device to
> > > the user provided string.
> > > 
> > > Signed-off-by: Alex Williamson <alex.williamson@...hat.com>
> > > ---
> > >  drivers/vfio/vfio.c  |   19 +++++++++++++++----
> > >  include/linux/vfio.h |    3 +++

> I think with your first option we arrive at something like this:
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index dda1726adda8..b5609a411139 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -883,14 +883,15 @@ static struct vfio_device *vfio_device_get_from_name(struct vfio_group *group,
>  
>  		if (it->ops->match) {
>  			ret = it->ops->match(it->device_data, buf);
> -			if (ret < 0 && ret != -ENODEV) {
> +			if (ret < 0) {
>  				device = ERR_PTR(ret);
>  				break;
>  			}
> -		} else
> -			ret = strcmp(dev_name(it->dev), buf);
> +		} else {
> +			ret = !strcmp(dev_name(it->dev), buf);
> +		}
>  
> -		if (!ret) {
> +		if (ret) {
>  			device = it;
>  			vfio_device_get(device);
>  			break;
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 755e0f0e2900..029694b977f2 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -26,8 +26,9 @@
>   *         operations documented below
>   * @mmap: Perform mmap(2) on a region of the device file descriptor
>   * @request: Request for the bus driver to release the device
> - * @match: Optional device name match callback (return: 0 for match, -ENODEV
> - *         (or >0) for no match and continue, other -errno: no match and stop)
> + * @match: Optional device name match callback (return: 0 for no-match, >0 for
> + *         match, -errno for abort (ex. match with insufficient or incorrect
> + *         additional args)
>   */
>  struct vfio_device_ops {
>  	char	*name;
> 
> 
> I like that.  Thanks,
> 
> Alex

Looks good to me.