public inbox for linux-fsdevel@vger.kernel.org
 help / color / mirror / Atom feed
From: Justin Suess <utilityemal77@gmail.com>
To: "Mickaël Salaün" <mic@digikod.net>
Cc: Song Liu <song@kernel.org>,
	ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
	kpsingh@kernel.org, paul@paul-moore.com, viro@zeniv.linux.org.uk,
	brauner@kernel.org, kees@kernel.org, gnoack@google.com,
	jack@suse.cz, jmorris@namei.org, serge@hallyn.com,
	yonghong.song@linux.dev, martin.lau@linux.dev, m@maowtm.org,
	eddyz87@gmail.com, john.fastabend@gmail.com, sdf@fomichev.me,
	skhan@linuxfoundation.org, bpf@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [RFC PATCH 08/20] bpf: Add Landlock ruleset map type
Date: Fri, 17 Apr 2026 12:51:40 -0400	[thread overview]
Message-ID: <aeJlHIoSjVPmccDx@suesslenovo> (raw)
In-Reply-To: <20260417.ohgoh0Eecome@digikod.net>

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.

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)

> > 
> > 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)

> 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.
> 
> > 
> > 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.
> > 

  parent reply	other threads:[~2026-04-17 16:51 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-07 20:01 [RFC PATCH 00/20] BPF interface for applying Landlock rulesets Justin Suess
2026-04-07 20:01 ` [RFC PATCH 01/20] landlock: Move operations from syscall into ruleset code Justin Suess
2026-04-07 20:01 ` [RFC PATCH 02/20] execve: Add set_nnp_on_point_of_no_return Justin Suess
2026-04-07 20:01 ` [RFC PATCH 03/20] landlock: Implement LANDLOCK_RESTRICT_SELF_NO_NEW_PRIVS Justin Suess
2026-04-07 20:01 ` [RFC PATCH 04/20] selftests/landlock: Cover LANDLOCK_RESTRICT_SELF_NO_NEW_PRIVS Justin Suess
2026-04-07 20:01 ` [RFC PATCH 05/20] landlock: Make ruleset deferred free RCU safe Justin Suess
2026-04-07 20:01 ` [RFC PATCH 06/20] bpf: lsm: Add Landlock kfuncs Justin Suess
2026-04-07 20:01 ` [RFC PATCH 07/20] bpf: arraymap: Implement Landlock ruleset map Justin Suess
2026-04-07 20:01 ` [RFC PATCH 08/20] bpf: Add Landlock ruleset map type Justin Suess
2026-04-16 21:12   ` Song Liu
2026-04-16 21:53     ` Justin Suess
2026-04-16 23:47       ` Song Liu
2026-04-17 14:09         ` Justin Suess
2026-04-17 15:18           ` Mickaël Salaün
2026-04-17 16:10             ` Song Liu
2026-04-17 18:01               ` Mickaël Salaün
2026-04-17 16:51             ` Justin Suess [this message]
2026-04-17 18:03               ` Mickaël Salaün
2026-04-17 16:01           ` Song Liu
2026-04-07 20:01 ` [RFC PATCH 09/20] bpf: syscall: Handle Landlock ruleset maps Justin Suess
2026-04-07 20:01 ` [RFC PATCH 10/20] bpf: verifier: Add Landlock ruleset map support Justin Suess
2026-04-07 20:01 ` [RFC PATCH 11/20] selftests/bpf: Add Landlock kfunc declarations Justin Suess
2026-04-07 20:01 ` [RFC PATCH 12/20] selftests/landlock: Rename gettid wrapper for BPF reuse Justin Suess
2026-04-07 20:01 ` [RFC PATCH 13/20] selftests/bpf: Enable Landlock in selftests kernel Justin Suess
2026-04-07 20:01 ` [RFC PATCH 14/20] selftests/bpf: Add Landlock kfunc test program Justin Suess
2026-04-07 20:01 ` [RFC PATCH 15/20] selftests/bpf: Add Landlock kfunc test runner Justin Suess
2026-04-07 20:01 ` [RFC PATCH 16/20] landlock: Bump ABI version Justin Suess
2026-04-07 20:01 ` [RFC PATCH 17/20] tools: bpftool: Add documentation for landlock_ruleset Justin Suess
2026-04-07 20:01 ` [RFC PATCH 18/20] landlock: Document LANDLOCK_RESTRICT_SELF_NO_NEW_PRIVS Justin Suess
2026-04-07 20:01 ` [RFC PATCH 19/20] bpf: Document BPF_MAP_TYPE_LANDLOCK_RULESET Justin Suess
2026-04-07 20:01 ` [RFC PATCH 20/20] MAINTAINERS: update entry for the Landlock subsystem Justin Suess
2026-04-08  4:40 ` [RFC PATCH 00/20] BPF interface for applying Landlock rulesets Ihor Solodrai
2026-04-08 11:41   ` Justin Suess
2026-04-08 14:00 ` Mickaël Salaün
2026-04-08 17:10   ` Justin Suess
2026-04-08 19:21     ` Mickaël Salaün
2026-04-10 12:43       ` Justin Suess
2026-04-13 15:06       ` Justin Suess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aeJlHIoSjVPmccDx@suesslenovo \
    --to=utilityemal77@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=brauner@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=gnoack@google.com \
    --cc=jack@suse.cz \
    --cc=jmorris@namei.org \
    --cc=john.fastabend@gmail.com \
    --cc=kees@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=m@maowtm.org \
    --cc=martin.lau@linux.dev \
    --cc=mic@digikod.net \
    --cc=paul@paul-moore.com \
    --cc=sdf@fomichev.me \
    --cc=serge@hallyn.com \
    --cc=skhan@linuxfoundation.org \
    --cc=song@kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox