linux-kernel - Re: [GIT PULL] overlayfs updates for 6.13

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20241123-bauhof-tischbein-579ff1db831a@brauner>
Date: Sat, 23 Nov 2024 13:06:14 +0100
From: Christian Brauner <brauner@...nel.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Amir Goldstein <amir73il@...il.com>, 
	Miklos Szeredi <miklos@...redi.hu>, linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org, 
	linux-unionfs@...r.kernel.org
Subject: Re: [GIT PULL] overlayfs updates for 6.13

On Fri, Nov 22, 2024 at 09:21:58PM -0800, Linus Torvalds wrote:
> On Fri, 22 Nov 2024 at 01:57, Amir Goldstein <amir73il@...il.com> wrote:
> >
> > - Introduction and use of revert/override_creds_light() helpers, that were
> >   suggested by Christian as a mitigation to cache line bouncing and false
> >   sharing of fields in overlayfs creator_cred long lived struct cred copy.
> 
> So I don't actively hate this, but I do wonder if this shouldn't have
> been done differently.
> 
> In particular, I suspect *most* users of override_creds() actually
> wants this "light" version, because they all already hold a ref to the
> cred that they want to use as the override.
> 
> We did it that safe way with the extra refcount not because most
> people would need it, but it was expected to not be a big deal.
> 
> Now you found that it *is* a big deal, and instead of just fixing the
> old interface, you create a whole new interface and the mental burden
> of having to know the difference between the two.

> So may I ask that you look at perhaps just converting the (not very
> many) users of the non-light cred override to the "light" version?

I think that could be a good idea in general.

But I have to say I'm feeling a bit defensive after having read your
message even though I usually try not to. :) 

So just to clarify when that issue was brought up I realized that the
cred bump was a big deal for overlayfs but from a quick grep I didn't
think for any of the other cases it really mattered that much.

Realistically, overlayfs is the prime example where that override cred
matters big time because it's called everywhere and in all core
operations one can think of. But so far I at least haven't heard
complaints outside of that and so the immediate focus was to bring about
a solution for overlayfs.

The reason the revert_creds_light() variant doesn't return the old creds
is so that callers don't put_cred() them blindly.

Because for overlayfs (and from a quick glance io_uring and nfs) the
refcount for the temporary creds is kept completely independent of the
callsites.

The lifetime is bound to the superblock and so the final put on the
temporary creds has nothing to do with the callers at all.