| Message ID | 20190315005420.20212-1-jniethe5@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | powerpc/powernv: Add mmap to opal export sysfs nodes | expand |
| 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 |
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 --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];
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(+)