Re: [PATCH v1 1/2] landlock: Fix log_subdomains_off inheritance across fork()

["Günther Noack" <gnoack3000@gmail.com>]

On Tue, Apr 07, 2026 at 06:03:58PM +0200, Mickaël Salaün wrote:
> On Tue, Apr 07, 2026 at 09:30:40AM +0200, Günther Noack wrote:
> > On Sat, Apr 04, 2026 at 10:49:57AM +0200, Mickaël Salaün wrote:
> > > --- a/security/landlock/cred.c
> > > +++ b/security/landlock/cred.c
> > > @@ -22,10 +22,8 @@ static void hook_cred_transfer(struct cred *const new,
> > >  	const struct landlock_cred_security *const old_llcred =
> > >  		landlock_cred(old);
> > >  
> > > -	if (old_llcred->domain) {
> > > -		landlock_get_ruleset(old_llcred->domain);
> > > -		*landlock_cred(new) = *old_llcred;
> > > -	}
> > > +	landlock_get_ruleset(old_llcred->domain);
> > > +	*landlock_cred(new) = *old_llcred;
> > 
> > This fix looks correct for the hook_cred_prepare() case (and of
> > course, hook_cred_prepare() calls hook_cred_transfer() in Landlock).
> > 
> > 
> > But I'm afraid I might have spotted another issue here:
> > 
> > If I look at the code in security/keys/process_keys.c, where
> > security_tranfer_creds() is called, the "old" object is actually
> > already initialized, and if we are not checking for that, I think we
> > are leaking memory.
> 
> old is only a partially initialized credential, and the Landlock
> part is not set yet, which is the goal of hook_transfer_creds(), so
> there is no leak.

Ah, you are right.  I think we might have mixed up the names "old" and
"new" in the discussion briefly, but it's still correct - the target
credential is only partially populated and its Landlock domain is not
set, so we don't need to call landlock_put_ruleset() on it.


> > I would suggest to use the helper landlock_cred_copy() from cred.h for
> 
> This is not required but if we would like to do it anyway, that would
> not be backportable and would introduce a (minimal) performance penalty.

Fair enough, the backportability is a reasonable argument.


> > Test looks fine.
> > 
> > While I do still think we should investigate the memory leak, this
> > commit is, as it is, already a strict improvement over what we had
> > before, so:
> > 
> > Reviewed-by: Günther Noack <gnoack3000@gmail.com>
> 
> I'll keep your tag if this patch is ok with you as-is.

Yes, absolutely.

–Günther