All of lore.kernel.org
 help / color / mirror / Atom feed
* RE: [PATCH v3 5/5] RFC: fw_cfg: add DMA write operation in sysfs
@ 2017-10-30 11:07 ` Hatayama, Daisuke
  0 siblings, 0 replies; 9+ messages in thread
From: Hatayama, Daisuke @ 2017-10-30 11:07 UTC (permalink / raw)
  To: 'marcandre.lureau@redhat.com'
  Cc: 'somlo@cmu.edu', 'mst@redhat.com',
	linux-kernel, 'qemu-devel@nongnu.org'

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Since qemu 2.9, DMA write operations are allowed. However, usage of this
> interface from kernel or user-space is strongly discouraged by the
> maintainers. This patch is meant for experimentations for now.
>

Could you (or maintainers?) tell me how experimental the DMA write
operations from kernel are?

>From some technical reason?
Or simply there has not been enough test yet so far?

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  drivers/firmware/qemu_fw_cfg.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
> index 54b569da3257..e2f2ad1c9c0c 100644
> --- a/drivers/firmware/qemu_fw_cfg.c
> +++ b/drivers/firmware/qemu_fw_cfg.c
> @@ -524,9 +524,28 @@ static ssize_t fw_cfg_sysfs_read_raw(struct file *filp, struct kobject *kobj,
>       return fw_cfg_read_blob(entry->f.select, buf, pos, count, true);
>  }
>
> +static ssize_t fw_cfg_sysfs_write_raw(struct file *filp, struct kobject *kobj,
> +                                   struct bin_attribute *bin_attr,
> +                                   char *buf, loff_t pos, size_t count)
> +{
> +     struct fw_cfg_sysfs_entry *entry = to_entry(kobj);
> +
> +     if (!fw_cfg_dma_enabled())
> +             return -ENOTSUPP;
> +
> +     if (pos > entry->f.size)
> +             return -EINVAL;
> +
> +     if (count > entry->f.size - pos)
> +             count = entry->f.size - pos;
> +
> +     return fw_cfg_write_blob(entry->f.select, buf, pos, count);
> +}
> +
>  static struct bin_attribute fw_cfg_sysfs_attr_raw = {
> -     .attr = { .name = "raw", .mode = S_IRUSR },
> +     .attr = { .name = "raw", .mode = S_IRUSR | S_IWUSR },
>       .read = fw_cfg_sysfs_read_raw,
> +     .write = fw_cfg_sysfs_write_raw,
>  };
>
>  /*
> --
> 2.14.1.146.gd35faa819

Thanks.
HATAYAMA, Daisuke

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v3 5/5] RFC: fw_cfg: add DMA write operation in sysfs
@ 2017-10-30 11:07 ` Hatayama, Daisuke
  0 siblings, 0 replies; 9+ messages in thread
From: Hatayama, Daisuke @ 2017-10-30 11:07 UTC (permalink / raw)
  To: 'marcandre.lureau@redhat.com'
  Cc: 'somlo@cmu.edu', 'mst@redhat.com',
	linux-kernel, 'qemu-devel@nongnu.org'

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Since qemu 2.9, DMA write operations are allowed. However, usage of this
> interface from kernel or user-space is strongly discouraged by the
> maintainers. This patch is meant for experimentations for now.
>

Could you (or maintainers?) tell me how experimental the DMA write
operations from kernel are?

From some technical reason?
Or simply there has not been enough test yet so far?

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  drivers/firmware/qemu_fw_cfg.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
> index 54b569da3257..e2f2ad1c9c0c 100644
> --- a/drivers/firmware/qemu_fw_cfg.c
> +++ b/drivers/firmware/qemu_fw_cfg.c
> @@ -524,9 +524,28 @@ static ssize_t fw_cfg_sysfs_read_raw(struct file *filp, struct kobject *kobj,
>       return fw_cfg_read_blob(entry->f.select, buf, pos, count, true);
>  }
>
> +static ssize_t fw_cfg_sysfs_write_raw(struct file *filp, struct kobject *kobj,
> +                                   struct bin_attribute *bin_attr,
> +                                   char *buf, loff_t pos, size_t count)
> +{
> +     struct fw_cfg_sysfs_entry *entry = to_entry(kobj);
> +
> +     if (!fw_cfg_dma_enabled())
> +             return -ENOTSUPP;
> +
> +     if (pos > entry->f.size)
> +             return -EINVAL;
> +
> +     if (count > entry->f.size - pos)
> +             count = entry->f.size - pos;
> +
> +     return fw_cfg_write_blob(entry->f.select, buf, pos, count);
> +}
> +
>  static struct bin_attribute fw_cfg_sysfs_attr_raw = {
> -     .attr = { .name = "raw", .mode = S_IRUSR },
> +     .attr = { .name = "raw", .mode = S_IRUSR | S_IWUSR },
>       .read = fw_cfg_sysfs_read_raw,
> +     .write = fw_cfg_sysfs_write_raw,
>  };
>
>  /*
> --
> 2.14.1.146.gd35faa819

Thanks.
HATAYAMA, Daisuke


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 5/5] RFC: fw_cfg: add DMA write operation in sysfs
  2017-10-30 11:07 ` [Qemu-devel] " Hatayama, Daisuke
@ 2017-10-30 16:19   ` Michael S. Tsirkin
  -1 siblings, 0 replies; 9+ messages in thread
From: Michael S. Tsirkin @ 2017-10-30 16:19 UTC (permalink / raw)
  To: Hatayama, Daisuke
  Cc: 'marcandre.lureau@redhat.com', 'somlo@cmu.edu',
	linux-kernel, 'qemu-devel@nongnu.org'

On Mon, Oct 30, 2017 at 11:07:20AM +0000, Hatayama, Daisuke wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Since qemu 2.9, DMA write operations are allowed. However, usage of this
> > interface from kernel or user-space is strongly discouraged by the
> > maintainers. This patch is meant for experimentations for now.
> >
> 
> Could you (or maintainers?) tell me how experimental the DMA write
> operations from kernel are?
> 
> From some technical reason?
> Or simply there has not been enough test yet so far?

The concern is security, talking from userspace to hypervisor
might become a trivial DOS vector.

If there's need for a specific entry to be accessible from userspace,
I'd rather white-list it.

> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  drivers/firmware/qemu_fw_cfg.c | 21 ++++++++++++++++++++-
> >  1 file changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
> > index 54b569da3257..e2f2ad1c9c0c 100644
> > --- a/drivers/firmware/qemu_fw_cfg.c
> > +++ b/drivers/firmware/qemu_fw_cfg.c
> > @@ -524,9 +524,28 @@ static ssize_t fw_cfg_sysfs_read_raw(struct file *filp, struct kobject *kobj,
> >       return fw_cfg_read_blob(entry->f.select, buf, pos, count, true);
> >  }
> >
> > +static ssize_t fw_cfg_sysfs_write_raw(struct file *filp, struct kobject *kobj,
> > +                                   struct bin_attribute *bin_attr,
> > +                                   char *buf, loff_t pos, size_t count)
> > +{
> > +     struct fw_cfg_sysfs_entry *entry = to_entry(kobj);
> > +
> > +     if (!fw_cfg_dma_enabled())
> > +             return -ENOTSUPP;
> > +
> > +     if (pos > entry->f.size)
> > +             return -EINVAL;
> > +
> > +     if (count > entry->f.size - pos)
> > +             count = entry->f.size - pos;
> > +
> > +     return fw_cfg_write_blob(entry->f.select, buf, pos, count);
> > +}
> > +
> >  static struct bin_attribute fw_cfg_sysfs_attr_raw = {
> > -     .attr = { .name = "raw", .mode = S_IRUSR },
> > +     .attr = { .name = "raw", .mode = S_IRUSR | S_IWUSR },
> >       .read = fw_cfg_sysfs_read_raw,
> > +     .write = fw_cfg_sysfs_write_raw,
> >  };
> >
> >  /*
> > --
> > 2.14.1.146.gd35faa819
> 
> Thanks.
> HATAYAMA, Daisuke
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v3 5/5] RFC: fw_cfg: add DMA write operation in sysfs
@ 2017-10-30 16:19   ` Michael S. Tsirkin
  0 siblings, 0 replies; 9+ messages in thread
From: Michael S. Tsirkin @ 2017-10-30 16:19 UTC (permalink / raw)
  To: Hatayama, Daisuke
  Cc: 'marcandre.lureau@redhat.com', 'somlo@cmu.edu',
	linux-kernel, 'qemu-devel@nongnu.org'

On Mon, Oct 30, 2017 at 11:07:20AM +0000, Hatayama, Daisuke wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Since qemu 2.9, DMA write operations are allowed. However, usage of this
> > interface from kernel or user-space is strongly discouraged by the
> > maintainers. This patch is meant for experimentations for now.
> >
> 
> Could you (or maintainers?) tell me how experimental the DMA write
> operations from kernel are?
> 
> From some technical reason?
> Or simply there has not been enough test yet so far?

The concern is security, talking from userspace to hypervisor
might become a trivial DOS vector.

If there's need for a specific entry to be accessible from userspace,
I'd rather white-list it.

> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  drivers/firmware/qemu_fw_cfg.c | 21 ++++++++++++++++++++-
> >  1 file changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
> > index 54b569da3257..e2f2ad1c9c0c 100644
> > --- a/drivers/firmware/qemu_fw_cfg.c
> > +++ b/drivers/firmware/qemu_fw_cfg.c
> > @@ -524,9 +524,28 @@ static ssize_t fw_cfg_sysfs_read_raw(struct file *filp, struct kobject *kobj,
> >       return fw_cfg_read_blob(entry->f.select, buf, pos, count, true);
> >  }
> >
> > +static ssize_t fw_cfg_sysfs_write_raw(struct file *filp, struct kobject *kobj,
> > +                                   struct bin_attribute *bin_attr,
> > +                                   char *buf, loff_t pos, size_t count)
> > +{
> > +     struct fw_cfg_sysfs_entry *entry = to_entry(kobj);
> > +
> > +     if (!fw_cfg_dma_enabled())
> > +             return -ENOTSUPP;
> > +
> > +     if (pos > entry->f.size)
> > +             return -EINVAL;
> > +
> > +     if (count > entry->f.size - pos)
> > +             count = entry->f.size - pos;
> > +
> > +     return fw_cfg_write_blob(entry->f.select, buf, pos, count);
> > +}
> > +
> >  static struct bin_attribute fw_cfg_sysfs_attr_raw = {
> > -     .attr = { .name = "raw", .mode = S_IRUSR },
> > +     .attr = { .name = "raw", .mode = S_IRUSR | S_IWUSR },
> >       .read = fw_cfg_sysfs_read_raw,
> > +     .write = fw_cfg_sysfs_write_raw,
> >  };
> >
> >  /*
> > --
> > 2.14.1.146.gd35faa819
> 
> Thanks.
> HATAYAMA, Daisuke
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH v3 5/5] RFC: fw_cfg: add DMA write operation in sysfs
  2017-10-30 16:19   ` [Qemu-devel] " Michael S. Tsirkin
@ 2017-10-31  9:55     ` Hatayama, Daisuke
  -1 siblings, 0 replies; 9+ messages in thread
From: Hatayama, Daisuke @ 2017-10-31  9:55 UTC (permalink / raw)
  To: 'Michael S. Tsirkin'
  Cc: 'marcandre.lureau@redhat.com', 'somlo@cmu.edu',
	linux-kernel, 'qemu-devel@nongnu.org'



> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org
> [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Michael S. Tsirkin
> Sent: Tuesday, October 31, 2017 1:19 AM
> To: Hatayama, Daisuke <d.hatayama@jp.fujitsu.com>
> Cc: 'marcandre.lureau@redhat.com' <marcandre.lureau@redhat.com>;
> 'somlo@cmu.edu' <somlo@cmu.edu>; linux-kernel@vger.kernel.org;
> 'qemu-devel@nongnu.org' <qemu-devel@nongnu.org>
> Subject: Re: [PATCH v3 5/5] RFC: fw_cfg: add DMA write operation in sysfs
> 
> On Mon, Oct 30, 2017 at 11:07:20AM +0000, Hatayama, Daisuke wrote:
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > Since qemu 2.9, DMA write operations are allowed. However, usage of this
> > > interface from kernel or user-space is strongly discouraged by the
> > > maintainers. This patch is meant for experimentations for now.
> > >
> >
> > Could you (or maintainers?) tell me how experimental the DMA write
> > operations from kernel are?
> >
> > From some technical reason?
> > Or simply there has not been enough test yet so far?
> 
> The concern is security, talking from userspace to hypervisor
> might become a trivial DOS vector.
> 
> If there's need for a specific entry to be accessible from userspace,
> I'd rather white-list it.
> 

Thanks for the explanation.

If so, I guess the white-list is done in userspace qemu side,
and no additional change would be needed for this kernel-side patch set for the white-list feature.
Is this understanding correct?

> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > >  drivers/firmware/qemu_fw_cfg.c | 21 ++++++++++++++++++++-
> > >  1 file changed, 20 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/firmware/qemu_fw_cfg.c
> b/drivers/firmware/qemu_fw_cfg.c
> > > index 54b569da3257..e2f2ad1c9c0c 100644
> > > --- a/drivers/firmware/qemu_fw_cfg.c
> > > +++ b/drivers/firmware/qemu_fw_cfg.c
> > > @@ -524,9 +524,28 @@ static ssize_t fw_cfg_sysfs_read_raw(struct file *filp,
> struct kobject *kobj,
> > >       return fw_cfg_read_blob(entry->f.select, buf, pos, count, true);
> > >  }
> > >
> > > +static ssize_t fw_cfg_sysfs_write_raw(struct file *filp, struct kobject
> *kobj,
> > > +                                   struct bin_attribute *bin_attr,
> > > +                                   char *buf, loff_t pos, size_t count)
> > > +{
> > > +     struct fw_cfg_sysfs_entry *entry = to_entry(kobj);
> > > +
> > > +     if (!fw_cfg_dma_enabled())
> > > +             return -ENOTSUPP;
> > > +
> > > +     if (pos > entry->f.size)
> > > +             return -EINVAL;
> > > +
> > > +     if (count > entry->f.size - pos)
> > > +             count = entry->f.size - pos;
> > > +
> > > +     return fw_cfg_write_blob(entry->f.select, buf, pos, count);
> > > +}
> > > +
> > >  static struct bin_attribute fw_cfg_sysfs_attr_raw = {
> > > -     .attr = { .name = "raw", .mode = S_IRUSR },
> > > +     .attr = { .name = "raw", .mode = S_IRUSR | S_IWUSR },
> > >       .read = fw_cfg_sysfs_read_raw,
> > > +     .write = fw_cfg_sysfs_write_raw,
> > >  };
> > >
> > >  /*
> > > --
> > > 2.14.1.146.gd35faa819
> >
> > Thanks.
> > HATAYAMA, Daisuke
> >
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v3 5/5] RFC: fw_cfg: add DMA write operation in sysfs
@ 2017-10-31  9:55     ` Hatayama, Daisuke
  0 siblings, 0 replies; 9+ messages in thread
From: Hatayama, Daisuke @ 2017-10-31  9:55 UTC (permalink / raw)
  To: 'Michael S. Tsirkin'
  Cc: 'marcandre.lureau@redhat.com', 'somlo@cmu.edu',
	linux-kernel, 'qemu-devel@nongnu.org'



> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org
> [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Michael S. Tsirkin
> Sent: Tuesday, October 31, 2017 1:19 AM
> To: Hatayama, Daisuke <d.hatayama@jp.fujitsu.com>
> Cc: 'marcandre.lureau@redhat.com' <marcandre.lureau@redhat.com>;
> 'somlo@cmu.edu' <somlo@cmu.edu>; linux-kernel@vger.kernel.org;
> 'qemu-devel@nongnu.org' <qemu-devel@nongnu.org>
> Subject: Re: [PATCH v3 5/5] RFC: fw_cfg: add DMA write operation in sysfs
> 
> On Mon, Oct 30, 2017 at 11:07:20AM +0000, Hatayama, Daisuke wrote:
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > Since qemu 2.9, DMA write operations are allowed. However, usage of this
> > > interface from kernel or user-space is strongly discouraged by the
> > > maintainers. This patch is meant for experimentations for now.
> > >
> >
> > Could you (or maintainers?) tell me how experimental the DMA write
> > operations from kernel are?
> >
> > From some technical reason?
> > Or simply there has not been enough test yet so far?
> 
> The concern is security, talking from userspace to hypervisor
> might become a trivial DOS vector.
> 
> If there's need for a specific entry to be accessible from userspace,
> I'd rather white-list it.
> 

Thanks for the explanation.

If so, I guess the white-list is done in userspace qemu side,
and no additional change would be needed for this kernel-side patch set for the white-list feature.
Is this understanding correct?

> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > >  drivers/firmware/qemu_fw_cfg.c | 21 ++++++++++++++++++++-
> > >  1 file changed, 20 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/firmware/qemu_fw_cfg.c
> b/drivers/firmware/qemu_fw_cfg.c
> > > index 54b569da3257..e2f2ad1c9c0c 100644
> > > --- a/drivers/firmware/qemu_fw_cfg.c
> > > +++ b/drivers/firmware/qemu_fw_cfg.c
> > > @@ -524,9 +524,28 @@ static ssize_t fw_cfg_sysfs_read_raw(struct file *filp,
> struct kobject *kobj,
> > >       return fw_cfg_read_blob(entry->f.select, buf, pos, count, true);
> > >  }
> > >
> > > +static ssize_t fw_cfg_sysfs_write_raw(struct file *filp, struct kobject
> *kobj,
> > > +                                   struct bin_attribute *bin_attr,
> > > +                                   char *buf, loff_t pos, size_t count)
> > > +{
> > > +     struct fw_cfg_sysfs_entry *entry = to_entry(kobj);
> > > +
> > > +     if (!fw_cfg_dma_enabled())
> > > +             return -ENOTSUPP;
> > > +
> > > +     if (pos > entry->f.size)
> > > +             return -EINVAL;
> > > +
> > > +     if (count > entry->f.size - pos)
> > > +             count = entry->f.size - pos;
> > > +
> > > +     return fw_cfg_write_blob(entry->f.select, buf, pos, count);
> > > +}
> > > +
> > >  static struct bin_attribute fw_cfg_sysfs_attr_raw = {
> > > -     .attr = { .name = "raw", .mode = S_IRUSR },
> > > +     .attr = { .name = "raw", .mode = S_IRUSR | S_IWUSR },
> > >       .read = fw_cfg_sysfs_read_raw,
> > > +     .write = fw_cfg_sysfs_write_raw,
> > >  };
> > >
> > >  /*
> > > --
> > > 2.14.1.146.gd35faa819
> >
> > Thanks.
> > HATAYAMA, Daisuke
> >
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 5/5] RFC: fw_cfg: add DMA write operation in sysfs
  2017-10-31  9:55     ` [Qemu-devel] " Hatayama, Daisuke
@ 2017-10-31 17:06       ` Michael S. Tsirkin
  -1 siblings, 0 replies; 9+ messages in thread
From: Michael S. Tsirkin @ 2017-10-31 17:06 UTC (permalink / raw)
  To: Hatayama, Daisuke
  Cc: 'marcandre.lureau@redhat.com', 'somlo@cmu.edu',
	linux-kernel, 'qemu-devel@nongnu.org'

On Tue, Oct 31, 2017 at 09:55:39AM +0000, Hatayama, Daisuke wrote:
> 
> 
> > -----Original Message-----
> > From: linux-kernel-owner@vger.kernel.org
> > [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Michael S. Tsirkin
> > Sent: Tuesday, October 31, 2017 1:19 AM
> > To: Hatayama, Daisuke <d.hatayama@jp.fujitsu.com>
> > Cc: 'marcandre.lureau@redhat.com' <marcandre.lureau@redhat.com>;
> > 'somlo@cmu.edu' <somlo@cmu.edu>; linux-kernel@vger.kernel.org;
> > 'qemu-devel@nongnu.org' <qemu-devel@nongnu.org>
> > Subject: Re: [PATCH v3 5/5] RFC: fw_cfg: add DMA write operation in sysfs
> > 
> > On Mon, Oct 30, 2017 at 11:07:20AM +0000, Hatayama, Daisuke wrote:
> > > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > >
> > > > Since qemu 2.9, DMA write operations are allowed. However, usage of this
> > > > interface from kernel or user-space is strongly discouraged by the
> > > > maintainers. This patch is meant for experimentations for now.
> > > >
> > >
> > > Could you (or maintainers?) tell me how experimental the DMA write
> > > operations from kernel are?
> > >
> > > From some technical reason?
> > > Or simply there has not been enough test yet so far?
> > 
> > The concern is security, talking from userspace to hypervisor
> > might become a trivial DOS vector.
> > 
> > If there's need for a specific entry to be accessible from userspace,
> > I'd rather white-list it.
> > 
> 
> Thanks for the explanation.
> 
> If so, I guess the white-list is done in userspace qemu side,
> and no additional change would be needed for this kernel-side patch set for the white-list feature.
> Is this understanding correct?

No - host side has no idea whether it's guest userspace or
kernel initiating the access.

> > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > ---
> > > >  drivers/firmware/qemu_fw_cfg.c | 21 ++++++++++++++++++++-
> > > >  1 file changed, 20 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/firmware/qemu_fw_cfg.c
> > b/drivers/firmware/qemu_fw_cfg.c
> > > > index 54b569da3257..e2f2ad1c9c0c 100644
> > > > --- a/drivers/firmware/qemu_fw_cfg.c
> > > > +++ b/drivers/firmware/qemu_fw_cfg.c
> > > > @@ -524,9 +524,28 @@ static ssize_t fw_cfg_sysfs_read_raw(struct file *filp,
> > struct kobject *kobj,
> > > >       return fw_cfg_read_blob(entry->f.select, buf, pos, count, true);
> > > >  }
> > > >
> > > > +static ssize_t fw_cfg_sysfs_write_raw(struct file *filp, struct kobject
> > *kobj,
> > > > +                                   struct bin_attribute *bin_attr,
> > > > +                                   char *buf, loff_t pos, size_t count)
> > > > +{
> > > > +     struct fw_cfg_sysfs_entry *entry = to_entry(kobj);
> > > > +
> > > > +     if (!fw_cfg_dma_enabled())
> > > > +             return -ENOTSUPP;
> > > > +
> > > > +     if (pos > entry->f.size)
> > > > +             return -EINVAL;
> > > > +
> > > > +     if (count > entry->f.size - pos)
> > > > +             count = entry->f.size - pos;
> > > > +
> > > > +     return fw_cfg_write_blob(entry->f.select, buf, pos, count);
> > > > +}
> > > > +
> > > >  static struct bin_attribute fw_cfg_sysfs_attr_raw = {
> > > > -     .attr = { .name = "raw", .mode = S_IRUSR },
> > > > +     .attr = { .name = "raw", .mode = S_IRUSR | S_IWUSR },
> > > >       .read = fw_cfg_sysfs_read_raw,
> > > > +     .write = fw_cfg_sysfs_write_raw,
> > > >  };
> > > >
> > > >  /*
> > > > --
> > > > 2.14.1.146.gd35faa819
> > >
> > > Thanks.
> > > HATAYAMA, Daisuke
> > >
> > 
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v3 5/5] RFC: fw_cfg: add DMA write operation in sysfs
@ 2017-10-31 17:06       ` Michael S. Tsirkin
  0 siblings, 0 replies; 9+ messages in thread
From: Michael S. Tsirkin @ 2017-10-31 17:06 UTC (permalink / raw)
  To: Hatayama, Daisuke
  Cc: 'marcandre.lureau@redhat.com', 'somlo@cmu.edu',
	linux-kernel, 'qemu-devel@nongnu.org'

On Tue, Oct 31, 2017 at 09:55:39AM +0000, Hatayama, Daisuke wrote:
> 
> 
> > -----Original Message-----
> > From: linux-kernel-owner@vger.kernel.org
> > [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Michael S. Tsirkin
> > Sent: Tuesday, October 31, 2017 1:19 AM
> > To: Hatayama, Daisuke <d.hatayama@jp.fujitsu.com>
> > Cc: 'marcandre.lureau@redhat.com' <marcandre.lureau@redhat.com>;
> > 'somlo@cmu.edu' <somlo@cmu.edu>; linux-kernel@vger.kernel.org;
> > 'qemu-devel@nongnu.org' <qemu-devel@nongnu.org>
> > Subject: Re: [PATCH v3 5/5] RFC: fw_cfg: add DMA write operation in sysfs
> > 
> > On Mon, Oct 30, 2017 at 11:07:20AM +0000, Hatayama, Daisuke wrote:
> > > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > >
> > > > Since qemu 2.9, DMA write operations are allowed. However, usage of this
> > > > interface from kernel or user-space is strongly discouraged by the
> > > > maintainers. This patch is meant for experimentations for now.
> > > >
> > >
> > > Could you (or maintainers?) tell me how experimental the DMA write
> > > operations from kernel are?
> > >
> > > From some technical reason?
> > > Or simply there has not been enough test yet so far?
> > 
> > The concern is security, talking from userspace to hypervisor
> > might become a trivial DOS vector.
> > 
> > If there's need for a specific entry to be accessible from userspace,
> > I'd rather white-list it.
> > 
> 
> Thanks for the explanation.
> 
> If so, I guess the white-list is done in userspace qemu side,
> and no additional change would be needed for this kernel-side patch set for the white-list feature.
> Is this understanding correct?

No - host side has no idea whether it's guest userspace or
kernel initiating the access.

> > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > ---
> > > >  drivers/firmware/qemu_fw_cfg.c | 21 ++++++++++++++++++++-
> > > >  1 file changed, 20 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/firmware/qemu_fw_cfg.c
> > b/drivers/firmware/qemu_fw_cfg.c
> > > > index 54b569da3257..e2f2ad1c9c0c 100644
> > > > --- a/drivers/firmware/qemu_fw_cfg.c
> > > > +++ b/drivers/firmware/qemu_fw_cfg.c
> > > > @@ -524,9 +524,28 @@ static ssize_t fw_cfg_sysfs_read_raw(struct file *filp,
> > struct kobject *kobj,
> > > >       return fw_cfg_read_blob(entry->f.select, buf, pos, count, true);
> > > >  }
> > > >
> > > > +static ssize_t fw_cfg_sysfs_write_raw(struct file *filp, struct kobject
> > *kobj,
> > > > +                                   struct bin_attribute *bin_attr,
> > > > +                                   char *buf, loff_t pos, size_t count)
> > > > +{
> > > > +     struct fw_cfg_sysfs_entry *entry = to_entry(kobj);
> > > > +
> > > > +     if (!fw_cfg_dma_enabled())
> > > > +             return -ENOTSUPP;
> > > > +
> > > > +     if (pos > entry->f.size)
> > > > +             return -EINVAL;
> > > > +
> > > > +     if (count > entry->f.size - pos)
> > > > +             count = entry->f.size - pos;
> > > > +
> > > > +     return fw_cfg_write_blob(entry->f.select, buf, pos, count);
> > > > +}
> > > > +
> > > >  static struct bin_attribute fw_cfg_sysfs_attr_raw = {
> > > > -     .attr = { .name = "raw", .mode = S_IRUSR },
> > > > +     .attr = { .name = "raw", .mode = S_IRUSR | S_IWUSR },
> > > >       .read = fw_cfg_sysfs_read_raw,
> > > > +     .write = fw_cfg_sysfs_write_raw,
> > > >  };
> > > >
> > > >  /*
> > > > --
> > > > 2.14.1.146.gd35faa819
> > >
> > > Thanks.
> > > HATAYAMA, Daisuke
> > >
> > 
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 5/5] RFC: fw_cfg: add DMA write operation in sysfs
  2017-09-22 13:24 [PATCH v3 0/5] fw_cfg: add DMA operations & etc/vmcoreinfo support marcandre.lureau
@ 2017-09-22 13:24 ` marcandre.lureau
  0 siblings, 0 replies; 9+ messages in thread
From: marcandre.lureau @ 2017-09-22 13:24 UTC (permalink / raw)
  To: linux-kernel; +Cc: somlo, qemu-devel, mst, Marc-André Lureau

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Since qemu 2.9, DMA write operations are allowed. However, usage of this
interface from kernel or user-space is strongly discouraged by the
maintainers. This patch is meant for experimentations for now.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 drivers/firmware/qemu_fw_cfg.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
index 54b569da3257..e2f2ad1c9c0c 100644
--- a/drivers/firmware/qemu_fw_cfg.c
+++ b/drivers/firmware/qemu_fw_cfg.c
@@ -524,9 +524,28 @@ static ssize_t fw_cfg_sysfs_read_raw(struct file *filp, struct kobject *kobj,
 	return fw_cfg_read_blob(entry->f.select, buf, pos, count, true);
 }
 
+static ssize_t fw_cfg_sysfs_write_raw(struct file *filp, struct kobject *kobj,
+				      struct bin_attribute *bin_attr,
+				      char *buf, loff_t pos, size_t count)
+{
+	struct fw_cfg_sysfs_entry *entry = to_entry(kobj);
+
+	if (!fw_cfg_dma_enabled())
+		return -ENOTSUPP;
+
+	if (pos > entry->f.size)
+		return -EINVAL;
+
+	if (count > entry->f.size - pos)
+		count = entry->f.size - pos;
+
+	return fw_cfg_write_blob(entry->f.select, buf, pos, count);
+}
+
 static struct bin_attribute fw_cfg_sysfs_attr_raw = {
-	.attr = { .name = "raw", .mode = S_IRUSR },
+	.attr = { .name = "raw", .mode = S_IRUSR | S_IWUSR },
 	.read = fw_cfg_sysfs_read_raw,
+	.write = fw_cfg_sysfs_write_raw,
 };
 
 /*
-- 
2.14.1.146.gd35faa819

^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-10-31 17:06 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-30 11:07 [PATCH v3 5/5] RFC: fw_cfg: add DMA write operation in sysfs Hatayama, Daisuke
2017-10-30 11:07 ` [Qemu-devel] " Hatayama, Daisuke
2017-10-30 16:19 ` Michael S. Tsirkin
2017-10-30 16:19   ` [Qemu-devel] " Michael S. Tsirkin
2017-10-31  9:55   ` Hatayama, Daisuke
2017-10-31  9:55     ` [Qemu-devel] " Hatayama, Daisuke
2017-10-31 17:06     ` Michael S. Tsirkin
2017-10-31 17:06       ` [Qemu-devel] " Michael S. Tsirkin
  -- strict thread matches above, loose matches on Subject: below --
2017-09-22 13:24 [PATCH v3 0/5] fw_cfg: add DMA operations & etc/vmcoreinfo support marcandre.lureau
2017-09-22 13:24 ` [PATCH v3 5/5] RFC: fw_cfg: add DMA write operation in sysfs marcandre.lureau

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.