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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8760rfzczy.fsf@gmail.com>
Date:	Fri, 05 Aug 2016 13:11:45 +0200
From:	Nicolai Stange <nicstange@...il.com>
To:	Brian Starkey <brian.starkey@....com>
Cc:	Nicolai Stange <nicstange@...il.com>,
	Liviu Dudau <Liviu.Dudau@....com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] debugfs: Add proxy function for the mmap file operation

Brian Starkey <brian.starkey@....com> writes:

> On Tue, Aug 02, 2016 at 07:31:36PM +0200, Nicolai Stange wrote:
>>Nicolai Stange <nicstange@...il.com> writes:
>>> 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.
>
> So we do have an implementation using this, but it's likely we will
> keep it out-of-tree (it's a stop-gap until we can get a non-debugfs
> implementation of the functionality into mainline).
>
> Do you think it's worth merging this (and your cocci script) anyway to
> save someone else doing the same thing later?

I personally think that having ->mmap() support in debugfs would be a
good thing to have in general and I expect there to be some further
demand in the future.

But I also think that it is a little bit fragile in the current state:
how many people actually run the Cocci scripts on their changes? AFAICT,
even the kbuild test robot doesn't do this. And after all, the Cocci
script I provided could very well miss some obfuscated writes to
vma->vm_ops: if they aren't done from ->mmap() themselves, but from some
helper function invoked therein, for example.

I would personally prefer a hand coded full_proxy_mmap() which WARN()s
if the proxied ->mmap() changes vma->vm_ops:
- this would add an extra safety net
- ->mmap() for debugfs files isn't performance critical
- and lastly, we're already doing something similar to this in
  open_proxy_open().

But in the end, it's not mine but Greg K-H's opinion that matters here...

Thanks,

Nicolai