[PATCH] powerpc/powernv: Add mmap to opal export sysfs nodes
The sysfs nodes created under /opal/exports/ do not currently support mmap. Skiboot trace buffers are exported here with in the series https://patchwork.ozlabs.org/cover/1073501/. Adding mmap support makes it possible to use the functions for reading traces in external/trace. This improves on the current read/lseek method as it handles cases like the buffer wrapping and overflowing. Signed-off-by: Jordan Niethe --- v2: ensure only whole pages can be mapped --- arch/powerpc/platforms/powernv/opal.c | 12 1 file changed, 12 insertions(+) diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c index 2b0eca104f86..3611b5b9c5d2 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,9 @@ static void opal_export_attrs(void) attr->attr.name = kstrdup(prop->name, GFP_KERNEL); attr->attr.mode = 0400; attr->read = export_attr_read; + /* Ensure only whole pages are mapped */ + if (vals[0] % PAGE_SIZE == 0 && vals[1] % PAGE_SIZE == 0) + attr->mmap = export_attr_mmap; attr->private = __va(vals[0]); attr->size = vals[1]; -- 2.20.1
Re: [PATCH] powerpc/powernv: Add mmap to opal export sysfs nodes
On Fri, Mar 15, 2019 at 11:55 AM Jordan Niethe 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 > --- > 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 30125640 0003598d hdat_map 3120 0080 phandle 12e8 (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 0x3012 to 0x3016 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 >
[PATCH] powerpc/powernv: Add mmap to opal export sysfs nodes
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 --- 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; attr->private = __va(vals[0]); attr->size = vals[1]; -- 2.20.1