Received-SPF: pass (google.com: domain of linux-kernel+bounces-4433-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
From: Vinicius Costa Gomes <vinicius.gomes@intel.com>
To: Christian Brauner <brauner@kernel.org>, Amir Goldstein <amir73il@gmail.com>
Cc: hu1.chen@intel.com, miklos@szeredi.hu, malini.bhandaru@intel.com,
 tim.c.chen@intel.com, mikko.ylinen@intel.com, lizhen.you@intel.com,
 linux-unionfs@vger.kernel.org, linux-kernel@vger.kernel.org, linux-fsdevel
 <linux-fsdevel@vger.kernel.org>, Linus Torvalds
 <torvalds@linux-foundation.org>, David Howells <dhowells@redhat.com>, Seth
 Forshee <sforshee@kernel.org>
Subject: Re: [RFC] HACK: overlayfs: Optimize overlay/restore creds
In-Reply-To: <20231218-intim-lehrstellen-dbe053d6c3a8@brauner>
References: <CAOQ4uxg-WvdcuCrQg7zp03ocNZoT-G2bpi=Y6nVxMTodyFAUbg@mail.gmail.com>
 <20231214220222.348101-1-vinicius.gomes@intel.com>
 <CAOQ4uxhJmjeSSM5iQyDadbj5UNjPqvh1QPLpSOVEYFbNbsjDQQ@mail.gmail.com>
 <87v88zp76v.fsf@intel.com>
 <CAOQ4uxiCVv7zbfn2BPrR9kh=DvGxQtXUmRvy2pDJ=G7rxjBrgg@mail.gmail.com>
 <CAOQ4uxhxvFt3_Wb3BGcjj4pGp=OFTBHNPJ4r4eH8245t-+CW+g@mail.gmail.com>
 <20231218-intim-lehrstellen-dbe053d6c3a8@brauner>
Date: Mon, 18 Dec 2023 13:57:31 -0800
Message-ID: <875y0vp41g.fsf@intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

Christian Brauner <brauner@kernel.org> writes:

>> > Yes, the important thing is that an object cannot change
>> > its non_refcount property during its lifetime -
>> 
>> ... which means that put_creds_ref() should assert that
>> there is only a single refcount - the one handed out by
>> prepare_creds_ref() before removing non_refcount or
>> directly freeing the cred object.
>> 
>> I must say that the semantics of making a non-refcounted copy
>> to an object whose lifetime is managed by the caller sounds a lot
>> less confusing to me.
>
> So can't we do an override_creds() variant that is effectively just:
>
> /* caller guarantees lifetime of @new */
> const struct cred *foo_override_cred(const struct cred *new)
> {
> 	const struct cred *old = current->cred;
> 	rcu_assign_pointer(current->cred, new);
> 	return old;
> }
>
> /* caller guarantees lifetime of @old */
> void foo_revert_creds(const struct cred *old)
> {
> 	const struct cred *override = current->cred;
> 	rcu_assign_pointer(current->cred, old);
> }
>
> Maybe I really fail to understand this problem or the proposed solution:
> the single reference that overlayfs keeps in ovl->creator_cred is tied
> to the lifetime of the overlayfs superblock, no? And anyone who needs a
> long term cred reference e.g, file->f_cred will take it's own reference
> anyway. So it should be safe to just keep that reference alive until
> overlayfs is unmounted, no? I'm sure it's something quite obvious why
> that doesn't work but I'm just not seeing it currently.

My read of the code says that what you are proposing should work. (what
I am seeing is that in the "optimized" cases, the only practical effect
of override/revert is the rcu_assign_pointer() dance)

I guess that the question becomes: Do we want this property (that the
'cred' associated with a subperblock/similar is long lived and the
"inner" refcount can be omitted) to be encoded in the constructor? Or do
we want it to be "encoded" in a call by call basis?

I can see both working.


Cheers,
-- 
Vinicius