netdev - RE: [PATCH V2 vfio 03/11] vfio: Introduce DMA logging uAPIs

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-next>] [day] [month] [year] [list]

Message-ID: <BN9PR11MB5276BEDFBBD53A44C1525A118C959@BN9PR11MB5276.namprd11.prod.outlook.com>
Date:   Mon, 25 Jul 2022 07:30:46 +0000
From:   "Tian, Kevin" <kevin.tian@...el.com>
To:     Yishai Hadas <yishaih@...dia.com>,
        "alex.williamson@...hat.com" <alex.williamson@...hat.com>,
        "jgg@...dia.com" <jgg@...dia.com>
CC:     "saeedm@...dia.com" <saeedm@...dia.com>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "Martins, Joao" <joao.m.martins@...cle.com>,
        "leonro@...dia.com" <leonro@...dia.com>,
        "maorg@...dia.com" <maorg@...dia.com>,
        "cohuck@...hat.com" <cohuck@...hat.com>
Subject: RE: [PATCH V2 vfio 03/11] vfio: Introduce DMA logging uAPIs

<please use plain-text next time>

> From: Yishai Hadas <yishaih@...dia.com> 
> Sent: Thursday, July 21, 2022 7:06 PM
> > > +/*
> > > + * Upon VFIO_DEVICE_FEATURE_SET start device DMA logging.
> >
> > both 'start'/'stop' are via VFIO_DEVICE_FEATURE_SET
>
> Right, we have a note for that near VFIO_DEVICE_FEATURE_DMA_LOGGING_STOP.
> Here it refers to the start option.

let's make it accurate here.

> > > + * page_size is an input that hints what tracking granularity the device
> > > + * should try to achieve. If the device cannot do the hinted page size then it
> > > + * should pick the next closest page size it supports. On output the device
> >
> > next closest 'smaller' page size?
>
> Not only, it depends on the device capabilities/support and should be a driver choice.

'should pick next closest" is a guideline to the driver. If user requests 
8KB while the device supports 4KB and 16KB, which one is closest?

It's probably safer to just say that it's a driver choice when the hinted page
size cannot be set?

> > > +struct vfio_device_feature_dma_logging_control {
> > > +	__aligned_u64 page_size;
> > > +	__u32 num_ranges;
> > > +	__u32 __reserved;
> > > +	__aligned_u64 ranges;
> > > +};
> >
> > should we move the definition of LOG_MAX_RANGES to be here
> > so the user can know the max limits of tracked ranges?
> This was raised as an option as part of this mail thread.
> However, for now it seems redundant as we may not expect user space to hit this limit and it mainly comes to protect kernel from memory exploding by a malicious user.

No matter how realistic an user might hit an limitation, it doesn't
sound good to not expose it if existing.

> > > +
> > > +struct vfio_device_feature_dma_logging_range {
> > > +	__aligned_u64 iova;
> > > +	__aligned_u64 length;
> > > +};
> > > +
> > > +#define VFIO_DEVICE_FEATURE_DMA_LOGGING_START 3
> >
> > Can the user update the range list by doing another START?
>
> No, single start to ask the device what to track and a matching single stop should follow at the end.

let's document it then.

Thanks
Kevin