Re: [PATCH v6 3/9] landlock: Control pathname UNIX domain socket resolution by path

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

On Wed, Mar 18, 2026 at 06:52:57PM +0100, Mickaël Salaün wrote:
> On Wed, Mar 18, 2026 at 12:43:55PM -0400, Justin Suess wrote:
> > On Wed, Mar 18, 2026 at 05:26:20PM +0100, Mickaël Salaün wrote:
> > > On Wed, Mar 18, 2026 at 04:05:59PM +0100, Sebastian Andrzej Siewior wrote:
> > > > On 2026-03-18 10:14:52 [-0400], Justin Suess wrote:
> > > > > Sebastian,
> > > > Justin,
> > > > 
> > > > > In short: dom_other is a pointer to a landlock-owned refcounted struct.
> > > > …
> > > > > 
> > > > > But we copy the domain pointer, which points to a landlock allocated
> > > > > and controlled object.
> > > > 
> > > > and this is not going away while we are here and preempted after
> > > > dropping the lock? (if the landlock policy is updated/ changed/ …)
> > > 
> > > I agree with Sebastian, this is a bug, see my original proposal:
> > > https://lore.kernel.org/all/20260217.lievaS8eeng8@digikod.net/
> > Mickaël,
> > 
> > Just to make sure we're speaking of the same thing (I spotted a bug
> > shortly after replying to Sebastian).
> > 
> > This is a potential UAF if the dom_other is freed before the access
> > check takes place correct?
> 
> Yes
> 
> > 
> > 	dom_other = landlock_cred(other->sk_socket->file->f_cred)->domain;
> > 	unix_state_unlock(other);
> > 
> > 	unmask_scoped_access(subject->domain, dom_other, &layer_masks,
> > 			     fs_resolve_unix.fs);
> > 
> > If the dom_other->usage reaches zero, then the domain could be
> > freed after the unix_state_unlock while we're checking access??
> > 
> > (I guess I assumed the sock_hold on the @other would prevent the task
> > @other belongs to from being freed.)
> > 
> > Would it be better to move the access check under the unix_state_lock or
> > to acquire another reference to the ruleset (or something else)?
> 
> Because the unmask_scoped_access() only read a bounded array, it's
> simpler to unlock just after.
> 
> The other alternatives would be to use an RCU lock or a new reference
> but I don't think it's worth it.

Thank you, Sebastian, Mickaël and Justin for spotting this!

I agree, holding the existing lock across the unmask_scoped_access()
call seems like the simplest solution.  This function only walks a
previously loaded bounded memory structure, so it does not seem worth
switching to a different lock for that.

I'm changing my WIP for V7 to hold the unix_state_lock across that
function call.

–Günther