netdev - Re: [RFC][PATCH net] bpf: Use mount_nodev not mount

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: <bbb5386f-b7ea-a256-5dfd-c8c8681061c6@stressinduktion.org>
Date:	Wed, 18 May 2016 22:46:57 +0200
From:	Hannes Frederic Sowa <hannes@...essinduktion.org>
To:	Daniel Borkmann <daniel@...earbox.net>,
	"Eric W. Biederman" <ebiederm@...ssion.com>
Cc:	Alexei Starovoitov <ast@...nel.org>,
	"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [RFC][PATCH net] bpf: Use mount_nodev not mount_ns to mount the
 bpf filesystem

On 18.05.2016 22:43, Daniel Borkmann wrote:
> On 05/18/2016 04:56 PM, Eric W. Biederman wrote:
>> Hannes Frederic Sowa <hannes@...essinduktion.org> writes:
>>> On 18.05.2016 01:12, Eric W. Biederman wrote:
>>>>
>>>> While reviewing the filesystems that set FS_USERNS_MOUNT I spotted the
>>>> bpf filesystem.  Looking at the code I saw a broken usage of mount_ns
>>>> with current->nsproxy->mnt_ns. As the code does not acquire a reference
>>>> to the mount namespace it can not possibly be correct to store the
>>>> mount
>>>> namespace on the superblock as it does.
>>>>
>>>> Replace mount_ns with mount_nodev so that each mount of the bpf
>>>> filesystem returns a distinct instance, and the code is not utterly
>>>> broken.
>>>>
>>>> Fixes: b2197755b263 ("bpf: add support for persistent maps/progs")
>>>> Signed-off-by: "Eric W. Biederman" <ebiederm@...ssion.com>
>>>> ---
>>>>
>>>> No one should care about this change, as userspace typically only
>>>> mounts
>>>> things once and does not depend on things in one mount do not
>>>> showing up
>>>> in another.  Can someone who actually uses the bpf filesystem please
>>>> verify this.
> [...]
> 
> LGTM.
> 
> Acked-by: Daniel Borkmann <daniel@...earbox.net>
> 
>>> The idea is to have the bpf filesystem as a singeleton per mnt-namespace
>>> to prevent endless instances being created and kernel resources being
>>> hogged by pinning them to hard to discover bpf mounts.
> 
> Eric, please send the patch officially and feel free to add my Ack. Given
> the circumstances, moving to mount_nodev() seems the best way forward. To
> also address above mentioned concern from Hannes, we need to remove the
> FS_USERNS_MOUNT flag along with the change. It looks like the fix is best
> addressed in a single patch if you want to include it. If not, we can
> otherwise send it separately as well, I don't mind.

I agree. Would make most sense to make the change in one patch. Later on
we can reason about if it makes sense to use the net namespace to split
bpf maps and programs or maybe even introduce a new primitive for that.

Thanks,
Hannes