Messages in this thread | | | From | Nicolai Stange <> | Subject | Re: [PATCH] debugfs: Add proxy function for the mmap file operation | Date | Fri, 29 Jul 2016 19:35:02 +0200 |
| |
Liviu Dudau <Liviu.Dudau@arm.com> writes:
> Add proxy function for the mmap file_operations hook under the > full_proxy_fops structure. This is useful for providing a custom > mmap routine in a driver's debugfs implementation.
I guess you've got some specific use case for mmap() usage on some new debugfs file in mind?
Currently, there exist only two mmap providers: drivers/staging/android/sync_debug.c kernel/kcov.c
Both don't suffer from the lack of mmap support in the debugfs full proxy implementation because they don't use it -- their files never go away and thus, can be (and are) created via debugfs_create_file_unsafe().
However, if you wish to have some mmapable debugfs file which *can* go away, introducing mmap support in the debugfs full proxy is perfectly valid. But please see below.
> Cc: Nicolai Stange <nicstange@gmail.com> > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com> > --- > fs/debugfs/file.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c > index 592059f..d87148a 100644 > --- a/fs/debugfs/file.c > +++ b/fs/debugfs/file.c > @@ -168,6 +168,10 @@ FULL_PROXY_FUNC(write, ssize_t, filp, > loff_t *ppos), > ARGS(filp, buf, size, ppos)); > > +FULL_PROXY_FUNC(mmap, int, filp, > + PROTO(struct file *filp, struct vm_area_struct *vma), > + ARGS(filp, vma)); > +
While this protects the call to ->mmap() itself against file removal races, it doesn't protect anything possibly installed at vma->vm_ops from that.
I'm fine with this as long as vma->vm_ops isn't set from ->mmap() ;). At the very least, we should probably provide a Coccinelle script for this. I'll try to put something together at the weekend or at the beginning of next week (if you aren't faster).
Another option would be to add a check in the wrapping ->mmap() whether the vma->vm_ops has been set from the wrapped ->mmap(). Greg, do you think such a runtime check would be a good thing to have?
Btw, it would certainly be possible to even support a custom vma->vm_ops by proxying this one, too. However, we probably would have to SIGSEGV userspace if ->fault() was called on a stale debugfs file. And since nobody has asked for this feature yet, I don't think that it should be implemented now.
Thanks,
Nicolai
| |