Linux Security Modules development
 help / color / mirror / Atom feed
* Re: [PATCH v1] security/safesetid: fix comment and error handling
From: Micah Morton @ 2026-03-18 20:49 UTC (permalink / raw)
  To: yanwei.gao; +Cc: paul, linux-security-module
In-Reply-To: <20260303024025.37916-1-gaoyanwei.tx@gmail.com>

On Mon, Mar 2, 2026 at 6:40 PM yanwei.gao <gaoyanwei.tx@gmail.com> wrote:
>
> - Fix comment in lsm.c: use CAP_SETGID instead of CAP_SETUID in the
>   GID capability check comment to match the actual logic.
> - In handle_policy_update(), set err = -EINVAL and goto out_free_buf
>   when policy type is neither UID nor GID, so the error is returned
>   to the caller instead of only logging.
> - In safesetid_init_securityfs(), return ret directly when
>   policy_dir creation fails instead of goto error (no cleanup needed
>   at that point).
>
> Signed-off-by: yanwei.gao <gaoyanwei.tx@gmail.com>
> ---
>  security/safesetid/lsm.c        | 2 +-
>  security/safesetid/securityfs.c | 4 +++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
> index d5fb949050dd..a7b68e65996c 100644
> --- a/security/safesetid/lsm.c
> +++ b/security/safesetid/lsm.c
> @@ -128,7 +128,7 @@ static int safesetid_security_capable(const struct cred *cred,
>                 if (setid_policy_lookup((kid_t){.gid = cred->gid}, INVALID_ID, GID) == SIDPOL_DEFAULT)
>                         return 0;
>                 /*
> -                * Reject use of CAP_SETUID for functionality other than calling
> +                * Reject use of CAP_SETGID for functionality other than calling
Looks good.
>                  * set*gid() (e.g. setting up userns gid mappings).
>                  */
>                 pr_warn("Operation requires CAP_SETGID, which is not available to GID %u for operations besides approved set*gid transitions\n",
> diff --git a/security/safesetid/securityfs.c b/security/safesetid/securityfs.c
> index a71e548065a9..50682abd342b 100644
> --- a/security/safesetid/securityfs.c
> +++ b/security/safesetid/securityfs.c
> @@ -224,6 +224,8 @@ static ssize_t handle_policy_update(struct file *file,
>         } else {
>                 /* Error, policy type is neither UID or GID */
>                 pr_warn("error: bad policy type");
> +               err = -EINVAL;
> +               goto out_free_buf;

This makes sense to me and matches how things are done in the
functions above in verify_ruleset() and parse_policy_line().

Is this code actually reachable? I guess if verify_ruleset returns
-EINVAL due to a bad policy type value the code still hits this spot?

>         }
>         err = len;
>
> @@ -321,7 +323,7 @@ int __init safesetid_init_securityfs(void)
>         policy_dir = securityfs_create_dir("safesetid", NULL);
>         if (IS_ERR(policy_dir)) {
>                 ret = PTR_ERR(policy_dir);
> -               goto error;
> +               return ret;

We still need the `securityfs_remove(policy_dir)` call to clean up the
dir created with `policy_dir = securityfs_create_dir("safesetid",
NULL)` don't we?

>         }
>
>         uid_policy_file = securityfs_create_file("uid_allowlist_policy", 0600,
> --
> 2.43.5
>

^ permalink raw reply

* Re: [PATCH RFC bpf-next 0/4] audit: Expose audit subsystem to BPF LSM programs via BPF kfuncs
From: Alexei Starovoitov @ 2026-03-18 20:55 UTC (permalink / raw)
  To: Frederick Lawler
  Cc: Kumar Kartikeya Dwivedi, Paul Moore, James Morris,
	Serge E. Hallyn, Eric Paris, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Shuah Khan, Mickaël Salaün,
	Günther Noack, LKML, LSM List, audit, bpf,
	open list:KERNEL SELFTEST FRAMEWORK, kernel-team
In-Reply-To: <abrlvEjQKUdAOcw5@CMGLRV3>

On Wed, Mar 18, 2026 at 10:49 AM Frederick Lawler <fred@cloudflare.com> wrote:
>
> Hi Alexei,
>
> On Tue, Mar 17, 2026 at 06:15:59PM -0700, Alexei Starovoitov wrote:
> > On Mon, Mar 16, 2026 at 7:44 PM Kumar Kartikeya Dwivedi
> > <memxor@gmail.com> wrote:
> > >
> > > On Wed, 11 Mar 2026 at 22:31, Frederick Lawler <fred@cloudflare.com> wrote:
> > > >
> > > > The motivation behind the change is to give BPF LSM developers the
> > > > ability to report accesses via the audit subsystem much like how LSMs
> > > > operate today.
> >
> > Sure, but bpf lsm-s don't need to follow such conventions.
> > audit is nothing but a message passing from kernel to user space
> > and done in a very inefficient way by wrapping strings into skb/netlink.
> > bpf progs can do this message passing already via various ways:
> > perfbuf, ringbuf, streams.
> > Teach your user space to consume one of them.
>
> Audit provides additional functionality by keeping messages close to the
> syscall, and in-line with other messages for that syscall. Aside from
> that, BPF is arguably too flexible. I'm sure it's already understood,
> but the idea here is to reduce bespoke logging implementations and reduce
> attack surface for violation reporting. Audit is already provided by the
> kernel and a well leveraged pipeline.

Sorry, I don't buy any of these arguments.
Nack.

^ permalink raw reply

* Re: linux-next: Tree for Mar 18 (landlock docs)
From: Randy Dunlap @ 2026-03-18 22:12 UTC (permalink / raw)
  To: Mark Brown, Linux Next Mailing List
  Cc: Linux Kernel Mailing List, Dan Cojocaru, Mickaël Salaün,
	git, linux-security-module
In-Reply-To: <abrYTGOZRH9oP2GQ@sirena.org.uk>



On 3/18/26 9:52 AM, Mark Brown wrote:
> Hi all,
> 
> Changes since 20260317:
> 

linux-next/Documentation/userspace-api/landlock.rst:198: ERROR: Error in "code-block" directive:
maximum 1 argument(s) allowed, 105 supplied.
^^^^^^^^^^^^^^^^^^^^^^^^^^

.. code-block:: c
    __u32 restrict_flags =


Please insert a blank line between ".. code-block:: c"
and the code.


d37d3347aa59 ("landlock: Expand restrict flags example for ABI version 8")

-- 
~Randy


^ permalink raw reply

* Re: [PATCH v2 1/2] kthread: remove kthread_exit()
From: Steven Rostedt @ 2026-03-18 23:12 UTC (permalink / raw)
  To: David Laight
  Cc: Christian Brauner, Linus Torvalds, linux-kernel, linux-modules,
	linux-nfs, bpf, kunit-dev, linux-doc, linux-trace-kernel, netfs,
	io-uring, audit, rcu, kvm, virtualization, netdev, linux-mm,
	linux-security-module, Christian Loehle, linux-fsdevel
In-Reply-To: <20260311104736.51b53405@pumpkin>

On Wed, 11 Mar 2026 10:47:36 +0000
David Laight <david.laight.linux@gmail.com> wrote:

> > -#define module_put_and_kthread_exit(code) kthread_exit(code)
> > +#define module_put_and_kthread_exit(code) do_exit(code)  
> 
> I'm intrigued...
> How does that actually know to do the module_put()?
> (I know it does one - otherwise my driver wouldn't unload.)

It's in the !CONFIG_MODULES section. No module_put() necessary. Only the
kthread_exit (do_exit) is needed.

-- Steve

^ permalink raw reply

* Re: [PATCH v6] backing_file: store user_path_file
From: Paul Moore @ 2026-03-18 23:46 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Christian Brauner, Miklos Szeredi, Gao Xiang,
	linux-security-module, selinux, linux-erofs, linux-fsdevel,
	linux-unionfs
In-Reply-To: <20260318131258.1457101-1-amir73il@gmail.com>

On Wed, Mar 18, 2026 at 9:13 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> Instead of storing the user_path, store an O_PATH file for the
> user_path with the original user file creds and a security context.
>
> The user_path_file is only exported as a const pointer and its refcnt
> is initialized to FILE_REF_DEAD, because it is not a refcounted object.
>
> The file_ref_init() helper was changed to accept the FILE_REF_ constant
> instead of the fake +1 integer count.
>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>
> Christian,
>
> My v5 patch was sent by Paul along with his LSM/selinux pataches [1].
> Here are the changes you requested.
>
> I removed the ACKs and Tested-by because of the changes.
>
> Thanks,
> Amir.
>
> Changes since v5:
> - Restore file_ref_init() helper without refcnt -1 offset
> - Future proofing errors from backing_file_open_user_path()
>
> [1] https://lore.kernel.org/r/20260316213606.374109-6-paul@paul-moore.com/
>
>  fs/backing-file.c            | 26 ++++++++++--------
>  fs/erofs/ishare.c            | 13 +++++++--
>  fs/file_table.c              | 53 ++++++++++++++++++++++++++++--------
>  fs/fuse/passthrough.c        |  3 +-
>  fs/internal.h                |  5 ++--
>  fs/overlayfs/dir.c           |  3 +-
>  fs/overlayfs/file.c          |  1 +
>  include/linux/backing-file.h | 29 ++++++++++++++++++--
>  include/linux/file_ref.h     |  4 +--
>  9 files changed, 103 insertions(+), 34 deletions(-)

Still works for me.  I'm going to update lsm/stable-7.0 with this
patch so we can get some more linux-next testing.

Tested-by: Paul Moore <paul@paul-moore.com>

-- 
paul-moore.com

^ permalink raw reply

* 社長のNG行動
From: 神田/ナレッジリンク @ 2026-03-18 23:40 UTC (permalink / raw)
  To: linux-security-module

 お世話になります。ナレッジリンクセミナー事務局です。
 
 
 「社員のモチベーションを上げようと声をかける」
 「部下の相談に乗り、一緒に悩んであげる」
 「現場のトラブルに、自ら先頭に立って対応する」
 
 もし社長がこれらを率先しているようであれば、
 残念ながら、その組織の成長はそこで止まります。
 
 社長のその“優しさ”が、社員の甘えを生み、責任感を奪い、
 「指示待ち人間」を量産する装置になっているからです。
 
 
 4,800社の経営者が衝撃を受けた、
 良かれと思ってやってしまう「社長のNG行動」の正体。
 
 組織を劇的に変えるための、
 オンラインセミナーを開催いたします。
 
 1つでも心当たりがあれば、一度ご視聴ください。 
 
 >>視聴予約はこちら
 https://knowledge-corp.jp/shikigaku5/
 
----------------------------------------------
 
 テーマ : それ、危険です 『社長のNG行動』
      〜 その行動が、組織崩壊を招く 〜
 

 日 程 : 3月19日(木)13:00〜15:00 残り14枠
       4月14日(火)13:00&#12316;15:00
       4月21日(火)13:00&#12316;15:00
     ※どちらの日程も内容は同じ
 会 場 :Zoom開催
 定 員 :先着100名(費用は不要)
----------------------------------------------
 ※経営層の方限定です
 
 
 なぜ、社員は「言われたこと」しかやらないのか。
 なぜ、次世代のリーダー候補が育たないのか。
 
 それは、能力の問題ではなく、
 
 良かれと思って続けている「社長の配慮」こそが、
 組織成長を止める、最大のボトルネックかもしれません。
 
 本セミナーでは、4,800社以上が導入した
 独自の組織論「識学」に基づき、社長の「NG行動」と
 真の経営者へ脱皮するためのマインドセットを伝授します。
 
 
 【セミナー内容(一部抜粋)】
  ○ NG行動3選
  ○ なぜ優秀なNo.2や部長が育たないのか
  ○ マネジメントスタイルの変革について
  ○ 導入企業の事例
 
 「管理職が育ったら任せる」ではなく「任せるから育つ」
 という思考の逆転を提言。
 
 現場から「冷たくなった」と思われることを恐れず、
 機能的な階層構造(仕組み)を作ることで

 結果として社員全員を守り、
 利益を最大化させる道筋を明示します。

 「優しさ」で人を動かすのではなく、
 「正しさ」で組織を動かす。

 音声やお顔が表に出ることはございませんので
 お気軽にご視聴ください。
 
 >>視聴予約はこちら
 https://knowledge-corp.jp/shikigaku5/
 
 
-----------------------
 一般社団法人 ナレッジリンク
 東京都千代田区神田小川町1-8-3
 電話:03-5256-7638

 セミナーのご案内が不要な方は大変残念ではございますが、
 下記URLより手続き下さいませ。
 
 メール配信のワンクリック解除はこちら
 https://fc-knowledgelink-corp.jp/mail/
 

^ permalink raw reply

* Re: [PATCH v2 7/10] security: Hornet LSM
From: Blaise Boscaccy @ 2026-03-19  0:18 UTC (permalink / raw)
  To: Paul Moore, Jonathan Corbet, James Morris, Serge E. Hallyn,
	Mickaël Salaün, Günther Noack,
	Dr. David Alan Gilbert, Andrew Morton, James.Bottomley, dhowells,
	Fan Wu, Ryan Foster, linux-security-module, linux-doc,
	linux-kernel, bpf
In-Reply-To: <a4ffe6eac1f9963e1bb092b4a6096154@paul-moore.com>

Paul Moore <paul@paul-moore.com> writes:

> On Feb 27, 2026 Blaise Boscaccy <bboscaccy@linux.microsoft.com> wrote:
>> 
>> This adds the Hornet Linux Security Module which provides enhanced
>> signature verification and data validation for eBPF programs. This
>> allows users to continue to maintain an invariant that all code
>> running inside of the kernel has actually been signed and verified, by
>> the kernel.
>> 
>> This effort builds upon the currently excepted upstream solution. It
>> further hardens it by providing deterministic, in-kernel checking of
>> map hashes to solidify auditing along with preventing TOCTOU attacks
>> against lskel map hashes.
>> 
>> Target map hashes are passed in via PKCS#7 signed attributes. Hornet
>> determines the extent which the eBFP program is signed and defers to
>> other LSMs for policy decisions.
>> 
>> Signed-off-by: Blaise Boscaccy <bboscaccy@linux.microsoft.com>
>> Nacked-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
>> ---
>>  Documentation/admin-guide/LSM/Hornet.rst | 310 ++++++++++++++++++++++
>>  Documentation/admin-guide/LSM/index.rst  |   1 +
>>  MAINTAINERS                              |   9 +
>>  include/linux/oid_registry.h             |   3 +
>>  include/uapi/linux/lsm.h                 |   1 +
>>  security/Kconfig                         |   3 +-
>>  security/Makefile                        |   1 +
>>  security/hornet/Kconfig                  |  11 +
>>  security/hornet/Makefile                 |   7 +
>>  security/hornet/hornet.asn1              |  13 +
>>  security/hornet/hornet_lsm.c             | 323 +++++++++++++++++++++++
>>  11 files changed, 681 insertions(+), 1 deletion(-)
>>  create mode 100644 Documentation/admin-guide/LSM/Hornet.rst
>>  create mode 100644 security/hornet/Kconfig
>>  create mode 100644 security/hornet/Makefile
>>  create mode 100644 security/hornet/hornet.asn1
>>  create mode 100644 security/hornet/hornet_lsm.c
>> 
>> diff --git a/Documentation/admin-guide/LSM/Hornet.rst b/Documentation/admin-guide/LSM/Hornet.rst
>> new file mode 100644
>> index 000000000000..0dd4c03b8a7e
>> --- /dev/null
>> +++ b/Documentation/admin-guide/LSM/Hornet.rst
>> @@ -0,0 +1,310 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +======
>> +Hornet
>> +======
>> +
>> +Hornet is a Linux Security Module that provides extensible signature
>> +verification for eBPF programs. This is selectable at build-time with
>> +``CONFIG_SECURITY_HORNET``.
>> +
>> +Overview
>> +========
>> +
>> +Hornet addresses concerns from users who require strict audit trails and
>> +verification guarantees for eBPF programs, especially in
>> +security-sensitive environments. Many production systems need assurance
>> +that only authorized, unmodified eBPF programs are loaded into the
>> +kernel. Hornet provides this assurance through cryptographic signature
>> +verification.
>> +
>> +When an eBPF program is loaded via the ``bpf()`` syscall, Hornet
>> +verifies a PKCS#7 signature attached to the program instructions. The
>> +signature is checked against the kernel's secondary keyring using the
>> +existing kernel cryptographic infrastructure. In addition to signing the
>> +program bytecode, Hornet supports signing SHA-256 hashes of associated
>> +BPF maps, enabling integrity verification of map contents at load time
>> +and at runtime.
>> +
>> +After verification, Hornet classifies the program into one of the
>> +following integrity states and passes the result to a downstream LSM hook
>> +(``bpf_prog_load_post_integrity``), allowing other security modules to
>> +make policy decisions based on the verification outcome:
>> +
>> +``LSM_INT_VERDICT_OK``
>> +  The program signature and all map hashes verified successfully.
>> +
>> +``LSM_INT_VERDICT_UNSIGNED``
>> +  No signature was provided with the program.
>> +
>> +``LSM_INT_VERDICT_PARTIALSIG``
>> +  The program signature verified, but the signing certificate is not
>> +  trusted in the secondary keyring ...
>
> Do you think there is value in separating this case out from _PARTIALSIG?
> Maybe a LSM_INT_VERDICT_UNKNOWNKEY?
>

Yeah we can break these out a bit with this one and the VERDICT_FAULT
discussed below. Looking at this we, may want to add a VERDICT_UNEXPECTED
or something akin to that to handle the runtime check failure or any
other discrepancies. 

>> +  ... or the signature did not contain
>> +  hornet map hash data.
>> +
>> +``LSM_INT_VERDICT_BADSIG``
>> +  The signature or a map hash failed verification.
>> +
>> +Hornet itself does not enforce a policy on whether unsigned or partially
>> +signed programs should be rejected. It delegates that decision to
>> +downstream LSMs via the ``bpf_prog_load_post_integrity`` hook, making it
>> +a composable building block in a larger security architecture.
>> +
>> +Use Cases
>> +=========
>> +
>> +- **Locked-down production environments**: Ensure only eBPF programs
>> +  signed by a trusted authority can be loaded, preventing unauthorized
>> +  or tampered programs from running in the kernel.
>> +
>> +- **Audit and compliance**: Provide cryptographic evidence that loaded
>> +  eBPF programs match their expected build artifacts, supporting
>> +  compliance requirements in regulated industries.
>> +
>> +- **Supply chain integrity**: Verify that eBPF programs and their
>> +  associated map data have not been modified since they were built and
>> +  signed, protecting against supply chain attacks.
>> +
>> +Threat Model
>> +============
>> +
>> +Hornet protects against the following threats:
>> +
>> +- **Unauthorized eBPF program loading**: Programs that have not been
>> +  signed by a trusted key will be reported as unsigned or badly signed.
>> +
>> +- **Tampering with program instructions**: Any modification to the eBPF
>> +  bytecode after signing will cause signature verification to fail.
>> +
>> +- **Tampering with map data**: When map hashes are included in the
>> +  signature, Hornet verifies that frozen BPF maps match their expected
>> +  SHA-256 hashes at load time. Maps are also re-verified before program
>> +  execution via ``BPF_PROG_RUN``.
>> +
>> +Hornet does **not** protect against:
>> +
>> +- Compromise of the signing key itself.
>> +- Attacks that occur after a program has been loaded and verified.
>> +- Programs loaded by the kernel itself (kernel-internal loads bypass
>> +  the ``BPF_PROG_RUN`` map check).
>> +
>> +Known Limitations
>> +=================
>> +
>> +- Hornet requires programs to use :doc:`light skeletons
>> +  </bpf/libbpf/libbpf_naming_convention>` (lskels) for the signing
>> +  workflow, as the tooling operates on lskel-generated headers.
>> +
>> +- A maximum of 64 maps per program can be tracked for hash
>> +  verification.
>> +
>> +- Map hash verification requires the maps to be frozen before loading.
>> +  Maps that are not frozen at load time will cause verification to fail
>> +  when their hashes are included in the signature.
>> +
>> +- Hornet relies on the kernel's secondary keyring
>> +  (``VERIFY_USE_SECONDARY_KEYRING``) for certificate trust. Keys must
>> +  be provisioned into this keyring before programs can be verified.
>
> I would add a bullet point describing the SHA256 limitation.  If I
> understand things correctly this restriction comes from the core BPF
> code and not Hornet itself, so it would be nice to have this documented
> as it isn't immediately clear when looking only at the Hornet code.
>

Sure we can do that. Yes that's correct. The hashing algorithm is hard
coded and baked into bpf subsystem.

>> +Configuration
>> +=============
>> +
>> +Build Configuration
>> +-------------------
>> +
>> +Enable Hornet by setting the following kernel configuration option::
>> +
>> +  CONFIG_SECURITY_HORNET=y
>> +
>> +This option is found under :menuselection:`Security options --> Hornet
>> +support` and depends on ``CONFIG_SECURITY``.
>> +
>> +When enabled, Hornet is included in the default LSM initialization order
>> +and will appear in ``/sys/kernel/security/lsm``.
>> +
>> +Architecture
>> +============
>> +
>> +Signature Verification Flow
>> +---------------------------
>> +
>> +The following describes what happens when a userspace program calls
>> +``bpf(BPF_PROG_LOAD, ...)`` with a signature attached:
>> +
>> +1. The ``bpf_prog_load_integrity`` LSM hook is invoked.
>> +
>> +2. Hornet reads the signature from the userspace buffer specified by
>> +   ``attr->signature`` (with length ``attr->signature_size``).
>> +
>> +3. The PKCS#7 signature is verified against the program instructions
>> +   using ``verify_pkcs7_signature()`` with the kernel's secondary
>> +   keyring.
>> +
>> +4. The PKCS#7 message is parsed and its trust chain is validated via
>> +   ``validate_pkcs7_trust()``.
>> +
>> +5. Hornet extracts the authenticated attribute identified by
>> +   ``OID_hornet_data`` (OID ``2.25.316487325684022475439036912669789383960``)
>> +   from the PKCS#7 message. This attribute contains an ASN.1-encoded set
>> +   of map index/hash pairs.
>> +
>> +6. For each map hash entry, Hornet retrieves the corresponding BPF map
>> +   via its file descriptor, confirms it is frozen, computes its SHA-256
>> +   hash, and compares it against the signed hash.
>> +
>> +7. The resulting integrity verdict is passed to the
>> +   ``bpf_prog_load_post_integrity`` hook so that downstream LSMs can
>> +   enforce policy.
>> +
>> +Runtime Map Verification
>> +------------------------
>> +
>> +When ``bpf(BPF_PROG_RUN, ...)`` is called from userspace, Hornet
>> +re-verifies the hashes of all maps associated with the program. This
>> +ensures that map contents have not been modified between program load
>> +and execution. If any map hash no longer matches, the ``BPF_PROG_RUN``
>> +command is denied.
>> +
>> +Userspace Interface
>> +-------------------
>> +
>> +Signatures are passed to the kernel through fields in ``union bpf_attr``
>> +when using the ``BPF_PROG_LOAD`` command:
>> +
>> +``signature``
>> +  A pointer to a userspace buffer containing the PKCS#7 signature.
>> +
>> +``signature_size``
>> +  The size of the signature buffer in bytes.
>> +
>> +ASN.1 Schema
>> +------------
>> +
>> +Map hashes are encoded as a signed attribute in the PKCS#7 message using
>> +the following ASN.1 schema::
>> +
>> +  HornetData ::= SET OF Map
>> +
>> +  Map ::= SEQUENCE {
>> +      index   INTEGER,
>> +      sha     OCTET STRING
>> +  }
>> +
>> +Each ``Map`` entry contains the index of the map in the program's
>> +``fd_array`` and its expected SHA-256 hash. A zero-length ``sha`` field
>> +indicates that the map at that index should be skipped during
>> +verification.
>> +
>> +Tooling
>> +=======
>> +
>> +Helper scripts and a signature generation tool are provided in
>> +``scripts/hornet/`` to support the development of signed eBPF light
>> +skeletons.
>> +
>> +gen_sig
>> +-------
>> +
>> +``gen_sig`` is a C program (using OpenSSL) that creates a PKCS#7
>> +signature over eBPF program instructions and optionally includes
>> +SHA-256 hashes of BPF maps as signed attributes.
>> +
>> +Usage::
>> +
>> +  gen_sig --data <instructions.bin> \
>> +          --cert <signer.crt> \
>> +          --key <signer.key> \
>> +          [--pass <passphrase>] \
>> +          --out <signature.p7b> \
>> +          [--add <mapfile.bin>:<index> ...]
>> +
>> +``--data``
>> +  Path to the binary file containing eBPF program instructions to sign.
>> +
>> +``--cert``
>> +  Path to the signing certificate (PEM or DER format).
>> +
>> +``--key``
>> +  Path to the private key (PEM or DER format).
>> +
>> +``--pass``
>> +  Optional passphrase for the private key.
>> +
>> +``--out``
>> +  Path to write the output PKCS#7 signature.
>> +
>> +``--add``
>> +  Attach a map hash as a signed attribute. The argument is a path to a
>> +  binary map file followed by a colon and the map's index in the
>> +  ``fd_array``. This option may be specified multiple times.
>> +
>> +extract-skel.sh
>> +---------------
>> +
>> +Extracts a named field from an autogenerated eBPF lskel header file.
>> +Used internally by other helper scripts.
>> +
>> +extract-insn.sh
>> +---------------
>> +
>> +Extracts the eBPF program instructions (``opts_insn``) from an lskel
>> +header into a binary file suitable for signing with ``gen_sig``.
>> +
>> +extract-map.sh
>> +--------------
>> +
>> +Extracts the map data (``opts_data``) from an lskel header into a
>> +binary file suitable for hashing with ``gen_sig``.
>> +
>> +write-sig.sh
>> +------------
>> +
>> +Replaces the signature data in an lskel header with a new signature
>> +from a binary file. This is used to embed a freshly generated signature
>> +back into the header after signing.
>> +
>> +Signing Workflow
>> +================
>> +
>> +A typical workflow for building and signing an eBPF light skeleton is:
>> +
>> +1. **Compile the eBPF program**::
>> +
>> +     clang -O2 -target bpf -c program.bpf.c -o program.bpf.o
>> +
>> +2. **Generate the light skeleton header** using ``bpftool``::
>> +
>> +     bpftool gen skeleton -S program.bpf.o > loader.h
>> +
>> +3. **Extract instructions and map data** from the generated header::
>> +
>> +     scripts/hornet/extract-insn.sh loader.h > insn.bin
>> +     scripts/hornet/extract-map.sh loader.h > map.bin
>> +
>> +4. **Generate the signature** with ``gen_sig``::
>> +
>> +     scripts/hornet/gen_sig \
>> +       --key signing_key.pem \
>> +       --cert signing_key.x509 \
>> +       --data insn.bin \
>> +       --add map.bin:0 \
>> +       --out sig.bin
>> +
>> +5. **Embed the signature** back into the header::
>> +
>> +     scripts/hornet/write-sig.sh loader.h sig.bin > signed_loader.h
>> +
>> +6. **Build the loader program** using the signed header::
>> +
>> +     cc -o loader loader.c -lbpf
>> +
>> +The resulting loader program will pass the embedded signature to the
>> +kernel when loading the eBPF program, enabling Hornet to verify it.
>> +
>> +Testing
>> +=======
>> +
>> +Self-tests are provided in ``tools/testing/selftests/hornet/``. The test
>> +suite builds a minimal eBPF program (``trivial.bpf.c``), signs it using
>> +the workflow described above, and verifies that the signed program loads
>> +successfully.
>> diff --git a/Documentation/admin-guide/LSM/index.rst b/Documentation/admin-guide/LSM/index.rst
>> index b44ef68f6e4d..57f6e9fbe5fd 100644
>> --- a/Documentation/admin-guide/LSM/index.rst
>> +++ b/Documentation/admin-guide/LSM/index.rst
>> @@ -49,3 +49,4 @@ subdirectories.
>>     SafeSetID
>>     ipe
>>     landlock
>> +   Hornet
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 55af015174a5..6e91234a9ba4 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -11682,6 +11682,15 @@ S:	Maintained
>>  F:	Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml
>>  F:	drivers/iio/pressure/mprls0025pa*
>>  
>> +HORNET SECURITY MODULE
>> +M:	Blaise Boscaccy <bboscaccy@linux.microsoft.com>
>> +L:	linux-security-module@vger.kernel.org
>> +S:	Supported
>> +T:	git https://github.com/blaiseboscaccy/hornet.git
>> +F:	Documentation/admin-guide/LSM/Hornet.rst
>> +F:	scripts/hornet/
>> +F:	security/hornet/
>> +
>>  HP BIOSCFG DRIVER
>>  M:	Jorge Lopez <jorge.lopez2@hp.com>
>>  L:	platform-driver-x86@vger.kernel.org
>> diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h
>> index ebce402854de..bf852715aaea 100644
>> --- a/include/linux/oid_registry.h
>> +++ b/include/linux/oid_registry.h
>> @@ -150,6 +150,9 @@ enum OID {
>>  	OID_id_ml_dsa_65,			/* 2.16.840.1.101.3.4.3.18 */
>>  	OID_id_ml_dsa_87,			/* 2.16.840.1.101.3.4.3.19 */
>>  
>> +	/* Hornet LSM */
>> +	OID_hornet_data,	  /* 2.25.316487325684022475439036912669789383960 */
>> +
>>  	OID__NR
>>  };
>>  
>> diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
>> index 938593dfd5da..2ff9bcdd551e 100644
>> --- a/include/uapi/linux/lsm.h
>> +++ b/include/uapi/linux/lsm.h
>> @@ -65,6 +65,7 @@ struct lsm_ctx {
>>  #define LSM_ID_IMA		111
>>  #define LSM_ID_EVM		112
>>  #define LSM_ID_IPE		113
>> +#define LSM_ID_HORNET		114
>>  
>>  /*
>>   * LSM_ATTR_XXX definitions identify different LSM attributes
>> diff --git a/security/Kconfig b/security/Kconfig
>> index 6a4393fce9a1..283c4a103209 100644
>> --- a/security/Kconfig
>> +++ b/security/Kconfig
>> @@ -230,6 +230,7 @@ source "security/safesetid/Kconfig"
>>  source "security/lockdown/Kconfig"
>>  source "security/landlock/Kconfig"
>>  source "security/ipe/Kconfig"
>> +source "security/hornet/Kconfig"
>>  
>>  source "security/integrity/Kconfig"
>>  
>> @@ -274,7 +275,7 @@ config LSM
>>  	default "landlock,lockdown,yama,loadpin,safesetid,apparmor,selinux,smack,tomoyo,ipe,bpf" if DEFAULT_SECURITY_APPARMOR
>>  	default "landlock,lockdown,yama,loadpin,safesetid,tomoyo,ipe,bpf" if DEFAULT_SECURITY_TOMOYO
>>  	default "landlock,lockdown,yama,loadpin,safesetid,ipe,bpf" if DEFAULT_SECURITY_DAC
>> -	default "landlock,lockdown,yama,loadpin,safesetid,selinux,smack,tomoyo,apparmor,ipe,bpf"
>> +	default "landlock,lockdown,yama,loadpin,safesetid,selinux,smack,tomoyo,apparmor,ipe,hornet,bpf"
>>  	help
>>  	  A comma-separated list of LSMs, in initialization order.
>>  	  Any LSMs left off this list, except for those with order
>> diff --git a/security/Makefile b/security/Makefile
>> index 4601230ba442..b68cb56e419b 100644
>> --- a/security/Makefile
>> +++ b/security/Makefile
>> @@ -26,6 +26,7 @@ obj-$(CONFIG_CGROUPS)			+= device_cgroup.o
>>  obj-$(CONFIG_BPF_LSM)			+= bpf/
>>  obj-$(CONFIG_SECURITY_LANDLOCK)		+= landlock/
>>  obj-$(CONFIG_SECURITY_IPE)		+= ipe/
>> +obj-$(CONFIG_SECURITY_HORNET)		+= hornet/
>>  
>>  # Object integrity file lists
>>  obj-$(CONFIG_INTEGRITY)			+= integrity/
>> diff --git a/security/hornet/Kconfig b/security/hornet/Kconfig
>> new file mode 100644
>> index 000000000000..19406aa237ac
>> --- /dev/null
>> +++ b/security/hornet/Kconfig
>> @@ -0,0 +1,11 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +config SECURITY_HORNET
>> +	bool "Hornet support"
>> +	depends on SECURITY
>> +	default n
>> +	help
>> +	  This selects Hornet.
>> +	  Further information can be found in
>> +	  Documentation/admin-guide/LSM/Hornet.rst.
>> +
>> +	  If you are unsure how to answer this question, answer N.
>> diff --git a/security/hornet/Makefile b/security/hornet/Makefile
>> new file mode 100644
>> index 000000000000..26b6f954f762
>> --- /dev/null
>> +++ b/security/hornet/Makefile
>> @@ -0,0 +1,7 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +obj-$(CONFIG_SECURITY_HORNET) := hornet.o
>> +
>> +hornet-y := hornet.asn1.o \
>> +	hornet_lsm.o \
>> +
>> +$(obj)/hornet.asn1.o: $(obj)/hornet.asn1.c $(obj)/hornet.asn1.h
>> diff --git a/security/hornet/hornet.asn1 b/security/hornet/hornet.asn1
>> new file mode 100644
>> index 000000000000..c8d47b16b65d
>> --- /dev/null
>> +++ b/security/hornet/hornet.asn1
>> @@ -0,0 +1,13 @@
>> +-- SPDX-License-Identifier: BSD-3-Clause
>> +--
>> +-- Copyright (C) 2009 IETF Trust and the persons identified as authors
>> +-- of the code
>> +--
>> +-- https://www.rfc-editor.org/rfc/rfc5652#section-3
>> +
>> +HornetData ::= SET OF Map
>> +
>> +Map ::= SEQUENCE {
>> +	index			INTEGER ({ hornet_map_index }),
>> +	sha			OCTET STRING ({ hornet_map_hash })
>> +} ({ hornet_next_map })
>> diff --git a/security/hornet/hornet_lsm.c b/security/hornet/hornet_lsm.c
>> new file mode 100644
>> index 000000000000..6c821d6441fb
>> --- /dev/null
>> +++ b/security/hornet/hornet_lsm.c
>> @@ -0,0 +1,323 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Hornet Linux Security Module
>> + *
>> + * Author: Blaise Boscaccy <bboscaccy@linux.microsoft.com>
>> + *
>> + * Copyright (C) 2026 Microsoft Corporation
>> + */
>> +
>> +#include <linux/lsm_hooks.h>
>> +#include <uapi/linux/lsm.h>
>> +#include <linux/bpf.h>
>> +#include <linux/verification.h>
>> +#include <crypto/public_key.h>
>> +#include <linux/module_signature.h>
>> +#include <crypto/pkcs7.h>
>> +#include <linux/sort.h>
>> +#include <linux/asn1_decoder.h>
>> +#include <linux/oid_registry.h>
>> +#include "hornet.asn1.h"
>> +
>> +#define MAX_USED_MAPS 64
>> +
>> +struct hornet_maps {
>> +	bpfptr_t fd_array;
>> +};
>> +
>> +struct hornet_parse_context {
>> +	int indexes[MAX_USED_MAPS];
>> +	bool skips[MAX_USED_MAPS];
>> +	unsigned char hashes[SHA256_DIGEST_SIZE * MAX_USED_MAPS];
>> +	int hash_count;
>> +};
>
> I might include a brief comment at near the top of this file referencing
> the hash algorithm limitation in the Hornet docs, otherwise someone is
> surely going to advocate for hash agility improvements at some point.
>

Good idea.

>> +struct hornet_prog_security_struct {
>> +	bool checked[MAX_USED_MAPS];
>> +	unsigned char hashes[SHA256_DIGEST_SIZE * MAX_USED_MAPS];
>> +};
>> +
>> +struct hornet_map_security_struct {
>> +	bool checked;
>> +	int index;
>> +};
>> +
>> +struct lsm_blob_sizes hornet_blob_sizes __ro_after_init = {
>> +	.lbs_bpf_map = sizeof(struct hornet_map_security_struct),
>> +	.lbs_bpf_prog = sizeof(struct hornet_prog_security_struct),
>> +};
>> +
>> +static inline struct hornet_prog_security_struct *
>> +hornet_bpf_prog_security(struct bpf_prog *prog)
>> +{
>> +	return prog->aux->security + hornet_blob_sizes.lbs_bpf_prog;
>> +}
>> +
>> +static inline struct hornet_map_security_struct *
>> +hornet_bpf_map_security(struct bpf_map *map)
>> +{
>> +	return map->security + hornet_blob_sizes.lbs_bpf_map;
>> +}
>> +
>> +static int hornet_verify_hashes(struct hornet_maps *maps,
>> +				struct hornet_parse_context *ctx,
>> +				struct bpf_prog *prog)
>> +{
>> +	int map_fd;
>> +	u32 i;
>> +	struct bpf_map *map;
>> +	int err = 0;
>> +	unsigned char hash[SHA256_DIGEST_SIZE];
>> +	struct hornet_prog_security_struct *security = hornet_bpf_prog_security(prog);
>> +	struct hornet_map_security_struct *map_security;
>> +
>> +	for (i = 0; i < ctx->hash_count; i++) {
>> +		if (ctx->skips[i]) {
>> +			security->checked[i] = false;
>
> I'm not going to argue against an explicit false assignement here, but
> as a FYI, when the LSM framework allocates the various object blobs it
> (re)sets the blob memory to zero via kzalloc().  Even if/when the LSM
> framwork moves to some other allocation scheme we will still need to keep
> that reset-to-zero behavior.
>
> The same applies to the BPF map blobs.
>

Yeah, I'm fine with that change. 

>> +			continue;
>> +		}
>> +
>> +		err = copy_from_bpfptr_offset(&map_fd, maps->fd_array,
>> +					      ctx->indexes[i] * sizeof(map_fd),
>> +					      sizeof(map_fd));
>> +		if (err < 0)
>> +			return LSM_INT_VERDICT_BADSIG;
>> +
>> +		CLASS(fd, f)(map_fd);
>> +		if (fd_empty(f))
>> +			return LSM_INT_VERDICT_BADSIG;
>> +		if (unlikely(fd_file(f)->f_op != &bpf_map_fops))
>> +			return LSM_INT_VERDICT_BADSIG;
>
> I'm wondering if it is worth defining a generic LSM_INT_VERDICT_FAULT
> verdict to indicate a system error when verifying the integrity rather
> than a bad signature.  Yes, the enforcement action will likely be the
> same, but it might help when debugging or chasing forensic data.
>

I like that. At this point there was actually a signature that was
verified against the progam's instruction making VERDICT_BAD_SIG
misleading. 



>> +		map = fd_file(f)->private_data;
>> +		if (!map->frozen)
>> +			return LSM_INT_VERDICT_BADSIG;
>> +
>> +		map->ops->map_get_hash(map, SHA256_DIGEST_SIZE, hash);
>> +
>> +		err = memcmp(hash, &ctx->hashes[i * SHA256_DIGEST_SIZE],
>> +			      SHA256_DIGEST_SIZE);
>> +		if (err)
>> +			return LSM_INT_VERDICT_BADSIG;
>> +
>> +		security->checked[i] = true;
>> +		memcpy(&security->hashes[i * SHA256_DIGEST_SIZE], hash, SHA256_DIGEST_SIZE);
>> +		map_security = hornet_bpf_map_security(map);
>> +		map_security->checked = true;
>> +		map_security->index = i;
>> +	}
>> +	return LSM_INT_VERDICT_OK;
>> +}
>> +
>> +int hornet_next_map(void *context, size_t hdrlen,
>> +		     unsigned char tag,
>> +		     const void *value, size_t vlen)
>> +{
>> +	struct hornet_parse_context *ctx = (struct hornet_parse_context *)context;
>> +
>> +	ctx->hash_count++;
>
> Do we need a check here to ensure that ctx->hash_count doesn't exceed
> MAX_USED_MAPS?  If not here, where do we ensure we don't blow past
> MAX_USED_MAPS?
>

Good idea.

> What does Hornet do if the number of hashed maps is greater then
> MAX_USED_MAPS?  I'm guessing we would want it to return an error and
> fail the load?
>

I'm thinking that's a degenerate case of VERDICT_BADSIG and we should
pass that to the downstream policy handler. 

>> +	return 0;
>> +}
>> +
>> +int hornet_map_index(void *context, size_t hdrlen,
>> +		     unsigned char tag,
>> +		     const void *value, size_t vlen)
>> +{
>> +	struct hornet_parse_context *ctx = (struct hornet_parse_context *)context;
>> +
>> +	if (vlen > 1)
>> +		return -EINVAL;
>> +
>> +	ctx->indexes[ctx->hash_count] = *(u8 *)value;
>> +	return 0;
>> +}
>> +
>> +int hornet_map_hash(void *context, size_t hdrlen,
>> +		    unsigned char tag,
>> +		    const void *value, size_t vlen)
>> +
>> +{
>> +	struct hornet_parse_context *ctx = (struct hornet_parse_context *)context;
>> +
>> +	if (vlen != SHA256_DIGEST_SIZE && vlen != 0)
>> +		return -EINVAL;
>> +
>> +	if (vlen) {
>> +		ctx->skips[ctx->hash_count] = false;
>> +		memcpy(&ctx->hashes[ctx->hash_count * SHA256_DIGEST_SIZE], value, vlen);
>> +	} else
>> +		ctx->skips[ctx->hash_count] = true;
>> +
>> +	return 0;
>> +}
>> +
>> +static int hornet_check_program(struct bpf_prog *prog, union bpf_attr *attr,
>> +				struct bpf_token *token, bool is_kernel)
>> +{
>> +	struct hornet_maps maps = {0};
>> +	bpfptr_t usig = make_bpfptr(attr->signature, is_kernel);
>> +	struct pkcs7_message *msg;
>> +	struct hornet_parse_context *ctx;
>> +	void *sig;
>> +	int err;
>> +	const void *authattrs;
>> +	size_t authattrs_len;
>> +
>> +	if (!attr->signature)
>> +		return LSM_INT_VERDICT_UNSIGNED;
>> +
>> +	ctx = kzalloc(sizeof(struct hornet_parse_context), GFP_KERNEL);
>> +	if (!ctx)
>> +		return -ENOMEM;
>
> I think I mentioned this previously, but let me repeat myself in case I
> didn't ... we don't want to mix LSM_INT_VERDICT enums and errno values
> in the return value.  Yes, you can probably get away with it in the
> majority of cases, but I worry it is a problem waiting to happen.  I
> count only four parameters right now, so adding a verdict enum pointer
> shouldn't be too difficult.
>

Sounds good. I can refactor that. 

>> +	maps.fd_array = make_bpfptr(attr->fd_array, is_kernel);
>> +	sig = kzalloc(attr->signature_size, GFP_KERNEL);
>> +	if (!sig) {
>> +		err = -ENOMEM;
>> +		goto out;
>> +	}
>> +	err = copy_from_bpfptr(sig, usig, attr->signature_size);
>> +	if (err != 0)
>> +		goto cleanup_sig;
>> +
>> +	err = verify_pkcs7_signature(prog->insnsi, prog->len * sizeof(struct bpf_insn),
>> +				     sig, attr->signature_size, VERIFY_USE_SECONDARY_KEYRING,
>> +				     VERIFYING_BPF_SIGNATURE, NULL, NULL);
>> +	if (err < 0) {
>> +		err = LSM_INT_VERDICT_BADSIG;
>> +		goto cleanup_sig;
>> +	}
>> +
>> +	msg = pkcs7_parse_message(sig, attr->signature_size);
>> +	if (IS_ERR(msg)) {
>> +		err = LSM_INT_VERDICT_BADSIG;
>> +		goto cleanup_sig;
>> +	}
>> +
>> +	if (validate_pkcs7_trust(msg, VERIFY_USE_SECONDARY_KEYRING)) {
>> +		err = LSM_INT_VERDICT_PARTIALSIG;
>> +		goto cleanup_msg;
>> +	}
>> +	if (pkcs7_get_authattr(msg, OID_hornet_data,
>> +			       &authattrs, &authattrs_len) == -ENODATA) {
>> +		err = LSM_INT_VERDICT_PARTIALSIG;
>> +		goto cleanup_msg;
>> +	}
>> +
>> +	err = asn1_ber_decoder(&hornet_decoder, ctx, authattrs, authattrs_len);
>> +	if (err < 0 || authattrs == NULL) {
>> +		err = LSM_INT_VERDICT_PARTIALSIG;
>> +		goto cleanup_msg;
>> +	}
>> +	err = hornet_verify_hashes(&maps, ctx, prog);
>> +
>> +cleanup_msg:
>> +	pkcs7_free_message(msg);
>> +cleanup_sig:
>> +	kfree(sig);
>> +out:
>> +	kfree(ctx);
>> +	return err;
>> +}
>> +
>> +static const struct lsm_id hornet_lsmid = {
>> +	.name = "hornet",
>> +	.id = LSM_ID_HORNET,
>> +};
>> +
>> +static int hornet_bpf_prog_load_integrity(struct bpf_prog *prog, union bpf_attr *attr,
>> +					  struct bpf_token *token, bool is_kernel)
>> +{
>> +	int result = hornet_check_program(prog, attr, token, is_kernel);
>
> Can you explain a bit why we check for the kernel flag in hornet_bpf(),
> but not here?  It may be that a brief comment in hornet_bpf() explaining
> the kernel flag exception would be helpful.
>

I can add a comment. The gist of this is that in hornet_bpf(), anything
that had originated from kernel space we assume has already been
checked, in some form or another, so we don't bother checking the
intergity of any maps. In this function, hornet doesn't make
any opinion on that and delegates that to the downstream policy
enforcement. 

>> +	if (result < 0)
>> +		return result;
>> +
>> +	return security_bpf_prog_load_post_integrity(prog, attr, token, is_kernel,
>> +						     &hornet_lsmid, result);
>> +}
>> +
>> +static int hornet_verify_map(struct bpf_prog *prog, int index)>> +{
>> +	unsigned char hash[SHA256_DIGEST_SIZE];
>> +	int i;
>> +	struct bpf_map *map;
>> +	struct hornet_prog_security_struct *security = hornet_bpf_prog_security(prog);
>> +	struct hornet_map_security_struct *map_security;
>> +
>> +	if (!security->checked[index])
>> +		return 0;
>> +
>> +	for (i = 0; i < prog->aux->used_map_cnt; i++) {
>> +		map = prog->aux->used_maps[i];
>> +		map_security = hornet_bpf_map_security(map);
>> +		if (map_security->index != index)
>> +			continue;
>> +
>> +		if (!map->frozen)
>> +			return -EINVAL;
>
> Unless there is serious tampering going on we should never see an
> unfrozen map here, yes?

Someone could write a custom loader program in userspace that doesn't
freeze the map. Having the map frozen to check the hash wasn't enforced
in the BPF subsystem until very recently and it's still only enforced in
the syscall handler, not the actual hash-checking itself. 

>
> We probably also want to use a return value other than -EINVAL as this
> is a access/permission denial.  I would think -EACCES or -EPERM would be
> more appropriate.
>

I'm fine with that. 

>> +		map->ops->map_get_hash(map, SHA256_DIGEST_SIZE, hash);
>> +		if (memcmp(hash, &security->hashes[index * SHA256_DIGEST_SIZE],
>> +			   SHA256_DIGEST_SIZE) != 0)
>
> Presumably this is just being extra careful?
>

Yes. There are some small windows where someone could engage in some
shennagians between loader verification and when the program is actually
run. The check here verifies that the hashes are seeing now still match up
with the hashes we calculated from the uapi fd array. 

>> +			return -EINVAL;
>
> See above, -EACCES or -EPERM is likely a better choice here.
>

I'm fine with that. 

>> +		else
>> +			return 0;
>> +	}
>> +	return -EINVAL;
>
> See above.
>
>> +}
>> +
>> +static int hornet_check_prog_maps(u32 ufd)
>> +{
>> +	CLASS(fd, f)(ufd);
>> +	struct bpf_prog *prog;
>> +	int i, result = 0;
>> +
>> +	if (fd_empty(f))
>> +		return -EBADF;
>> +	if (fd_file(f)->f_op != &bpf_prog_fops)
>> +		return -EINVAL;
>> +
>> +	prog = fd_file(f)->private_data;
>> +
>> +	mutex_lock(&prog->aux->used_maps_mutex);
>> +	if (!prog->aux->used_map_cnt)
>> +		goto out;
>> +
>> +	for (i = 0; i < prog->aux->used_map_cnt; i++) {
>> +		result = hornet_verify_map(prog, i);
>> +		if (result)
>> +			goto out;
>> +	}
>> +out:
>> +	mutex_unlock(&prog->aux->used_maps_mutex);
>> +	return result;
>> +}
>> +
>> +static int hornet_bpf(int cmd, union bpf_attr *attr, unsigned int size, bool kernel)
>> +{
>> +	if (cmd != BPF_PROG_RUN)
>> +		return 0;
>> +	if (kernel)
>> +		return 0;
>> +
>> +	return hornet_check_prog_maps(attr->test.prog_fd);
>> +}
>> +
>> +static struct security_hook_list hornet_hooks[] __ro_after_init = {
>> +	LSM_HOOK_INIT(bpf_prog_load_integrity, hornet_bpf_prog_load_integrity),
>> +	LSM_HOOK_INIT(bpf, hornet_bpf),
>> +};
>> +
>> +static int __init hornet_init(void)
>> +{
>> +	pr_info("Hornet: eBPF signature verification enabled\n");
>> +	security_add_hooks(hornet_hooks, ARRAY_SIZE(hornet_hooks), &hornet_lsmid);
>> +	return 0;
>> +}
>> +
>> +DEFINE_LSM(hornet) = {
>> +	.id = &hornet_lsmid,
>> +	.blobs = &hornet_blob_sizes,
>> +	.init = hornet_init,
>> +};
>> -- 
>> 2.52.0
>
> --
> paul-moore.com

Thanks Paul. 

-blaise

^ permalink raw reply

* Re: [PATCH] lsm: Fix the crash issue in xfrm_decode_session
From: Feng Yang @ 2026-03-19  2:22 UTC (permalink / raw)
  To: casey
  Cc: jmorris, linux-kernel, linux-security-module, paul, serge,
	yangfeng59949
In-Reply-To: <df35542e-d58f-47db-8a4f-92698281a69a@schaufler-ca.com>

On Wed, 18 Mar 2026 10:09:47 -0700, Casey Schaufler wrote:
> On 3/17/2026 11:19 PM, Feng Yang wrote:
> > From: Feng Yang <yangfeng@kylinos.cn>
> >
> > After hooking the following BPF program:
> > SEC("lsm/xfrm_decode_session")
> > int BPF_PROG(lsm_hook_xfrm_decode_session, struct sk_buff *skb, u32 *secid, int ckall)
> > {
> >     return 1; // Any non-zero value
> > }
> > Subsequent packet transmission triggers will cause a kernel panic:

> LSM hooks that use or provide secids cannot be stacked. That is,
> you can't provide a BPF LSM hook and an SELinux LSM hook and expect
> correct behavior. Your proposed "fix" removes a legitimate check.

I'm very sorry, I didn't quite understand what you meant.

Maybe my commit message wasn't clear. I only used a BPF LSM hook without SELinux stacking enabled.

Therefore, is it the expected behavior that simply using
SEC("lsm/xfrm_decode_session")
int BPF_PROG(lsm_hook_xfrm_decode_session, struct sk_buff *skb, u32 *secid, int ckall) {
    return -1;
}
would cause a kernel panic? If not, and if the BUG_ON check is still necessary,
then does it mean we need to modify the return value validation logic in the BPF
verifier to ensure that only BPF programs returning 0 are accepted for this hook?

Thanks.


^ permalink raw reply

* Re: [PATCH v1] security/safesetid: fix comment and error handling
From: Micah Morton @ 2026-03-19  3:27 UTC (permalink / raw)
  To: yanwei.gao; +Cc: paul, linux-security-module
In-Reply-To: <CAJ-EccNLTuj74vKsNQFnOSXgjdyRpUrFVARsuqn6VNatQu33WA@mail.gmail.com>

On Wed, Mar 18, 2026 at 1:49 PM Micah Morton <mortonm@chromium.org> wrote:
>
> On Mon, Mar 2, 2026 at 6:40 PM yanwei.gao <gaoyanwei.tx@gmail.com> wrote:
> >
> > - Fix comment in lsm.c: use CAP_SETGID instead of CAP_SETUID in the
> >   GID capability check comment to match the actual logic.
> > - In handle_policy_update(), set err = -EINVAL and goto out_free_buf
> >   when policy type is neither UID nor GID, so the error is returned
> >   to the caller instead of only logging.
> > - In safesetid_init_securityfs(), return ret directly when
> >   policy_dir creation fails instead of goto error (no cleanup needed
> >   at that point).
> >
> > Signed-off-by: yanwei.gao <gaoyanwei.tx@gmail.com>
> > ---
> >  security/safesetid/lsm.c        | 2 +-
> >  security/safesetid/securityfs.c | 4 +++-
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
> > index d5fb949050dd..a7b68e65996c 100644
> > --- a/security/safesetid/lsm.c
> > +++ b/security/safesetid/lsm.c
> > @@ -128,7 +128,7 @@ static int safesetid_security_capable(const struct cred *cred,
> >                 if (setid_policy_lookup((kid_t){.gid = cred->gid}, INVALID_ID, GID) == SIDPOL_DEFAULT)
> >                         return 0;
> >                 /*
> > -                * Reject use of CAP_SETUID for functionality other than calling
> > +                * Reject use of CAP_SETGID for functionality other than calling
> Looks good.
> >                  * set*gid() (e.g. setting up userns gid mappings).
> >                  */
> >                 pr_warn("Operation requires CAP_SETGID, which is not available to GID %u for operations besides approved set*gid transitions\n",
> > diff --git a/security/safesetid/securityfs.c b/security/safesetid/securityfs.c
> > index a71e548065a9..50682abd342b 100644
> > --- a/security/safesetid/securityfs.c
> > +++ b/security/safesetid/securityfs.c
> > @@ -224,6 +224,8 @@ static ssize_t handle_policy_update(struct file *file,
> >         } else {
> >                 /* Error, policy type is neither UID or GID */
> >                 pr_warn("error: bad policy type");
> > +               err = -EINVAL;
> > +               goto out_free_buf;
>
> This makes sense to me and matches how things are done in the
> functions above in verify_ruleset() and parse_policy_line().
>
> Is this code actually reachable? I guess if verify_ruleset returns
> -EINVAL due to a bad policy type value the code still hits this spot?
>
> >         }
> >         err = len;
> >
> > @@ -321,7 +323,7 @@ int __init safesetid_init_securityfs(void)
> >         policy_dir = securityfs_create_dir("safesetid", NULL);
> >         if (IS_ERR(policy_dir)) {
> >                 ret = PTR_ERR(policy_dir);
> > -               goto error;
> > +               return ret;
>
> We still need the `securityfs_remove(policy_dir)` call to clean up the
> dir created with `policy_dir = securityfs_create_dir("safesetid",
> NULL)` don't we?

I guess securityfs_remove() just returns right away if the
'policy_dir' pointer IS_ERR, so either way is fine. I don't really see
a reason to change this though.. seems better to err on the side of
calling the securityfs_remove() function even if its currently a
no-op.

>
> >         }
> >
> >         uid_policy_file = securityfs_create_file("uid_allowlist_policy", 0600,
> > --
> > 2.43.5
> >

^ permalink raw reply

* [PATCH] apparmor: Fix string overrun due to missing termination
From: Daniel J Blueman @ 2026-03-19  6:24 UTC (permalink / raw)
  To: John Johansen, Paul Moore, James Morris, Serge E. Hallyn
  Cc: Thorsten Blum, apparmor, linux-security-module, linux-kernel,
	stable, Daniel J Blueman

When booting Ubuntu 26.04 with Linux 7.0-rc4 on an ARM64 Qualcomm
Snapdragon X1 we see a string buffer overrun:

BUG: KASAN: slab-out-of-bounds in aa_dfa_match (security/apparmor/match.c:535)
Read of size 1 at addr ffff0008901cc000 by task snap-update-ns/2120

CPU: 5 UID: 60578 PID: 2120 Comm: snap-update-ns Not tainted 7.0.0-rc4+ #22 PREEMPTLAZY
Hardware name: LENOVO 83ED/LNVNB161216, BIOS NHCN60WW 09/11/2025
Call trace:
show_stack (arch/arm64/kernel/stacktrace.c:501) (C)
dump_stack_lvl (lib/dump_stack.c:122)
print_report (mm/kasan/report.c:379 mm/kasan/report.c:482)
kasan_report (mm/kasan/report.c:597)
__asan_report_load1_noabort (mm/kasan/report_generic.c:378)
aa_dfa_match (security/apparmor/match.c:535)
match_mnt_path_str (security/apparmor/mount.c:244 security/apparmor/mount.c:336)
match_mnt (security/apparmor/mount.c:371)
aa_bind_mount (security/apparmor/mount.c:447 (discriminator 4))
apparmor_sb_mount (security/apparmor/lsm.c:719 (discriminator 1))
security_sb_mount (security/security.c:1062 (discriminator 31))
path_mount (fs/namespace.c:4101)
__arm64_sys_mount (fs/namespace.c:4172 fs/namespace.c:4361 fs/namespace.c:4338 fs/namespace.c:4338)
invoke_syscall.constprop.0 (arch/arm64/kernel/syscall.c:35 arch/arm64/kernel/syscall.c:49)
el0_svc_common.constprop.0 (./include/linux/thread_info.h:142 (discriminator 2) arch/arm64/kernel/syscall.c:140 (discriminator 2))
do_el0_svc (arch/arm64/kernel/syscall.c:152)
el0_svc (arch/arm64/kernel/entry-common.c:80 arch/arm64/kernel/entry-common.c:725)
el0t_64_sync_handler (arch/arm64/kernel/entry-common.c:744)
el0t_64_sync (arch/arm64/kernel/entry.S:596)

Allocated by task 2120:
kasan_save_stack (mm/kasan/common.c:58)
kasan_save_track (./arch/arm64/include/asm/current.h:19 mm/kasan/common.c:70 mm/kasan/common.c:79)
kasan_save_alloc_info (mm/kasan/generic.c:571)
__kasan_kmalloc (mm/kasan/common.c:419)
__kmalloc_noprof (./include/linux/kasan.h:263 mm/slub.c:5260 mm/slub.c:5272)
aa_get_buffer (security/apparmor/lsm.c:2201)
aa_bind_mount (security/apparmor/mount.c:442)
apparmor_sb_mount (security/apparmor/lsm.c:719 (discriminator 1))
security_sb_mount (security/security.c:1062 (discriminator 31))
path_mount (fs/namespace.c:4101)
__arm64_sys_mount (fs/namespace.c:4172 fs/namespace.c:4361 fs/namespace.c:4338 fs/namespace.c:4338)
invoke_syscall.constprop.0 (arch/arm64/kernel/syscall.c:35 arch/arm64/kernel/syscall.c:49)
el0_svc_common.constprop.0 (./include/linux/thread_info.h:142 (discriminator 2) arch/arm64/kernel/syscall.c:140 (discriminator 2))
do_el0_svc (arch/arm64/kernel/syscall.c:152)
el0_svc (arch/arm64/kernel/entry-common.c:80 arch/arm64/kernel/entry-common.c:725)
el0t_64_sync_handler (arch/arm64/kernel/entry-common.c:744)
el0t_64_sync (arch/arm64/kernel/entry.S:596)

The buggy address belongs to the object at ffff0008901ca000
which belongs to the cache kmalloc-rnd-06-8k of size 8192
The buggy address is located 0 bytes to the right of
allocated 8192-byte region [ffff0008901ca000, ffff0008901cc000)

The buggy address belongs to the physical page:
page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x9101c8
head: order:3 mapcount:0 entire_mapcount:0 nr_pages_mapped:-1 pincount:0
flags: 0x8000000000000040(head|zone=2)
page_type: f5(slab)
raw: 8000000000000040 ffff000800016c40 fffffdffe2d14e10 ffff000800015c70
raw: 0000000000000000 0000000800010001 00000000f5000000 0000000000000000
head: 8000000000000040 ffff000800016c40 fffffdffe2d14e10 ffff000800015c70
head: 0000000000000000 0000000800010001 00000000f5000000 0000000000000000
head: 8000000000000003 fffffdffe2407201 fffffdffffffffff 00000000ffffffff
head: ffffffffffffffff 0000000000000000 00000000ffffffff 0000000000000008
page dumped because: kasan: bad access detected

Memory state around the buggy address:
ffff0008901cbf00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff0008901cbf80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>ffff0008901cc000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
^
ffff0008901cc080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff0008901cc100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc

This was introduced by previous incorrect conversion from strcpy(). Fix it
by adding the missing terminator.

Signed-off-by: Daniel J Blueman <daniel@quora.org>
Fixes: 93d4dbdc8da0 ("apparmor: Replace deprecated strcpy in d_namespace_path")
---
 security/apparmor/path.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/security/apparmor/path.c b/security/apparmor/path.c
index 65a0ca5cc1bd..2494e8101538 100644
--- a/security/apparmor/path.c
+++ b/security/apparmor/path.c
@@ -164,14 +164,16 @@ static int d_namespace_path(const struct path *path, char *buf, char **name,
 	}
 
 out:
-	/* Append "/" to directory paths, except for root "/" which
-	 * already ends in a slash.
+	/* Append "/" to directory paths and reterminate string, except for
+	 * root "/" which already ends in a slash.
 	 */
 	if (!error && isdir) {
 		bool is_root = (*name)[0] == '/' && (*name)[1] == '\0';
 
-		if (!is_root)
+		if (!is_root) {
 			buf[aa_g_path_max - 2] = '/';
+			buf[aa_g_path_max - 1] = '\0';
+		}
 	}
 
 	return error;
-- 
2.53.0


^ permalink raw reply related

* Re: [PATCH] apparmor: Fix string overrun due to missing termination
From: Greg KH @ 2026-03-19  6:40 UTC (permalink / raw)
  To: Daniel J Blueman
  Cc: John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
	Thorsten Blum, apparmor, linux-security-module, linux-kernel,
	stable
In-Reply-To: <20260319062433.17648-1-daniel@quora.org>

On Thu, Mar 19, 2026 at 02:24:32PM +0800, Daniel J Blueman wrote:
> When booting Ubuntu 26.04 with Linux 7.0-rc4 on an ARM64 Qualcomm
> Snapdragon X1 we see a string buffer overrun:
> 
> BUG: KASAN: slab-out-of-bounds in aa_dfa_match (security/apparmor/match.c:535)
> Read of size 1 at addr ffff0008901cc000 by task snap-update-ns/2120
> 
> CPU: 5 UID: 60578 PID: 2120 Comm: snap-update-ns Not tainted 7.0.0-rc4+ #22 PREEMPTLAZY
> Hardware name: LENOVO 83ED/LNVNB161216, BIOS NHCN60WW 09/11/2025
> Call trace:
> show_stack (arch/arm64/kernel/stacktrace.c:501) (C)
> dump_stack_lvl (lib/dump_stack.c:122)
> print_report (mm/kasan/report.c:379 mm/kasan/report.c:482)
> kasan_report (mm/kasan/report.c:597)
> __asan_report_load1_noabort (mm/kasan/report_generic.c:378)
> aa_dfa_match (security/apparmor/match.c:535)
> match_mnt_path_str (security/apparmor/mount.c:244 security/apparmor/mount.c:336)
> match_mnt (security/apparmor/mount.c:371)
> aa_bind_mount (security/apparmor/mount.c:447 (discriminator 4))
> apparmor_sb_mount (security/apparmor/lsm.c:719 (discriminator 1))
> security_sb_mount (security/security.c:1062 (discriminator 31))
> path_mount (fs/namespace.c:4101)
> __arm64_sys_mount (fs/namespace.c:4172 fs/namespace.c:4361 fs/namespace.c:4338 fs/namespace.c:4338)
> invoke_syscall.constprop.0 (arch/arm64/kernel/syscall.c:35 arch/arm64/kernel/syscall.c:49)
> el0_svc_common.constprop.0 (./include/linux/thread_info.h:142 (discriminator 2) arch/arm64/kernel/syscall.c:140 (discriminator 2))
> do_el0_svc (arch/arm64/kernel/syscall.c:152)
> el0_svc (arch/arm64/kernel/entry-common.c:80 arch/arm64/kernel/entry-common.c:725)
> el0t_64_sync_handler (arch/arm64/kernel/entry-common.c:744)
> el0t_64_sync (arch/arm64/kernel/entry.S:596)
> 
> Allocated by task 2120:
> kasan_save_stack (mm/kasan/common.c:58)
> kasan_save_track (./arch/arm64/include/asm/current.h:19 mm/kasan/common.c:70 mm/kasan/common.c:79)
> kasan_save_alloc_info (mm/kasan/generic.c:571)
> __kasan_kmalloc (mm/kasan/common.c:419)
> __kmalloc_noprof (./include/linux/kasan.h:263 mm/slub.c:5260 mm/slub.c:5272)
> aa_get_buffer (security/apparmor/lsm.c:2201)
> aa_bind_mount (security/apparmor/mount.c:442)
> apparmor_sb_mount (security/apparmor/lsm.c:719 (discriminator 1))
> security_sb_mount (security/security.c:1062 (discriminator 31))
> path_mount (fs/namespace.c:4101)
> __arm64_sys_mount (fs/namespace.c:4172 fs/namespace.c:4361 fs/namespace.c:4338 fs/namespace.c:4338)
> invoke_syscall.constprop.0 (arch/arm64/kernel/syscall.c:35 arch/arm64/kernel/syscall.c:49)
> el0_svc_common.constprop.0 (./include/linux/thread_info.h:142 (discriminator 2) arch/arm64/kernel/syscall.c:140 (discriminator 2))
> do_el0_svc (arch/arm64/kernel/syscall.c:152)
> el0_svc (arch/arm64/kernel/entry-common.c:80 arch/arm64/kernel/entry-common.c:725)
> el0t_64_sync_handler (arch/arm64/kernel/entry-common.c:744)
> el0t_64_sync (arch/arm64/kernel/entry.S:596)
> 
> The buggy address belongs to the object at ffff0008901ca000
> which belongs to the cache kmalloc-rnd-06-8k of size 8192
> The buggy address is located 0 bytes to the right of
> allocated 8192-byte region [ffff0008901ca000, ffff0008901cc000)
> 
> The buggy address belongs to the physical page:
> page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x9101c8
> head: order:3 mapcount:0 entire_mapcount:0 nr_pages_mapped:-1 pincount:0
> flags: 0x8000000000000040(head|zone=2)
> page_type: f5(slab)
> raw: 8000000000000040 ffff000800016c40 fffffdffe2d14e10 ffff000800015c70
> raw: 0000000000000000 0000000800010001 00000000f5000000 0000000000000000
> head: 8000000000000040 ffff000800016c40 fffffdffe2d14e10 ffff000800015c70
> head: 0000000000000000 0000000800010001 00000000f5000000 0000000000000000
> head: 8000000000000003 fffffdffe2407201 fffffdffffffffff 00000000ffffffff
> head: ffffffffffffffff 0000000000000000 00000000ffffffff 0000000000000008
> page dumped because: kasan: bad access detected
> 
> Memory state around the buggy address:
> ffff0008901cbf00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffff0008901cbf80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >ffff0008901cc000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ^
> ffff0008901cc080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff0008901cc100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> 
> This was introduced by previous incorrect conversion from strcpy(). Fix it
> by adding the missing terminator.
> 
> Signed-off-by: Daniel J Blueman <daniel@quora.org>
> Fixes: 93d4dbdc8da0 ("apparmor: Replace deprecated strcpy in d_namespace_path")
> ---
>  security/apparmor/path.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/security/apparmor/path.c b/security/apparmor/path.c
> index 65a0ca5cc1bd..2494e8101538 100644
> --- a/security/apparmor/path.c
> +++ b/security/apparmor/path.c
> @@ -164,14 +164,16 @@ static int d_namespace_path(const struct path *path, char *buf, char **name,
>  	}
>  
>  out:
> -	/* Append "/" to directory paths, except for root "/" which
> -	 * already ends in a slash.
> +	/* Append "/" to directory paths and reterminate string, except for
> +	 * root "/" which already ends in a slash.
>  	 */
>  	if (!error && isdir) {
>  		bool is_root = (*name)[0] == '/' && (*name)[1] == '\0';
>  
> -		if (!is_root)
> +		if (!is_root) {
>  			buf[aa_g_path_max - 2] = '/';
> +			buf[aa_g_path_max - 1] = '\0';
> +		}
>  	}
>  
>  	return error;
> -- 
> 2.53.0
> 
> 

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

^ permalink raw reply

* Re: [apparmor][PATCH] apparmor: fix incorrect success return value in unpack_tag_headers()
From: Massimiliano Pellizzer @ 2026-03-19  8:23 UTC (permalink / raw)
  To: John Johansen; +Cc: apparmor, linux-security-module, linux-kernel
In-Reply-To: <779f2d5b-a5af-4137-a1ee-78dc9fed58e1@canonical.com>

On Wed, Mar 18, 2026 at 6:53 AM John Johansen
<john.johansen@canonical.com> wrote:
>
> On 2/10/26 09:21, Massimiliano Pellizzer wrote:
> > unpack_tag_headers() returns `true` (1) on success instead of 0.
> > Since it's caller unpack_tags() checks the return value with
> > `if (error)`, a non-zero success value is incorrectly treated as
> > a failure, causing tag header unpacking to always even if the data
> > is well-formed.
> >
> > Change the success return in unpack_tag_headers() from `true` to 0.
> >
> > Fixes: 3d28e2397af7 ("apparmor: add support loading per permission tagging")
> > Signed-off-by: Massimiliano Pellizzer <mpellizzer.dev@gmail.com>
>
> sorry, my reply to this seems to have failed. This was pulled in for the
> 7.0 PR
>
> Acked-by: John Johansen <john.johansen@canonical.com>
>
>
> > ---
> >   security/apparmor/policy_unpack.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
> > index dc908e1f5a88..221208788025 100644
> > --- a/security/apparmor/policy_unpack.c
> > +++ b/security/apparmor/policy_unpack.c
> > @@ -825,7 +825,7 @@ static int unpack_tag_headers(struct aa_ext *e, struct aa_tags_struct *tags)
> >       tags->hdrs.size = size;
> >       tags->hdrs.table = hdrs;
> >       AA_DEBUG(DEBUG_UNPACK, "headers %ld size %d", (long) hdrs, size);
> > -     return true;
> > +     return 0;
> >
> >   fail:
> >       kfree_sensitive(hdrs);
>

Hello JJ,
I don't see this patch being part of v7.0-rc4:
$ git --no-pager log --grep "apparmor: fix incorrect success return
value in unpack"

I don't see the change applied to the apparmor-next branch either
(https://gitlab.com/apparmor/apparmor-kernel/-/blob/apparmor-next/security/apparmor/policy_unpack.c).

---
Massimiliano Pellizzer

^ permalink raw reply

* [PATCH] fs: allow vfs code to open an O_PATH file with negative dentry
From: Amir Goldstein @ 2026-03-19 12:46 UTC (permalink / raw)
  To: Christian Brauner
  Cc: Al Viro, Miklos Szeredi, Paul Moore, Gao Xiang,
	linux-security-module, selinux, linux-erofs, linux-fsdevel,
	linux-unionfs, syzbot+f34aab278bf5d664e2be

The fields f_mapping, f_wb_err, f_sb_err are irrelevant for O_PATH file.
Skip setting them for O_PATH file, so that the O_PATH file could be
opened with a negative dentry.

This is not something that a user should be able to do, but vfs code,
such as ovl_tmpfile() can use this to open a backing O_PATH tmpfile
before instantiating the dentry.

Reported-by: syzbot+f34aab278bf5d664e2be@syzkaller.appspotmail.com
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---

Christian,

This patch fixes the syzbot report [1] that the
backing_file_user_path_file() patch [2] introduces.

This is not the only possible fix, but it is the cleanest one IMO.
There is a small risk in introducing a state of an O_PATH file with
NULL f_inode, but I (and the bots that I asked) did not find any
obvious risk in this state.

Note that specifically, the user path inode is accessed via d_inode()
and not via file_inode(), which makes this safe for file_user_inode()
callers.

BTW, I missed this regression with the original patch because I
only ran the quick overlayfs sanity test.

Now I ran a full quick fstest cycle and verified that the O_TMPFILE
test case is covered and that the bug is detected.

WDYT?

Thanks,
Amir.

[1] https://syzkaller.appspot.com/bug?extid=f34aab278bf5d664e2be
[2] https://lore.kernel.org/linux-fsdevel/20260318131258.1457101-1-amir73il@gmail.com/

 fs/open.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/open.c b/fs/open.c
index 91f1139591abe..2004a8c0d9c97 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -893,9 +893,6 @@ static int do_dentry_open(struct file *f,
 
 	path_get(&f->f_path);
 	f->f_inode = inode;
-	f->f_mapping = inode->i_mapping;
-	f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
-	f->f_sb_err = file_sample_sb_err(f);
 
 	if (unlikely(f->f_flags & O_PATH)) {
 		f->f_mode = FMODE_PATH | FMODE_OPENED;
@@ -904,6 +901,10 @@ static int do_dentry_open(struct file *f,
 		return 0;
 	}
 
+	f->f_mapping = inode->i_mapping;
+	f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
+	f->f_sb_err = file_sample_sb_err(f);
+
 	if ((f->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) {
 		i_readcount_inc(inode);
 	} else if (f->f_mode & FMODE_WRITE && !special_file(inode->i_mode)) {
-- 
2.53.0


^ permalink raw reply related

* Re: [PATCH] fs: allow vfs code to open an O_PATH file with negative dentry
From: Christian Brauner @ 2026-03-19 13:13 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Al Viro, Miklos Szeredi, Paul Moore, Gao Xiang,
	linux-security-module, selinux, linux-erofs, linux-fsdevel,
	linux-unionfs, syzbot+f34aab278bf5d664e2be
In-Reply-To: <20260319124616.1544415-1-amir73il@gmail.com>

On Thu, Mar 19, 2026 at 01:46:16PM +0100, Amir Goldstein wrote:
> The fields f_mapping, f_wb_err, f_sb_err are irrelevant for O_PATH file.
> Skip setting them for O_PATH file, so that the O_PATH file could be
> opened with a negative dentry.
> 
> This is not something that a user should be able to do, but vfs code,
> such as ovl_tmpfile() can use this to open a backing O_PATH tmpfile
> before instantiating the dentry.
> 
> Reported-by: syzbot+f34aab278bf5d664e2be@syzkaller.appspotmail.com
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
> 
> Christian,
> 
> This patch fixes the syzbot report [1] that the
> backing_file_user_path_file() patch [2] introduces.
> 
> This is not the only possible fix, but it is the cleanest one IMO.
> There is a small risk in introducing a state of an O_PATH file with
> NULL f_inode, but I (and the bots that I asked) did not find any
> obvious risk in this state.
> 
> Note that specifically, the user path inode is accessed via d_inode()
> and not via file_inode(), which makes this safe for file_user_inode()
> callers.
> 
> BTW, I missed this regression with the original patch because I
> only ran the quick overlayfs sanity test.
> 
> Now I ran a full quick fstest cycle and verified that the O_TMPFILE
> test case is covered and that the bug is detected.
> 
> WDYT?
> 
> Thanks,
> Amir.
> 
> [1] https://syzkaller.appspot.com/bug?extid=f34aab278bf5d664e2be
> [2] https://lore.kernel.org/linux-fsdevel/20260318131258.1457101-1-amir73il@gmail.com/
> 
>  fs/open.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/open.c b/fs/open.c
> index 91f1139591abe..2004a8c0d9c97 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -893,9 +893,6 @@ static int do_dentry_open(struct file *f,
>  
>  	path_get(&f->f_path);
>  	f->f_inode = inode;
> -	f->f_mapping = inode->i_mapping;
> -	f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> -	f->f_sb_err = file_sample_sb_err(f);
>  
>  	if (unlikely(f->f_flags & O_PATH)) {
>  		f->f_mode = FMODE_PATH | FMODE_OPENED;
> @@ -904,6 +901,10 @@ static int do_dentry_open(struct file *f,
>  		return 0;
>  	}
>  
> +	f->f_mapping = inode->i_mapping;
> +	f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> +	f->f_sb_err = file_sample_sb_err(f);
> +
>  	if ((f->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) {
>  		i_readcount_inc(inode);
>  	} else if (f->f_mode & FMODE_WRITE && !special_file(inode->i_mode)) {

I think this is really ugly and I'm really unhappy that we should adjust
initialization of generic vfs code for this. My preference is to push
the pain into the backing file stuff. And my ultimate preference is for
this backing file stuff to be removed again for a simple struct path.
We're working around some more fundamental cleanup here imho.

^ permalink raw reply

* Re: [PATCH 12/61] quota: Prefer IS_ERR_OR_NULL over manual NULL check
From: Jan Kara @ 2026-03-19 14:13 UTC (permalink / raw)
  To: Philipp Hahn
  Cc: amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel, dri-devel,
	gfs2, intel-gfx, intel-wired-lan, iommu, kvm, linux-arm-kernel,
	linux-block, linux-bluetooth, linux-btrfs, linux-cifs, linux-clk,
	linux-erofs, linux-ext4, linux-fsdevel, linux-gpio, linux-hyperv,
	linux-input, linux-kernel, linux-leds, linux-media, linux-mips,
	linux-mm, linux-modules, linux-mtd, linux-nfs, linux-omap,
	linux-phy, linux-pm, linux-rockchip, linux-s390, linux-scsi,
	linux-sctp, linux-security-module, linux-sh, linux-sound,
	linux-stm32, linux-trace-kernel, linux-usb, linux-wireless,
	netdev, ntfs3, samba-technical, sched-ext, target-devel,
	tipc-discussion, v9fs, Jan Kara
In-Reply-To: <20260310-b4-is_err_or_null-v1-12-bd63b656022d@avm.de>

On Tue 10-03-26 12:48:38, Philipp Hahn wrote:
> Prefer using IS_ERR_OR_NULL() over using IS_ERR() and a manual NULL
> check.
> 
> Change generated with coccinelle.
> 
> To: Jan Kara <jack@suse.com>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Philipp Hahn <phahn-oss@avm.de>

Thanks for the patch but frankly I find the original variant clearer wrt
what is going on. So I prefer to keep the code as is.

								Honza

> ---
>  fs/quota/quota.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/quota/quota.c b/fs/quota/quota.c
> index 33bacd70758007129e0375bab44d7431195ec441..2e09fc247d0cf45b9e83a4f8a0be7ea694c8c2a1 100644
> --- a/fs/quota/quota.c
> +++ b/fs/quota/quota.c
> @@ -965,7 +965,7 @@ SYSCALL_DEFINE4(quotactl, unsigned int, cmd, const char __user *, special,
>  	else
>  		drop_super_exclusive(sb);
>  out:
> -	if (pathp && !IS_ERR(pathp))
> +	if (!IS_ERR_OR_NULL(pathp))
>  		path_put(pathp);
>  	return ret;
>  }
> 
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply

* Re: [PATCH v9 01/11] KEYS: trusted: Use get_random-fallback for TPM
From: Mimi Zohar @ 2026-03-19 14:28 UTC (permalink / raw)
  To: Chris Fenner
  Cc: Jarkko Sakkinen, linux-integrity, Jonathan McDowell, Eric Biggers,
	James Bottomley, David Howells, Paul Moore, James Morris,
	Serge E. Hallyn, open list:KEYS-TRUSTED,
	open list:SECURITY SUBSYSTEM, open list, Roberto Sassu
In-Reply-To: <CAMigqh2kW_srDgnOs+1t=sk9Q9jJgaQVswO0ZRHhV-6zrOAZ6A@mail.gmail.com>

On Wed, 2026-03-18 at 10:36 -0700, Chris Fenner wrote:
> Apologies if my long message derailed this discussion. I meant to
> support Mimi's concern here and project a future vision where
> TCG_TPM2_HMAC doesn't conflict with other features.
> 
> More concisely, I think that:
> 
> > tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled
> 
> is not a compelling argument for removing TPM as an RNG source,
> because TCG_TPM2_HMAC is known to have poor performance already
> anyway.

Agreed.  Thanks, Chris!  FYI, we raised concerns about IMA performance with the
TPM HMAC and encrypted feature while it was being developed. James had some
ideas, at the time, as to how to resolve the performance issue for IMA.  Yet it
was upstreamed without those changes and with CONFIG_TCG_TPM2_HMAC enabled by
default on x86 systems.

Jarkko has queued this patch in the "queue" branch, without indicating whether
it will eventually be upstreamed or not.

Mimi

^ permalink raw reply

* Re: [PATCH] fs: allow vfs code to open an O_PATH file with negative dentry
From: Amir Goldstein @ 2026-03-19 14:50 UTC (permalink / raw)
  To: Christian Brauner, Paul Moore
  Cc: Al Viro, Miklos Szeredi, Gao Xiang, linux-security-module,
	selinux, linux-erofs, linux-fsdevel, linux-unionfs,
	syzbot+f34aab278bf5d664e2be
In-Reply-To: <20260319-unentbehrlich-reitturnier-f78d9fb01230@brauner>

On Thu, Mar 19, 2026 at 2:13 PM Christian Brauner <brauner@kernel.org> wrote:
>
> On Thu, Mar 19, 2026 at 01:46:16PM +0100, Amir Goldstein wrote:
> > The fields f_mapping, f_wb_err, f_sb_err are irrelevant for O_PATH file.
> > Skip setting them for O_PATH file, so that the O_PATH file could be
> > opened with a negative dentry.
> >
> > This is not something that a user should be able to do, but vfs code,
> > such as ovl_tmpfile() can use this to open a backing O_PATH tmpfile
> > before instantiating the dentry.
> >
> > Reported-by: syzbot+f34aab278bf5d664e2be@syzkaller.appspotmail.com
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >
> > Christian,
> >
> > This patch fixes the syzbot report [1] that the
> > backing_file_user_path_file() patch [2] introduces.
> >
> > This is not the only possible fix, but it is the cleanest one IMO.
> > There is a small risk in introducing a state of an O_PATH file with
> > NULL f_inode, but I (and the bots that I asked) did not find any
> > obvious risk in this state.
> >
> > Note that specifically, the user path inode is accessed via d_inode()
> > and not via file_inode(), which makes this safe for file_user_inode()
> > callers.
> >
> > BTW, I missed this regression with the original patch because I
> > only ran the quick overlayfs sanity test.
> >
> > Now I ran a full quick fstest cycle and verified that the O_TMPFILE
> > test case is covered and that the bug is detected.
> >
> > WDYT?
> >
> > Thanks,
> > Amir.
> >
> > [1] https://syzkaller.appspot.com/bug?extid=f34aab278bf5d664e2be
> > [2] https://lore.kernel.org/linux-fsdevel/20260318131258.1457101-1-amir73il@gmail.com/
> >
> >  fs/open.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/open.c b/fs/open.c
> > index 91f1139591abe..2004a8c0d9c97 100644
> > --- a/fs/open.c
> > +++ b/fs/open.c
> > @@ -893,9 +893,6 @@ static int do_dentry_open(struct file *f,
> >
> >       path_get(&f->f_path);
> >       f->f_inode = inode;
> > -     f->f_mapping = inode->i_mapping;
> > -     f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> > -     f->f_sb_err = file_sample_sb_err(f);
> >
> >       if (unlikely(f->f_flags & O_PATH)) {
> >               f->f_mode = FMODE_PATH | FMODE_OPENED;
> > @@ -904,6 +901,10 @@ static int do_dentry_open(struct file *f,
> >               return 0;
> >       }
> >
> > +     f->f_mapping = inode->i_mapping;
> > +     f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> > +     f->f_sb_err = file_sample_sb_err(f);
> > +
> >       if ((f->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) {
> >               i_readcount_inc(inode);
> >       } else if (f->f_mode & FMODE_WRITE && !special_file(inode->i_mode)) {
>
> I think this is really ugly and I'm really unhappy that we should adjust
> initialization of generic vfs code for this. My preference is to push
> the pain into the backing file stuff. And my ultimate preference is for
> this backing file stuff to be removed again for a simple struct path.
> We're working around some more fundamental cleanup here imho.

Fair enough, we can rip the entire thing from vfs if you don't like it.
The user path file can be opened and stored internally by selinux
without adding all the associated risks in vfs.

Paul,

Please see compile tested code at:
https://github.com/amir73il/linux/commits/user_path_file/

Feel free to use my patch as is or reorder/squash/rebase as you see fit.
Feel free to attribute my contribution any way you see fit, just pls cc me for
review when you post v2.

Thanks,
Amir.

^ permalink raw reply

* Re: [PATCH v6] backing_file: store user_path_file
From: Amir Goldstein @ 2026-03-19 14:53 UTC (permalink / raw)
  To: Paul Moore
  Cc: Christian Brauner, Miklos Szeredi, Gao Xiang,
	linux-security-module, selinux, linux-erofs, linux-fsdevel,
	linux-unionfs
In-Reply-To: <CAHC9VhTKvOzMS2ZyawE=qBN-EpyucfxvwcHZYjXF5zugRhE2rw@mail.gmail.com>

On Thu, Mar 19, 2026 at 12:47 AM Paul Moore <paul@paul-moore.com> wrote:
>
> On Wed, Mar 18, 2026 at 9:13 AM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > Instead of storing the user_path, store an O_PATH file for the
> > user_path with the original user file creds and a security context.
> >
> > The user_path_file is only exported as a const pointer and its refcnt
> > is initialized to FILE_REF_DEAD, because it is not a refcounted object.
> >
> > The file_ref_init() helper was changed to accept the FILE_REF_ constant
> > instead of the fake +1 integer count.
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >
> > Christian,
> >
> > My v5 patch was sent by Paul along with his LSM/selinux pataches [1].
> > Here are the changes you requested.
> >
> > I removed the ACKs and Tested-by because of the changes.
> >
> > Thanks,
> > Amir.
> >
> > Changes since v5:
> > - Restore file_ref_init() helper without refcnt -1 offset
> > - Future proofing errors from backing_file_open_user_path()
> >
> > [1] https://lore.kernel.org/r/20260316213606.374109-6-paul@paul-moore.com/
> >
> >  fs/backing-file.c            | 26 ++++++++++--------
> >  fs/erofs/ishare.c            | 13 +++++++--
> >  fs/file_table.c              | 53 ++++++++++++++++++++++++++++--------
> >  fs/fuse/passthrough.c        |  3 +-
> >  fs/internal.h                |  5 ++--
> >  fs/overlayfs/dir.c           |  3 +-
> >  fs/overlayfs/file.c          |  1 +
> >  include/linux/backing-file.h | 29 ++++++++++++++++++--
> >  include/linux/file_ref.h     |  4 +--
> >  9 files changed, 103 insertions(+), 34 deletions(-)
>
> Still works for me.  I'm going to update lsm/stable-7.0 with this
> patch so we can get some more linux-next testing.
>
> Tested-by: Paul Moore <paul@paul-moore.com>
>

Paul,

As you saw, syzbot found a nasty bug in this patch and it is too hard
to fix it without introducing more hazards.

Therefore, per Christian's request I am withdrawing this patch.

Please see compile tested alternative solution for selinux without
intrusive vfs change at:
https://github.com/amir73il/linux/commits/user_path_file/

Thanks,
Amir.

^ permalink raw reply

* Re: [PATCH] fs: allow vfs code to open an O_PATH file with negative dentry
From: Paul Moore @ 2026-03-19 15:54 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Christian Brauner, Al Viro, Miklos Szeredi, Gao Xiang,
	linux-security-module, selinux, linux-erofs, linux-fsdevel,
	linux-unionfs, syzbot+f34aab278bf5d664e2be
In-Reply-To: <CAOQ4uxiS1uM=mn4q6CQfganba1XcqyXYmXfQceWdfUVRFK_Pvg@mail.gmail.com>

On Thu, Mar 19, 2026 at 10:50 AM Amir Goldstein <amir73il@gmail.com> wrote:
> On Thu, Mar 19, 2026 at 2:13 PM Christian Brauner <brauner@kernel.org> wrote:
> > On Thu, Mar 19, 2026 at 01:46:16PM +0100, Amir Goldstein wrote:
> > > The fields f_mapping, f_wb_err, f_sb_err are irrelevant for O_PATH file.
> > > Skip setting them for O_PATH file, so that the O_PATH file could be
> > > opened with a negative dentry.
> > >
> > > This is not something that a user should be able to do, but vfs code,
> > > such as ovl_tmpfile() can use this to open a backing O_PATH tmpfile
> > > before instantiating the dentry.
> > >
> > > Reported-by: syzbot+f34aab278bf5d664e2be@syzkaller.appspotmail.com
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > ---
> > >
> > > Christian,
> > >
> > > This patch fixes the syzbot report [1] that the
> > > backing_file_user_path_file() patch [2] introduces.
> > >
> > > This is not the only possible fix, but it is the cleanest one IMO.
> > > There is a small risk in introducing a state of an O_PATH file with
> > > NULL f_inode, but I (and the bots that I asked) did not find any
> > > obvious risk in this state.
> > >
> > > Note that specifically, the user path inode is accessed via d_inode()
> > > and not via file_inode(), which makes this safe for file_user_inode()
> > > callers.
> > >
> > > BTW, I missed this regression with the original patch because I
> > > only ran the quick overlayfs sanity test.
> > >
> > > Now I ran a full quick fstest cycle and verified that the O_TMPFILE
> > > test case is covered and that the bug is detected.
> > >
> > > WDYT?
> > >
> > > Thanks,
> > > Amir.
> > >
> > > [1] https://syzkaller.appspot.com/bug?extid=f34aab278bf5d664e2be
> > > [2] https://lore.kernel.org/linux-fsdevel/20260318131258.1457101-1-amir73il@gmail.com/
> > >
> > >  fs/open.c | 7 ++++---
> > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/fs/open.c b/fs/open.c
> > > index 91f1139591abe..2004a8c0d9c97 100644
> > > --- a/fs/open.c
> > > +++ b/fs/open.c
> > > @@ -893,9 +893,6 @@ static int do_dentry_open(struct file *f,
> > >
> > >       path_get(&f->f_path);
> > >       f->f_inode = inode;
> > > -     f->f_mapping = inode->i_mapping;
> > > -     f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> > > -     f->f_sb_err = file_sample_sb_err(f);
> > >
> > >       if (unlikely(f->f_flags & O_PATH)) {
> > >               f->f_mode = FMODE_PATH | FMODE_OPENED;
> > > @@ -904,6 +901,10 @@ static int do_dentry_open(struct file *f,
> > >               return 0;
> > >       }
> > >
> > > +     f->f_mapping = inode->i_mapping;
> > > +     f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> > > +     f->f_sb_err = file_sample_sb_err(f);
> > > +
> > >       if ((f->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) {
> > >               i_readcount_inc(inode);
> > >       } else if (f->f_mode & FMODE_WRITE && !special_file(inode->i_mode)) {
> >
> > I think this is really ugly and I'm really unhappy that we should adjust
> > initialization of generic vfs code for this. My preference is to push
> > the pain into the backing file stuff. And my ultimate preference is for
> > this backing file stuff to be removed again for a simple struct path.
> > We're working around some more fundamental cleanup here imho.
>
> Fair enough, we can rip the entire thing from vfs if you don't like it.
> The user path file can be opened and stored internally by selinux
> without adding all the associated risks in vfs.
>
> Paul,
>
> Please see compile tested code at:
> https://github.com/amir73il/linux/commits/user_path_file/

No.  Definitely no.  Ignoring the fact that there is no reason we
should pushing this into the LSM, doing it in this way means it is
very likely that each LSM wanting to provide mmap/mprotect controls on
overlayfs will have to create a new O_PATH file.  No.

... and let me preemptively comment that this doesn't belong in the
LSM framework either.

As Christian already mentioned, this really needs to be addressed in
the backing file code, please do it there.

-- 
paul-moore.com

^ permalink raw reply

* Re: [PATCH v6] backing_file: store user_path_file
From: Paul Moore @ 2026-03-19 15:56 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Christian Brauner, Miklos Szeredi, Gao Xiang,
	linux-security-module, selinux, linux-erofs, linux-fsdevel,
	linux-unionfs
In-Reply-To: <CAOQ4uxj8SZqaFbrVifw0uw7skbUvJP1_nLWj4PYCeZuaLy5t_A@mail.gmail.com>

On Thu, Mar 19, 2026 at 10:54 AM Amir Goldstein <amir73il@gmail.com> wrote:
> On Thu, Mar 19, 2026 at 12:47 AM Paul Moore <paul@paul-moore.com> wrote:
> > On Wed, Mar 18, 2026 at 9:13 AM Amir Goldstein <amir73il@gmail.com> wrote:
> > >
> > > Instead of storing the user_path, store an O_PATH file for the
> > > user_path with the original user file creds and a security context.
> > >
> > > The user_path_file is only exported as a const pointer and its refcnt
> > > is initialized to FILE_REF_DEAD, because it is not a refcounted object.
> > >
> > > The file_ref_init() helper was changed to accept the FILE_REF_ constant
> > > instead of the fake +1 integer count.
> > >
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > ---
> > >
> > > Christian,
> > >
> > > My v5 patch was sent by Paul along with his LSM/selinux pataches [1].
> > > Here are the changes you requested.
> > >
> > > I removed the ACKs and Tested-by because of the changes.
> > >
> > > Thanks,
> > > Amir.
> > >
> > > Changes since v5:
> > > - Restore file_ref_init() helper without refcnt -1 offset
> > > - Future proofing errors from backing_file_open_user_path()
> > >
> > > [1] https://lore.kernel.org/r/20260316213606.374109-6-paul@paul-moore.com/
> > >
> > >  fs/backing-file.c            | 26 ++++++++++--------
> > >  fs/erofs/ishare.c            | 13 +++++++--
> > >  fs/file_table.c              | 53 ++++++++++++++++++++++++++++--------
> > >  fs/fuse/passthrough.c        |  3 +-
> > >  fs/internal.h                |  5 ++--
> > >  fs/overlayfs/dir.c           |  3 +-
> > >  fs/overlayfs/file.c          |  1 +
> > >  include/linux/backing-file.h | 29 ++++++++++++++++++--
> > >  include/linux/file_ref.h     |  4 +--
> > >  9 files changed, 103 insertions(+), 34 deletions(-)
> >
> > Still works for me.  I'm going to update lsm/stable-7.0 with this
> > patch so we can get some more linux-next testing.
> >
> > Tested-by: Paul Moore <paul@paul-moore.com>
> >
>
> Paul,
>
> As you saw, syzbot found a nasty bug in this patch and it is too hard
> to fix it without introducing more hazards.
>
> Therefore, per Christian's request I am withdrawing this patch.
>
> Please see compile tested alternative solution for selinux without
> intrusive vfs change at:
> https://github.com/amir73il/linux/commits/user_path_file/

Let's let this thread die in favor of the other.  I'll already
commented there, but the quick summary is that pushing the ugliness
into an individual LSM, or the LSM framework itself, is not a
solution.

-- 
paul-moore.com

^ permalink raw reply

* Re: linux-next: Tree for Mar 18 (landlock docs)
From: Mickaël Salaün @ 2026-03-19 17:37 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Mark Brown, Linux Next Mailing List, Linux Kernel Mailing List,
	Dan Cojocaru, git, linux-security-module, Günther Noack
In-Reply-To: <f2a83432-f0ee-40a2-a3d0-71eef9db48fc@infradead.org>

On Wed, Mar 18, 2026 at 03:12:39PM -0700, Randy Dunlap wrote:
> 
> 
> On 3/18/26 9:52 AM, Mark Brown wrote:
> > Hi all,
> > 
> > Changes since 20260317:
> > 
> 
> linux-next/Documentation/userspace-api/landlock.rst:198: ERROR: Error in "code-block" directive:
> maximum 1 argument(s) allowed, 105 supplied.
> ^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> .. code-block:: c
>     __u32 restrict_flags =
> 
> 
> Please insert a blank line between ".. code-block:: c"
> and the code.
> 

Thanks, it's now fixed and rebased.

> 
> d37d3347aa59 ("landlock: Expand restrict flags example for ABI version 8")
> 
> -- 
> ~Randy
> 
> 

^ permalink raw reply

* Re: [PATCH v1] selftests/landlock: Test tsync interruption and cancellation paths
From: Mickaël Salaün @ 2026-03-19 17:47 UTC (permalink / raw)
  To: Günther Noack
  Cc: Günther Noack, linux-security-module, Justin Suess,
	Tingmao Wang, Yihan Ding
In-Reply-To: <20260314.61cb1f2dc01d@gnoack.org>

On Sat, Mar 14, 2026 at 10:10:23PM +0100, Günther Noack wrote:
> Hello Mickaël!
> 
> On Tue, Mar 10, 2026 at 08:04:15PM +0100, Mickaël Salaün wrote:
> > Add tsync_interrupt test to exercise the signal interruption path in
> > landlock_restrict_sibling_threads().  When a signal interrupts
> > wait_for_completion_interruptible() while the calling thread waits for
> > sibling threads to finish credential preparation, the kernel:
> > 
> > 1. Sets ERESTARTNOINTR to request a transparent syscall restart.
> > 2. Calls cancel_tsync_works() to opportunistically dequeue task works
> >    that have not started running yet.
> > 3. Breaks out of the preparation loop, then unblocks remaining
> >    task works via complete_all() and waits for them to finish.
> > 4. Returns the error, causing abort_creds() in the syscall handler.
> > 
> > Specifically, cancel_tsync_works() in its entirety, the ERESTARTNOINTR
> > error branch in landlock_restrict_sibling_threads(), and the
> > abort_creds() error branch in the landlock_restrict_self() syscall
> > handler are timing-dependent and not exercised by the existing tsync
> > tests, making code coverage measurements non-deterministic.
> > 
> > The test spawns a signaler thread that rapidly sends SIGUSR1 to the
> > calling thread while it performs landlock_restrict_self() with
> > LANDLOCK_RESTRICT_SELF_TSYNC.  Since ERESTARTNOINTR causes a
> > transparent restart, userspace always sees the syscall succeed.
> > 
> > This is a best-effort coverage test: the interruption path is exercised
> > when the signal lands during the preparation wait, which depends on
> > thread scheduling.  The test creates enough idle sibling threads (200)
> > to ensure multiple serialized waves of credential preparation even on
> > machines with many cores (e.g., 64), widening the window for the
> > signaler.  Deterministic coverage would require wrapping the wait call
> > with ALLOW_ERROR_INJECTION() and using CONFIG_FAIL_FUNCTION.
> > 
> > Cc: Günther Noack <gnoack@google.com>
> > Cc: Justin Suess <utilityemal77@gmail.com>
> > Cc: Tingmao Wang <m@maowtm.org>
> > Cc: Yihan Ding <dingyihan@uniontech.com>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > ---
> >  tools/testing/selftests/landlock/tsync_test.c | 91 ++++++++++++++++++-
> >  1 file changed, 90 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/testing/selftests/landlock/tsync_test.c b/tools/testing/selftests/landlock/tsync_test.c
> > index 37ef0d2270db..2b9ad4f154f4 100644
> > --- a/tools/testing/selftests/landlock/tsync_test.c
> > +++ b/tools/testing/selftests/landlock/tsync_test.c
> > @@ -6,9 +6,10 @@
> >   */
> >  
> >  #define _GNU_SOURCE
> > +#include <linux/landlock.h>
> >  #include <pthread.h>
> > +#include <signal.h>
> >  #include <sys/prctl.h>
> > -#include <linux/landlock.h>
> >  
> >  #include "common.h"
> >  
> > @@ -158,4 +159,92 @@ TEST(competing_enablement)
> >  	EXPECT_EQ(0, close(ruleset_fd));
> >  }
> >  
> > +static void signal_nop_handler(int sig)
> > +{
> > +}
> > +
> > +struct signaler_data {
> > +	pthread_t target;
> > +	volatile bool stop;
> > +};
> > +
> > +static void *signaler_thread(void *data)
> > +{
> > +	struct signaler_data *sd = data;
> > +
> > +	while (!sd->stop)
> > +		pthread_kill(sd->target, SIGUSR1);
> > +
> > +	return NULL;
> > +}
> > +
> > +/*
> > + * Number of idle sibling threads.  This must be large enough that even on
> > + * machines with many cores, the sibling threads cannot all complete their
> > + * credential preparation in a single parallel wave, otherwise the signaler
> > + * thread has no window to interrupt wait_for_completion_interruptible().
> > + * 200 threads on a 64-core machine yields ~3 serialized waves, giving the
> > + * tight signal loop enough time to land an interruption.
> > + */
> > +#define NUM_IDLE_THREADS 200
> > +
> > +/*
> > + * Exercises the tsync interruption and cancellation paths in tsync.c.
> > + *
> > + * When a signal interrupts the calling thread while it waits for sibling
> > + * threads to finish their credential preparation
> > + * (wait_for_completion_interruptible in landlock_restrict_sibling_threads),
> > + * the kernel sets ERESTARTNOINTR, cancels queued task works that have not
> > + * started yet (cancel_tsync_works), then waits for the remaining works to
> > + * finish.  On the error return, syscalls.c aborts the prepared credentials.
> > + * The kernel automatically restarts the syscall, so userspace sees success.
> > + */
> > +TEST(tsync_interrupt)
> > +{
> > +	size_t i;
> > +	pthread_t threads[NUM_IDLE_THREADS];
> > +	pthread_t signaler;
> > +	struct signaler_data sd;
> > +	struct sigaction sa = {};
> > +	const int ruleset_fd = create_ruleset(_metadata);
> > +
> > +	disable_caps(_metadata);
> > +
> > +	/* Install a no-op SIGUSR1 handler so the signal does not kill us. */
> > +	sa.sa_handler = signal_nop_handler;
> > +	sigemptyset(&sa.sa_mask);
> > +	ASSERT_EQ(0, sigaction(SIGUSR1, &sa, NULL));
> > +
> > +	ASSERT_EQ(0, prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0));
> > +
> > +	for (i = 0; i < NUM_IDLE_THREADS; i++)
> > +		ASSERT_EQ(0, pthread_create(&threads[i], NULL, idle, NULL));
> > +
> > +	/*
> > +	 * Start a signaler thread that continuously sends SIGUSR1 to the
> > +	 * calling thread.  This maximizes the chance of interrupting
> > +	 * wait_for_completion_interruptible() in the kernel's tsync path.
> > +	 */
> > +	sd.target = pthread_self();
> > +	sd.stop = false;
> > +	ASSERT_EQ(0, pthread_create(&signaler, NULL, signaler_thread, &sd));
> > +
> > +	/*
> > +	 * The syscall may be interrupted and transparently restarted by the
> > +	 * kernel (ERESTARTNOINTR).  From userspace, it should always succeed.
> > +	 */
> > +	EXPECT_EQ(0, landlock_restrict_self(ruleset_fd,
> > +					    LANDLOCK_RESTRICT_SELF_TSYNC));
> > +
> > +	sd.stop = true;
> > +	ASSERT_EQ(0, pthread_join(signaler, NULL));
> > +
> > +	for (i = 0; i < NUM_IDLE_THREADS; i++) {
> > +		ASSERT_EQ(0, pthread_cancel(threads[i]));
> > +		ASSERT_EQ(0, pthread_join(threads[i], NULL));
> > +	}
> > +
> > +	EXPECT_EQ(0, close(ruleset_fd));
> > +}
> > +
> >  TEST_HARNESS_MAIN
> > -- 
> > 2.53.0
> 
> The purpose of a test is to catch errors, so I broke the
> ERESTARTNOINTR error handling code path, but I could not get the test
> to fail.  Did you manage to reproduce any of these bugs with it, by
> any chance, and in what configuration did that work?  I tried with
> both QEMU (a bit more) and UML (a bit less), but had no luck.
> 
> (Does this need to run in a loop like the Syzkaller-generated deadlock
> reproducer, so that we have a chance of catching these bugs at all?)

There is no bug and the test should always pass, it's just to improve
test coverage in the new kernel code introduced by the tsync feature.

Without this patch, most of the time we get: 90.2% of 2105 lines.

With this patch, we always get: 91.1% of 2105 lines.

^ permalink raw reply

* Re: [PATCH] lsm: Fix the crash issue in xfrm_decode_session
From: Casey Schaufler @ 2026-03-19 17:51 UTC (permalink / raw)
  To: Feng Yang
  Cc: jmorris, linux-kernel, linux-security-module, paul, serge,
	Casey Schaufler
In-Reply-To: <20260319022208.69924-1-yangfeng59949@163.com>

On 3/18/2026 7:22 PM, Feng Yang wrote:
> On Wed, 18 Mar 2026 10:09:47 -0700, Casey Schaufler wrote:
>> On 3/17/2026 11:19 PM, Feng Yang wrote:
>>> From: Feng Yang <yangfeng@kylinos.cn>
>>>
>>> After hooking the following BPF program:
>>> SEC("lsm/xfrm_decode_session")
>>> int BPF_PROG(lsm_hook_xfrm_decode_session, struct sk_buff *skb, u32 *secid, int ckall)
>>> {
>>>     return 1; // Any non-zero value
>>> }
>>> Subsequent packet transmission triggers will cause a kernel panic:
>> LSM hooks that use or provide secids cannot be stacked. That is,
>> you can't provide a BPF LSM hook and an SELinux LSM hook and expect
>> correct behavior. Your proposed "fix" removes a legitimate check.
> I'm very sorry, I didn't quite understand what you meant.
>
> Maybe my commit message wasn't clear. I only used a BPF LSM hook without SELinux stacking enabled.

Do i understand correctly that you do not have SELinux enabled?

> Therefore, is it the expected behavior that simply using
> SEC("lsm/xfrm_decode_session")
> int BPF_PROG(lsm_hook_xfrm_decode_session, struct sk_buff *skb, u32 *secid, int ckall) {
>     return -1;
> }
> would cause a kernel panic?

Yes. As the infrastructure is written, any return other than 0 is erroneous.
You will need to query the SELinux developers about why they decided to
implement the xfrm handling in this way.

>  If not, and if the BUG_ON check is still necessary,
> then does it mean we need to modify the return value validation logic in the BPF
> verifier to ensure that only BPF programs returning 0 are accepted for this hook?
>
> Thanks.
>
>

^ permalink raw reply

* Re: [PATCH] lsm: Fix the crash issue in xfrm_decode_session
From: Stephen Smalley @ 2026-03-19 18:22 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: Feng Yang, jmorris, linux-kernel, linux-security-module, paul,
	serge
In-Reply-To: <80ff003b-51fd-4ec8-9505-dfc1c5890ced@schaufler-ca.com>

On Thu, Mar 19, 2026 at 1:52 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 3/18/2026 7:22 PM, Feng Yang wrote:
> > On Wed, 18 Mar 2026 10:09:47 -0700, Casey Schaufler wrote:
> >> On 3/17/2026 11:19 PM, Feng Yang wrote:
> >>> From: Feng Yang <yangfeng@kylinos.cn>
> >>>
> >>> After hooking the following BPF program:
> >>> SEC("lsm/xfrm_decode_session")
> >>> int BPF_PROG(lsm_hook_xfrm_decode_session, struct sk_buff *skb, u32 *secid, int ckall)
> >>> {
> >>>     return 1; // Any non-zero value
> >>> }
> >>> Subsequent packet transmission triggers will cause a kernel panic:
> >> LSM hooks that use or provide secids cannot be stacked. That is,
> >> you can't provide a BPF LSM hook and an SELinux LSM hook and expect
> >> correct behavior. Your proposed "fix" removes a legitimate check.
> > I'm very sorry, I didn't quite understand what you meant.
> >
> > Maybe my commit message wasn't clear. I only used a BPF LSM hook without SELinux stacking enabled.
>
> Do i understand correctly that you do not have SELinux enabled?
>
> > Therefore, is it the expected behavior that simply using
> > SEC("lsm/xfrm_decode_session")
> > int BPF_PROG(lsm_hook_xfrm_decode_session, struct sk_buff *skb, u32 *secid, int ckall) {
> >     return -1;
> > }
> > would cause a kernel panic?
>
> Yes. As the infrastructure is written, any return other than 0 is erroneous.
> You will need to query the SELinux developers about why they decided to
> implement the xfrm handling in this way.

Looks like this BUG_ON() has just been preserved since first being
introduced by:
https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/?id=beb8d13bed80f8388f1a9a107d07ddd342e627e8

I think we can remove it at this point - it evidently doesn't ever
trigger for SELinux or we would have seen it by now and it is
definitely unsafe for BPF LSM.

^ permalink raw reply

* Re: [PATCH] fs: allow vfs code to open an O_PATH file with negative dentry
From: Amir Goldstein @ 2026-03-19 18:30 UTC (permalink / raw)
  To: Paul Moore
  Cc: Christian Brauner, Al Viro, Miklos Szeredi, Gao Xiang,
	linux-security-module, selinux, linux-erofs, linux-fsdevel,
	linux-unionfs, syzbot+f34aab278bf5d664e2be
In-Reply-To: <CAHC9VhQDXHh_=X-OKD9oN8fapVXBwLakNp4rEHLUL0cQ1RSteA@mail.gmail.com>

On Thu, Mar 19, 2026 at 4:55 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Thu, Mar 19, 2026 at 10:50 AM Amir Goldstein <amir73il@gmail.com> wrote:
> > On Thu, Mar 19, 2026 at 2:13 PM Christian Brauner <brauner@kernel.org> wrote:
> > > On Thu, Mar 19, 2026 at 01:46:16PM +0100, Amir Goldstein wrote:
> > > > The fields f_mapping, f_wb_err, f_sb_err are irrelevant for O_PATH file.
> > > > Skip setting them for O_PATH file, so that the O_PATH file could be
> > > > opened with a negative dentry.
> > > >
> > > > This is not something that a user should be able to do, but vfs code,
> > > > such as ovl_tmpfile() can use this to open a backing O_PATH tmpfile
> > > > before instantiating the dentry.
> > > >
> > > > Reported-by: syzbot+f34aab278bf5d664e2be@syzkaller.appspotmail.com
> > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > > ---
> > > >
> > > > Christian,
> > > >
> > > > This patch fixes the syzbot report [1] that the
> > > > backing_file_user_path_file() patch [2] introduces.
> > > >
> > > > This is not the only possible fix, but it is the cleanest one IMO.
> > > > There is a small risk in introducing a state of an O_PATH file with
> > > > NULL f_inode, but I (and the bots that I asked) did not find any
> > > > obvious risk in this state.
> > > >
> > > > Note that specifically, the user path inode is accessed via d_inode()
> > > > and not via file_inode(), which makes this safe for file_user_inode()
> > > > callers.
> > > >
> > > > BTW, I missed this regression with the original patch because I
> > > > only ran the quick overlayfs sanity test.
> > > >
> > > > Now I ran a full quick fstest cycle and verified that the O_TMPFILE
> > > > test case is covered and that the bug is detected.
> > > >
> > > > WDYT?
> > > >
> > > > Thanks,
> > > > Amir.
> > > >
> > > > [1] https://syzkaller.appspot.com/bug?extid=f34aab278bf5d664e2be
> > > > [2] https://lore.kernel.org/linux-fsdevel/20260318131258.1457101-1-amir73il@gmail.com/
> > > >
> > > >  fs/open.c | 7 ++++---
> > > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/fs/open.c b/fs/open.c
> > > > index 91f1139591abe..2004a8c0d9c97 100644
> > > > --- a/fs/open.c
> > > > +++ b/fs/open.c
> > > > @@ -893,9 +893,6 @@ static int do_dentry_open(struct file *f,
> > > >
> > > >       path_get(&f->f_path);
> > > >       f->f_inode = inode;
> > > > -     f->f_mapping = inode->i_mapping;
> > > > -     f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> > > > -     f->f_sb_err = file_sample_sb_err(f);
> > > >
> > > >       if (unlikely(f->f_flags & O_PATH)) {
> > > >               f->f_mode = FMODE_PATH | FMODE_OPENED;
> > > > @@ -904,6 +901,10 @@ static int do_dentry_open(struct file *f,
> > > >               return 0;
> > > >       }
> > > >
> > > > +     f->f_mapping = inode->i_mapping;
> > > > +     f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> > > > +     f->f_sb_err = file_sample_sb_err(f);
> > > > +
> > > >       if ((f->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) {
> > > >               i_readcount_inc(inode);
> > > >       } else if (f->f_mode & FMODE_WRITE && !special_file(inode->i_mode)) {
> > >
> > > I think this is really ugly and I'm really unhappy that we should adjust
> > > initialization of generic vfs code for this. My preference is to push
> > > the pain into the backing file stuff. And my ultimate preference is for
> > > this backing file stuff to be removed again for a simple struct path.
> > > We're working around some more fundamental cleanup here imho.
> >
> > Fair enough, we can rip the entire thing from vfs if you don't like it.
> > The user path file can be opened and stored internally by selinux
> > without adding all the associated risks in vfs.
> >
> > Paul,
> >
> > Please see compile tested code at:
> > https://github.com/amir73il/linux/commits/user_path_file/
>
> No.  Definitely no.  Ignoring the fact that there is no reason we
> should pushing this into the LSM, doing it in this way means it is
> very likely that each LSM wanting to provide mmap/mprotect controls on
> overlayfs will have to create a new O_PATH file.  No.
>
> ... and let me preemptively comment that this doesn't belong in the
> LSM framework either.
>
> As Christian already mentioned, this really needs to be addressed in
> the backing file code, please do it there.
>

OK, will give it another try.

Thanks,
Amir.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox