Re: [PATCH 1/4] ns: add bpf hooks

From: Christian Brauner

Date: Mon Mar 02 2026 - 04:49:36 EST


On Fri, Feb 27, 2026 at 08:38:48AM -0800, Song Liu wrote:
> On Fri, Feb 27, 2026 at 2:28 AM Christian Brauner <brauner@xxxxxxxxxx> wrote:
> [...]
> > >
> > > If we change the hook as
> > >
> > > bpf_lsm_namespace_alloc(ns, inum);
> > >
> > > We can move it to the beginning of __ns_common_init().
> > > This change allows blocking __ns_common_init() before
> > > it makes any changes to the ns. Is this a better approach?
> >
> > I don't think it matters tbh. We have no control when exactly
> > __ns_common_init() is called. That's up to the containing namespace. We
> > can't rely on the namespace to have been correctly set up at this time.
> > My main goal was to have struct ns_common to be fully initialized
> > already so that direct access to it's field already makes sense.
>
> Good point on having ns_common initialized. Besides inum, we
> should also pass ns_type and ops into the hook.

But why? The struct ns_common is already fully initialized when it is
passed to bpf_lsm_namespace_alloc() including ops, inum, ns_type etc.

>
> OTOH, shall we have the hook before proc_alloc_inum()? With
> this change, the hook can block the operation before it causes
> any contention on proc_inum_ida. IOW, how about we have:

I think that contention is meaningless and I'd rather have struct
ns_common fully set up so that all fields can be accessed.

>
> @@ -71,6 +71,10 @@ int __ns_common_init(struct ns_common *ns, u32
> ns_type, const struct proc_ns_ope
> ns_debug(ns, ops);
> #endif
>
> + ret = bpf_lsm_namespace_alloc(ns, inum);
> + if (ret)
> + return ret;
> +
> if (inum)
> ns->inum = inum;
> else
>
> With this change, ns is already initialized, except the inum.
>
> WDYT?
>
> Thanks,
> Song
>
> > The containing namespace my already have to rollback a bunch of stuff
> > anyway.