linux-kernel - Re: [PATCH 2/2] acct: block access to kernel internal filesystems

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: <20250212-summen-vergibt-6c6562c0b0bd@brauner>
Date: Wed, 12 Feb 2025 11:32:35 +0100
From: Christian Brauner <brauner@...nel.org>
To: Al Viro <viro@...iv.linux.org.uk>
Cc: Zicheng Qu <quzicheng@...wei.com>, 
	Linus Torvalds <torvalds@...ux-foundation.org>, jlayton@...nel.org, axboe@...nel.dk, joel.granados@...nel.org, 
	tglx@...utronix.de, hch@....de, len.brown@...el.com, pavel@....cz, 
	pengfei.xu@...el.com, rafael@...nel.org, tanghui20@...wei.com, zhangqiao22@...wei.com, 
	judy.chenhui@...wei.com, linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org, 
	syzkaller-bugs@...glegroups.com, linux-pm@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH 2/2] acct: block access to kernel internal filesystems

On Tue, Feb 11, 2025 at 08:54:18PM +0000, Al Viro wrote:
> On Tue, Feb 11, 2025 at 06:16:00PM +0100, Christian Brauner wrote:
> > There's no point in allowing anything kernel internal nor procfs or
> > sysfs.
> 
> > +	/* Exclude kernel kernel internal filesystems. */
> > +	if (file_inode(file)->i_sb->s_flags & (SB_NOUSER | SB_KERNMOUNT)) {
> > +		kfree(acct);
> > +		filp_close(file, NULL);
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* Exclude procfs and sysfs. */
> > +	if (file_inode(file)->i_sb->s_iflags & SB_I_USERNS_VISIBLE) {
> > +		kfree(acct);
> > +		filp_close(file, NULL);
> > +		return -EINVAL;
> > +	}
> 
> That looks like a really weird way to test it, especially the second
> part...

SB_I_USERNS_VISIBLE has only ever applied to procfs and sysfs.

Granted, it's main purpose is to indicate that a caller in an
unprivileged userns might have a restricted view of sysfs/procfs already
so mounting it again must be prevented to not reveal any overmounted
entities (A Strong candidate for the price of least transparent cause of
EPERMs from the kernel imho.).

That flag could reasonably go and be replaced by explicit checks for
procfs and sysfs in general because we haven't ever grown any additional
candidates for that mess and it's unlikely that we ever will. But as
long as we have this I don't mind using it. If it's important to you
I'll happily change it. If you can live with the comment I added I'll
leave it.

To be perfectly blunt: Imho, this api isn't worth massaging a single
line of VFS code which is why this isn't going to win the price of
prettiest fix of a NULL-deref.