Re: [PATCH] debugfs: Add proxy function for the mmap file operation

[Nicolai Stange]

Nicolai Stange <nicstange@xxxxxxxxx> writes:
> Liviu Dudau <Liviu.Dudau@xxxxxxx> 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.

Assuming that you've got such a use case, please consider resending your
patch along with the Cocci script below (and the Coccinelle team CC'ed,
of course). If OTOH your mmapable debugfs files are never removed, just
drop this message and use debugfs_create_file_unsafe() instead.


>> 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).

Here it is:

--8<---------------cut here---------------start------------->8---