Re: [RFC PATCH 08/20] bpf: Add Landlock ruleset map type

["Mickaël Salaün" <mic@digikod.net>]

On Fri, Apr 17, 2026 at 12:51:40PM -0400, Justin Suess wrote:
> On Fri, Apr 17, 2026 at 05:18:05PM +0200, Mickaël Salaün wrote:
> > On Fri, Apr 17, 2026 at 10:09:13AM -0400, Justin Suess wrote:
> > > On Thu, Apr 16, 2026 at 04:47:40PM -0700, Song Liu wrote:
> > > > On Thu, Apr 16, 2026 at 2:53 PM Justin Suess <utilityemal77@gmail.com> wrote:
> > > > [...]
> > > > > I don't think we can pass the FD number via a map, since the FD is
> > > > > process specific. And it needs to be done in a way where we can lookup
> > > > > the specific ruleset the FD points to safely.
> > > > >
> > > > > So we'd need some other way to load the ruleset from a file descriptor,
> > > > > either through a new userspace side BPF call or similar mechanism.
> > > > >
> > > > > Is there some other common pattern for FDs --> kptr I can follow?
> > > > 
> > > > I didn't find an exact example like this. There must be a way to achieve
> > > > this. In the worst case, we can add a kfunc for this.
> > > >
> > > 
> > > I think new kfunc is a doable approach. I could make a kfunc taking a struct
> > > *task_struct and an FD that looks up a landlock ruleset within a given
> > > task that returns a trusted kptr.
> > > 
> > > Something like:
> > > 
> > > struct bpf_landlock_ruleset* bpf_landlock_get_ruleset_from_fd(struct
> > > task_struct* task, int fd)
> 
> Thanks Mickaël and Song,
> 
> There are definitely pros and cons to both approaches.
> 
> I think it would be OK to have a dedicated map and indeed cleaner from
> the userspace side since there would be no intermediate step to find the
> task and lookup the fd since that would be handled by the map_ops.
> 
> Cons of the new map type:
> 
> The main issue is with the new map type is then we are limited to the
> specific data structure we define in the map. For instance if we want
> to use a hash or other data structure instead of an array to store
> rulesets, we'd need to define variants of the landlock map type for
> all data structures.
> 
> So this kind of bungles the "data structure" and "data type" layers.
> 
> Pros of the new map type:
> 
> The ruleset_fd conversion would be implicitly handled by the map_ops.
> Userspace could insert the fd and bpf would not have to deal with it at
> all.
> 
> Cons of bpf_landlock_get_ruleset_from_fd:
> 
> Awkward conversion step. We need to find the task of the original
> ruleset creator and recieve the fd before looking it up and converting
> it to a kptr to the bpf_landlock_ruleset.
> 
> Pros of bpf_landlock_get_ruleset_from_fd:
> 
> We can use any existing map data structure to store our kptrs.
> 
> Not having a dedicated map type simplifies implementation.
> 
> ...
> 
> Appreciate the feedback from both of you.
> > 
> > That looks like a hack that would not handle FD's (object) lifetime
> > (e.g. what happen when the task is gone?).
> >
> 
> If we take an underlying reference on the ruleset backing the fd, then the fd
> being closed shouldn't matter right?
> 
> This is how the lifetime management works for that
> bpf_landlock_get_ruleset_from fd, in my draft implementation prior to
> the RFC:
> 
>   /**
>    * bpf_landlock_get_ruleset_from_fd - acquire a Landlock ruleset from a task FD
>    * @task: task owning the file descriptor table to look up
>    * @fd: Landlock ruleset file descriptor in @task
>    *
>    * Returns: a referenced opaque Landlock ruleset, or NULL if the FD lookup or
>    * validation fails.
>    */
>   __bpf_kfunc struct bpf_landlock_ruleset *
>   bpf_landlock_get_ruleset_from_fd(struct task_struct *task, int fd)
>   {
>   	struct landlock_ruleset *ruleset;
>     /* does landlock_get_ruleset and increments refcount */
>   	ruleset = landlock_get_task_ruleset_from_fd(task, fd, FMODE_CAN_READ);
>   	if (IS_ERR(ruleset))
>   		return NULL;
>   
>   	return (struct bpf_landlock_ruleset *)ruleset;
>   }
> 
> The landlock_get_task_ruleset_from_fd increments the usage with
> landlock_get_ruleset. (This may sleep, so it must be tagged with
> KF_SLEEPABLE)
> 
> If the fd is closed before landlock_get_ruleset_from_fd, then null is returned.
> The verifier will force the program to do the null check b/c of KF_RET_NULL.
> 
> __kptr also have destructor kfuncs. So if we get the reference in
> bpf_landlock_get_ruleset_from_fd with landlock_get_ruleset_from_fd, and
> put the reference to the ruleset.
> 
> The destructor path looks like this:
> 
>   /* Define ID for destructor * /
>   BTF_ID_LIST(bpf_landlock_dtor_ids)
>   BTF_ID(struct, bpf_landlock_ruleset)
>   BTF_ID(func, bpf_landlock_put_ruleset_dtor)
>   ...
>   /**
>   * bpf_landlock_put_ruleset - put a Landlock ruleset
>   * @ruleset: Landlock ruleset to put
>   */
>   __bpf_kfunc void
>   bpf_landlock_put_ruleset(const struct bpf_landlock_ruleset *ruleset)
>   {
> 	  landlock_put_ruleset((struct landlock_ruleset *)ruleset);
>   }
> 
>   __bpf_kfunc void bpf_landlock_put_ruleset_dtor(void *ruleset)
>   {
> 	  bpf_landlock_put_ruleset(ruleset);
>   }
> 
>   static int __init bpf_landlock_kfunc_init(void)
>   {
>   	const struct btf_id_dtor_kfunc bpf_landlock_dtors[] = {
>   		{
>   			.btf_id = bpf_landlock_dtor_ids[0],
>   			.kfunc_btf_id = bpf_landlock_dtor_ids[1],
>   		},
>   	};
>   	int ret;
>   
>   	ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_LSM,
>   					&bpf_landlock_kfunc_set);
>   	if (ret)
>   		return ret;
>   
>   	return register_btf_id_dtor_kfuncs(bpf_landlock_dtors,
>   					   ARRAY_SIZE(bpf_landlock_dtors),
>   					   THIS_MODULE);
>   }
> 
> Good reminder I need to include a test making sure the ruleset
> remains valid after the FD and/or task is closed. :)
> 
> > Why not using proper typing with a dedicated map?
> > 
> 
> I may be misunderstanding, but from what I see, a __kptr DOES give
> proper typing, __kptr is an annotation not a type.

Ok, good.

> 
> This is what it would look like in an BPF_MAP_TYPE_ARRAY.
> 
>     struct ruleset_kptr_value {
> 	    struct bpf_landlock_ruleset __kptr * ruleset;
>     };
> 
>     struct {
> 	      __uint(type, BPF_MAP_TYPE_ARRAY);
> 	      __uint(max_entries, 1);
> 	      __type(key, __u32);
> 	      __type(value, struct ruleset_kptr_value);
>     } ruleset_kptr_map SEC(".maps");
> 
> So we get proper typing from what I see. (It's not like a __kptr is a
> special void*, it has a type)

Looks good.

> 
> > > 
> > > And tagging it with KF_ACQUIRE + KF_RET_NULL.
> > > 
> > > Then keep the existing kfunc for putting the ruleset and enforcing it on
> > > a struct linux_binprm.
> > > 
> > > The BPF program would need to get a reference to a task struct
> > > of the program creating the rulesets with bpf_task_from_pid for
> > > instance. Then they could use the task_struct with another plain integer
> > > map to store FD numbers and then use the rulesets or store them in a map
> > > of __kptr objects for later usage.
> > > 
> > > Would this be more acceptable?
> > > > > Basically the pattern I need is userspace must create the file
> > > > > descriptor, BPF converts that FD into a refcounted kernel object, and
> > > > > even if userspace closes the FD BPF needs to hold a reference on the
> > > > > underlying ruleset structure.
> > > > >
> > > > > (In this patch this was accomplished through the map_ops)
> > > > >
> > > > > Let me know what you think Song. I do understand the benefit of having a
> > > > > __kptr instead, the refcounting is all there, and it would allow storing
> > > > > rulesets in multiple map types. (and one less map type to maintain).
> > > > 
> > > > A new type of map for each FD referenced kernel type is non-starter.
> > > > It is impossible to add UAPI for a specific use case.
> > 
> > This new map type is only about one file descriptor type, similarly to
> > socket FDs.  From a UAPI point of view, it looks clean and safe,
> > especially to deal with underlying object lifetime (e.g. reference
> > tracking).
> > 
> > > >
> > > You've convinced me. I could see a lot of problems if everyone wanting
> > > to add their specialized maps, it would be difficult to maintain.
> > 
> > Is there another way to properly handle kernel object lifetime (not tied
> 
> The answer the the lifetime part is yes.
> 
> The kptr destructors and the landlock ruleset refcounting give us that
> abstraction. (along with the KF_ACQUIRE/KF_RELEASE annotations and
> destructor implementation)

Good.

> 
> > to the caller) and pass them as file descriptor?
> This "pass them as a file descriptor" is the tricky part. It would be
> very convenient if we could send the fd to bpf from userspace and have
> it be implicitly converted (like in the BPF_MAP_TYPE_LANDLOCK_RULESET
> implementation) in one step, but I just don't see a way to do that with
> the bpf_landlock_get_ruleset_from_fd kfunc approach.

Song's idea to have a generic FD map looks promising.

> > 
> > > 
> > > It's probably best to keep the specialized map types to core kernel
> > > interfaces only that are unlikely to change.
> > 
> > File descriptors are a stable interface.
> >
> No that's correct. I cede that point :)
> > > 
> > > > Thanks,
> > > > Song
> > > > 
> > > > > Mickaël, do you have any thoughts on this? I have v2 basically ready,
> > > > > although it uses the BPF_MAP_TYPE_LANDLOCK_RULESET it changes a lot on
> > > > > the Landlock side.
> > > 
>