lkml.org 
[lkml]   [2016]   [Jul]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] debugfs: Add proxy function for the mmap file operation
Date
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

\
 
 \ /
  Last update: 2016-07-29 20:21    [W:0.070 / U:1.300 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site