diff mbox series

powerpc/powernv: Add mmap to opal export sysfs nodes

Message ID 20190315005420.20212-1-jniethe5@gmail.com
State Superseded
Headers show
Series powerpc/powernv: Add mmap to opal export sysfs nodes | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch next (de3c83c2fd2b87cf68214eda76dfa66989d78cb6)
snowpatch_ozlabs/build-ppc64le success Build succeeded
snowpatch_ozlabs/build-ppc64be success Build succeeded
snowpatch_ozlabs/build-ppc64e success Build succeeded
snowpatch_ozlabs/build-pmac32 success Build succeeded
snowpatch_ozlabs/checkpatch warning total: 0 errors, 0 warnings, 1 checks, 22 lines checked

Commit Message

Jordan Niethe March 15, 2019, 12:54 a.m. UTC
The sysfs nodes created under /opal/exports/ do not currently support
mmap. Skiboot trace buffers are not yet added to this location but
this is a suitable for them to be exported to. Adding mmap support makes
using these trace buffers more convenient. The state in the header of
the trace buffer is needed to ensure the read position has not been
overwritten. Thus the header of the buffer must be read, then the
read position itself. Using lseek/read to do this introduces a delay
that could result in incorrect reads if the read position is overwritten
after the header is read.

Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
 arch/powerpc/platforms/powernv/opal.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Oliver O'Halloran April 2, 2019, 6:07 a.m. UTC | #1
On Fri, Mar 15, 2019 at 11:55 AM Jordan Niethe <jniethe5@gmail.com> wrote:
>
> The sysfs nodes created under /opal/exports/ do not currently support
> mmap. Skiboot trace buffers are not yet added to this location but
> this is a suitable for them to be exported to. Adding mmap support makes
> using these trace buffers more convenient. The state in the header of
> the trace buffer is needed to ensure the read position has not been
> overwritten. Thus the header of the buffer must be read, then the
> read position itself. Using lseek/read to do this introduces a delay
> that could result in incorrect reads if the read position is overwritten
> after the header is read.
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
>  arch/powerpc/platforms/powernv/opal.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
> index 2b0eca104f86..3cfc683bb060 100644
> --- a/arch/powerpc/platforms/powernv/opal.c
> +++ b/arch/powerpc/platforms/powernv/opal.c
> @@ -714,6 +714,15 @@ static ssize_t export_attr_read(struct file *fp, struct kobject *kobj,
>                                        bin_attr->size);
>  }
>
> +static int export_attr_mmap(struct file *fp, struct kobject *kobj,
> +                               struct bin_attribute *attr,
> +                               struct vm_area_struct *vma)
> +{
> +       return remap_pfn_range(vma, vma->vm_start,
> +                               __pa(attr->private) >> PAGE_SHIFT,
> +                               attr->size, PAGE_READONLY);
> +}
> +
>  /*
>   * opal_export_attrs: creates a sysfs node for each property listed in
>   * the device-tree under /ibm,opal/firmware/exports/
> @@ -759,6 +768,7 @@ static void opal_export_attrs(void)
>                 attr->attr.name = kstrdup(prop->name, GFP_KERNEL);
>                 attr->attr.mode = 0400;
>                 attr->read = export_attr_read;
> +               attr->mmap = export_attr_mmap;

You are assuming that it's safe to allow the export to be mmap()ed
here and that's not always the case. e.g.

[16:56 oliveroh@localhost .../firmware/exports]$ lsprop
symbol_map       00000000 30125640 00000000 0003598d
hdat_map         00000000 31200000 00000000 00800000
phandle          100002e8 (268436200)
name             "exports"

The symbol_map export there does not start (or end) at a page
boundary. If you allow user programs to mmap() that export it will
make everything from 0x30120000 to 0x30160000 and the additional space
might contain internal firmware data structures that we'd rather not
allow userspace to access.You can protect against that by validating
the export starts and ends on a page boundary and only populating the
.mmap when it's safe to do so.

>                 attr->private = __va(vals[0]);
>                 attr->size = vals[1];
>
> --
> 2.20.1
>
diff mbox series

Patch

diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 2b0eca104f86..3cfc683bb060 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -714,6 +714,15 @@  static ssize_t export_attr_read(struct file *fp, struct kobject *kobj,
 				       bin_attr->size);
 }
 
+static int export_attr_mmap(struct file *fp, struct kobject *kobj,
+				struct bin_attribute *attr,
+				struct vm_area_struct *vma)
+{
+	return remap_pfn_range(vma, vma->vm_start,
+				__pa(attr->private) >> PAGE_SHIFT,
+				attr->size, PAGE_READONLY);
+}
+
 /*
  * opal_export_attrs: creates a sysfs node for each property listed in
  * the device-tree under /ibm,opal/firmware/exports/
@@ -759,6 +768,7 @@  static void opal_export_attrs(void)
 		attr->attr.name = kstrdup(prop->name, GFP_KERNEL);
 		attr->attr.mode = 0400;
 		attr->read = export_attr_read;
+		attr->mmap = export_attr_mmap;
 		attr->private = __va(vals[0]);
 		attr->size = vals[1];