[PATCH] powerpc/powernv: Add mmap to opal export sysfs nodes

[Oliver]

On Fri, Mar 15, 2019 at 11:55 AM Jordan Niethe <jniethe5 at 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 at 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 at 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
>