* Re: [PATCH 1/3] lsm: add hook for firmware command validation
From: Jonathan Cameron @ 2026-03-09 15:02 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Paul Moore, James Morris, Serge E. Hallyn, Jason Gunthorpe,
Saeed Mahameed, Itay Avraham, Dave Jiang, linux-security-module,
linux-kernel, linux-rdma, Chiara Meiohas, Maher Sanalla,
Edward Srouji
In-Reply-To: <20260309-fw-lsm-hook-v1-1-4a6422e63725@nvidia.com>
On Mon, 9 Mar 2026 13:15:18 +0200
Leon Romanovsky <leon@kernel.org> wrote:
> From: Chiara Meiohas <cmeiohas@nvidia.com>
>
> Drivers typically communicate with device firmware either via
> register-based commands (writing parameters into device registers)
> or by passing a command buffer using shared-memory mechanisms.
>
> This hook targets the command buffer mechanism, which is commonly
> used on modern, complex devices.
>
> Add the LSM hook fw_validate_cmd. This hook allows inspecting
> firmware command buffers before they are sent to the device.
> The hook receives the command buffer, device, command class, and a
> class-specific id:
> - class_id (enum fw_cmd_class) allows security modules to
> differentiate between classes of firmware commands.
> In this series, class_id distinguishes between commands from the
> RDMA uverbs interface and from fwctl.
> - id is a class-specific device identifier. For uverbs, id is the
> RDMA driver identifier (enum rdma_driver_id). For fwctl, id is the
> device type (enum fwctl_device_type).
>
> Signed-off-by: Chiara Meiohas <cmeiohas@nvidia.com>
> Reviewed-by: Maher Sanalla <msanalla@nvidia.com>
> Signed-off-by: Edward Srouji <edwards@nvidia.com>
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
Hi Leon,
To me this seems sensible, but LSM isn't an area I know that much about.
With that in mind:
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
A few formatting related comments inline.
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 83a646d72f6f8..64786d013207a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -67,6 +67,7 @@ enum fs_value_type;
> struct watch;
> struct watch_notification;
> struct lsm_ctx;
> +struct device;
>
> /* Default (no) options for the capable function */
> #define CAP_OPT_NONE 0x0
> @@ -157,6 +158,21 @@ enum lockdown_reason {
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
>
> +/*
Could add the MAX entry and making this /**
The file is a bit inconsistent on that.
> + * enum fw_cmd_class - Class of the firmware command passed to
> + * security_fw_validate_cmd.
> + * This allows security modules to distinguish between different command
> + * classes.
> + *
> + * @FW_CMD_CLASS_UVERBS: Command originated from the RDMA uverbs interface
> + * @FW_CMD_CLASS_FWCTL: Command originated from the fwctl interface
> + */
> +enum fw_cmd_class {
> + FW_CMD_CLASS_UVERBS,
> + FW_CMD_CLASS_FWCTL,
> + FW_CMD_CLASS_MAX,
Nitpick. Drop the trailing comma to make it a tiny bit more obvious if
someone accidentally adds anything after this counting entry.
> +};
> +
> /*
> * Data exported by the security modules
> */
> @@ -575,6 +591,9 @@ int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
> int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
> int security_inode_getsecctx(struct inode *inode, struct lsm_context *cp);
> int security_locked_down(enum lockdown_reason what);
> +int security_fw_validate_cmd(const void *in, size_t in_len,
> + const struct device *dev,
> + enum fw_cmd_class class_id, u32 id);
> int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, u32 *uctx_len,
> void *val, size_t val_len, u64 id, u64 flags);
> int security_bdev_alloc(struct block_device *bdev);
> @@ -1589,6 +1608,12 @@ static inline int security_locked_down(enum lockdown_reason what)
> {
> return 0;
> }
> +static inline int security_fw_validate_cmd(const void *in, size_t in_len,
> + const struct device *dev,
> + enum fw_cmd_class class_id, u32 id)
> +{
> + return 0;
> +}
> static inline int lsm_fill_user_ctx(struct lsm_ctx __user *uctx,
> u32 *uctx_len, void *val, size_t val_len,
> u64 id, u64 flags)
> diff --git a/security/security.c b/security/security.c
> index 67af9228c4e94..d05941fe89a48 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -5373,6 +5373,32 @@ int security_locked_down(enum lockdown_reason what)
> }
> EXPORT_SYMBOL(security_locked_down);
>
> +/**
> + * security_fw_validate_cmd() - Validate a firmware command
> + * @in: pointer to the firmware command input buffer
> + * @in_len: length of the firmware command input buffer
> + * @dev: device associated with the command
> + * @class_id: class of the firmware command
> + * @id: device identifier, specific to the command @class_id
> + *
> + * Check permissions before sending a firmware command generated by
> + * userspace to the device.
> + *
> + * Return: Returns 0 if permission is granted.
> + */
> +int security_fw_validate_cmd(const void *in, size_t in_len,
> + const struct device *dev,
> + enum fw_cmd_class class_id,
> + u32 id)
I'd follow the wrapping you have in the header and have id on the line
above.
> +{
> + if (class_id >= FW_CMD_CLASS_MAX)
> + return -EINVAL;
> +
> + return call_int_hook(fw_validate_cmd, in, in_len,
> + dev, class_id, id);
Fits on one line < 80 chars.
> +}
> +EXPORT_SYMBOL_GPL(security_fw_validate_cmd);
> +
> /**
> * security_bdev_alloc() - Allocate a block device LSM blob
> * @bdev: block device
>
^ permalink raw reply
* Re: [PATCH] ima: check return value of crypto_shash_final() in boot aggregate
From: Mimi Zohar @ 2026-03-09 15:03 UTC (permalink / raw)
To: Daniel Hodges, Roberto Sassu, Dmitry Kasatkin
Cc: Eric Snowberg, Paul Moore, James Morris, Serge E . Hallyn,
linux-integrity, linux-security-module, linux-kernel
In-Reply-To: <20260201024015.2862236-1-hodgesd@meta.com>
On Sat, 2026-01-31 at 18:40 -0800, Daniel Hodges wrote:
> The return value of crypto_shash_final() is not checked in
> ima_calc_boot_aggregate_tfm(). If the hash finalization fails, the
> function returns success and a corrupted boot aggregate digest could
> be used for IMA measurements.
>
> Capture the return value and propagate any error to the caller.
>
> Fixes: 76bb28f6126f ("ima: use new crypto_shash API instead of old crypto_hash")
> Signed-off-by: Daniel Hodges <hodgesd@meta.com>
Thanks, Daniel. The patch is now queueud.
Mimi
^ permalink raw reply
* Re: [PATCH v2 v2] evm: check return values of crypto_shash functions
From: Mimi Zohar @ 2026-03-09 15:03 UTC (permalink / raw)
To: Roberto Sassu, Daniel Hodges
Cc: roberto.sassu, dmitry.kasatkin, eric.snowberg, paul, jmorris,
serge, linux-integrity, linux-security-module, linux-kernel
In-Reply-To: <6ce273a26b396232f3ee64a980575562e766c501.camel@huaweicloud.com>
On Thu, 2026-02-19 at 10:26 +0100, Roberto Sassu wrote:
> On Thu, 2026-02-05 at 21:42 -0500, Daniel Hodges wrote:
> > The crypto_shash_update() and crypto_shash_final() functions can fail
> > and return error codes, but their return values were not being checked
> > in several places in security/integrity/evm/evm_crypto.c:
> >
> > - hmac_add_misc() ignored returns from crypto_shash_update() and
> > crypto_shash_final()
> > - evm_calc_hmac_or_hash() ignored returns from crypto_shash_update()
> > - evm_init_hmac() ignored returns from crypto_shash_update()
> >
> > If these hash operations fail silently, the resulting HMAC could be
> > invalid or incomplete, which could weaken the integrity verification
> > security that EVM provides.
> >
> > This patch converts hmac_add_misc() from void to int return type and
> > adds proper error checking and propagation for all crypto_shash_*
> > function calls. All callers are updated to handle the new return values.
> > Additionally, error messages are logged when cryptographic operations
> > fail to provide visibility into the failure rather than silently
> > returning error codes.
> >
> > Fixes: 66dbc325afce ("evm: re-release")
> > Signed-off-by: Daniel Hodges <git@danielhodges.dev>
>
> After fixing the minor issue below:
>
> Reviewed-by: Roberto Sassu <roberto.sassu@huawei.com>
Thanks Daniel, Roberto. Daniel there are a couple of places where the line
length is greater than 80. To see them, add "--max-line-length=80" to
scripts/checkpatch.pl. I'd appreciate your fixing them. Otherwise, the patch
looks good.
Thanks,
Mimi
^ permalink raw reply
* Re: [PATCH 2/3] RDMA/mlx5: Invoke fw_validate_cmd LSM hook for DEVX commands
From: Jonathan Cameron @ 2026-03-09 15:10 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Paul Moore, James Morris, Serge E. Hallyn, Jason Gunthorpe,
Saeed Mahameed, Itay Avraham, Dave Jiang, linux-security-module,
linux-kernel, linux-rdma, Chiara Meiohas, Maher Sanalla,
Edward Srouji
In-Reply-To: <20260309-fw-lsm-hook-v1-2-4a6422e63725@nvidia.com>
On Mon, 9 Mar 2026 13:15:19 +0200
Leon Romanovsky <leon@kernel.org> wrote:
> From: Chiara Meiohas <cmeiohas@nvidia.com>
>
> DEVX is an RDMA uverbs extension that allows userspace to submit
> firmware command buffers. The driver inspects the command and then
> passes the buffer through for firmware execution.
>
> Call security_fw_validate_cmd() before dispatching firmware commands
> through DEVX.
>
> This allows security modules to implement custom policies and
> enforce per-command security policy on user-triggered firmware
> commands. For example, a BPF LSM program could restrict specific
> firmware operations to privileged users.
>
> Signed-off-by: Chiara Meiohas <cmeiohas@nvidia.com>
> Reviewed-by: Maher Sanalla <msanalla@nvidia.com>
> Signed-off-by: Edward Srouji <edwards@nvidia.com>
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
A few formatting inconsistencies. Actual code is fine as far as I can tell.
So given I don't know the driver, take this as vague at best...
Assuming formatting stuff tidied up.
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> ---
> drivers/infiniband/hw/mlx5/devx.c | 52 ++++++++++++++++++++++++++++++---------
> 1 file changed, 40 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/infiniband/hw/mlx5/devx.c b/drivers/infiniband/hw/mlx5/devx.c
> index 0066b2738ac89..48a2c4b4ad4eb 100644
> --- a/drivers/infiniband/hw/mlx5/devx.c
> +++ b/drivers/infiniband/hw/mlx5/devx.c
> @@ -18,6 +18,7 @@
> #include "devx.h"
> #include "qp.h"
> #include <linux/xarray.h>
> +#include <linux/security.h>
>
> #define UVERBS_MODULE_NAME mlx5_ib
> #include <rdma/uverbs_named_ioctl.h>
> @@ -1111,6 +1112,8 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OTHER)(
> struct mlx5_ib_dev *dev;
> void *cmd_in = uverbs_attr_get_alloced_ptr(
> attrs, MLX5_IB_ATTR_DEVX_OTHER_CMD_IN);
> + int cmd_in_len = uverbs_attr_get_len(attrs,
> + MLX5_IB_ATTR_DEVX_OTHER_CMD_IN);
> int cmd_out_len = uverbs_attr_get_len(attrs,
> MLX5_IB_ATTR_DEVX_OTHER_CMD_OUT);
> void *cmd_out;
> @@ -1135,9 +1138,12 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OTHER)(
> return PTR_ERR(cmd_out);
>
> MLX5_SET(general_obj_in_cmd_hdr, cmd_in, uid, uid);
> - err = mlx5_cmd_do(dev->mdev, cmd_in,
> - uverbs_attr_get_len(attrs, MLX5_IB_ATTR_DEVX_OTHER_CMD_IN),
> - cmd_out, cmd_out_len);
> + err = security_fw_validate_cmd(cmd_in, cmd_in_len, &dev->ib_dev.dev,
> + FW_CMD_CLASS_UVERBS, RDMA_DRIVER_MLX5);
> + if (err)
> + return err;
> +
> + err = mlx5_cmd_do(dev->mdev, cmd_in, cmd_in_len, cmd_out, cmd_out_len);
See below for inconsistency on how the changes have been done.
> if (err && err != -EREMOTEIO)
> return err;
>
> @@ -1570,6 +1576,12 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_CREATE)(
> devx_set_umem_valid(cmd_in);
> }
>
> + err = security_fw_validate_cmd(cmd_in, cmd_in_len,
> + &dev->ib_dev.dev,
> + FW_CMD_CLASS_UVERBS, RDMA_DRIVER_MLX5);
> + if (err)
> + goto obj_free;
> +
> if (opcode == MLX5_CMD_OP_CREATE_DCT) {
> obj->flags |= DEVX_OBJ_FLAGS_DCT;
> err = mlx5_core_create_dct(dev, &obj->core_dct, cmd_in,
> @@ -1582,8 +1594,8 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_CREATE)(
> cmd_in, cmd_in_len, cmd_out,
> cmd_out_len);
> } else {
> - err = mlx5_cmd_do(dev->mdev, cmd_in, cmd_in_len,
> - cmd_out, cmd_out_len);
> + err = mlx5_cmd_do(dev->mdev, cmd_in, cmd_in_len, cmd_out,
> + cmd_out_len);
Unrelated bit of reformatting. Shouldn't be in this patch.,
> }
>
> if (err == -EREMOTEIO)
> @@ -1646,6 +1658,8 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_MODIFY)(
> struct uverbs_attr_bundle *attrs)
> {
> void *cmd_in = uverbs_attr_get_alloced_ptr(attrs, MLX5_IB_ATTR_DEVX_OBJ_MODIFY_CMD_IN);
> + int cmd_in_len = uverbs_attr_get_len(attrs,
> + MLX5_IB_ATTR_DEVX_OBJ_MODIFY_CMD_IN);
> int cmd_out_len = uverbs_attr_get_len(attrs,
> MLX5_IB_ATTR_DEVX_OBJ_MODIFY_CMD_OUT);
> struct ib_uobject *uobj = uverbs_attr_get_uobject(attrs,
> @@ -1676,9 +1690,12 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_MODIFY)(
>
> MLX5_SET(general_obj_in_cmd_hdr, cmd_in, uid, uid);
> devx_set_umem_valid(cmd_in);
> + err = security_fw_validate_cmd(cmd_in, cmd_in_len, &mdev->ib_dev.dev,
> + FW_CMD_CLASS_UVERBS, RDMA_DRIVER_MLX5);
> + if (err)
> + return err;
>
> - err = mlx5_cmd_do(mdev->mdev, cmd_in,
> - uverbs_attr_get_len(attrs, MLX5_IB_ATTR_DEVX_OBJ_MODIFY_CMD_IN),
> + err = mlx5_cmd_do(mdev->mdev, cmd_in, cmd_in_len,
> cmd_out, cmd_out_len);
Similar to case above, this not fits on one line.
> if (err && err != -EREMOTEIO)
> return err;
> @@ -1693,6 +1710,8 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_QUERY)(
> struct uverbs_attr_bundle *attrs)
> {
> void *cmd_in = uverbs_attr_get_alloced_ptr(attrs, MLX5_IB_ATTR_DEVX_OBJ_QUERY_CMD_IN);
> + int cmd_in_len = uverbs_attr_get_len(attrs,
> + MLX5_IB_ATTR_DEVX_OBJ_QUERY_CMD_IN);
> int cmd_out_len = uverbs_attr_get_len(attrs,
> MLX5_IB_ATTR_DEVX_OBJ_QUERY_CMD_OUT);
> struct ib_uobject *uobj = uverbs_attr_get_uobject(attrs,
> @@ -1722,8 +1741,12 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_QUERY)(
> return PTR_ERR(cmd_out);
>
> MLX5_SET(general_obj_in_cmd_hdr, cmd_in, uid, uid);
> - err = mlx5_cmd_do(mdev->mdev, cmd_in,
> - uverbs_attr_get_len(attrs, MLX5_IB_ATTR_DEVX_OBJ_QUERY_CMD_IN),
> + err = security_fw_validate_cmd(cmd_in, cmd_in_len, &mdev->ib_dev.dev,
> + FW_CMD_CLASS_UVERBS, RDMA_DRIVER_MLX5);
> + if (err)
> + return err;
> +
> + err = mlx5_cmd_do(mdev->mdev, cmd_in, cmd_in_len,
> cmd_out, cmd_out_len);
As above.
> if (err && err != -EREMOTEIO)
> return err;
> @@ -1832,6 +1855,8 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_ASYNC_QUERY)(
> {
> void *cmd_in = uverbs_attr_get_alloced_ptr(attrs,
> MLX5_IB_ATTR_DEVX_OBJ_QUERY_ASYNC_CMD_IN);
> + int cmd_in_len = uverbs_attr_get_len(attrs,
> + MLX5_IB_ATTR_DEVX_OBJ_QUERY_ASYNC_CMD_IN);
> struct ib_uobject *uobj = uverbs_attr_get_uobject(
> attrs,
> MLX5_IB_ATTR_DEVX_OBJ_QUERY_ASYNC_HANDLE);
> @@ -1894,9 +1919,12 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_ASYNC_QUERY)(
> async_data->ev_file = ev_file;
>
> MLX5_SET(general_obj_in_cmd_hdr, cmd_in, uid, uid);
> - err = mlx5_cmd_exec_cb(&ev_file->async_ctx, cmd_in,
> - uverbs_attr_get_len(attrs,
> - MLX5_IB_ATTR_DEVX_OBJ_QUERY_ASYNC_CMD_IN),
> + err = security_fw_validate_cmd(cmd_in, cmd_in_len, &mdev->ib_dev.dev,
> + FW_CMD_CLASS_UVERBS, RDMA_DRIVER_MLX5);
> + if (err)
> + goto free_async;
> +
> + err = mlx5_cmd_exec_cb(&ev_file->async_ctx, cmd_in, cmd_in_len,
> async_data->hdr.out_data,
> async_data->cmd_out_len,
> devx_query_callback, &async_data->cb_work);
>
^ permalink raw reply
* Re: [PATCH 3/3] fwctl/mlx5: Invoke fw_validate_cmd LSM hook for fwctl commands
From: Jonathan Cameron @ 2026-03-09 15:12 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Paul Moore, James Morris, Serge E. Hallyn, Jason Gunthorpe,
Saeed Mahameed, Itay Avraham, Dave Jiang, linux-security-module,
linux-kernel, linux-rdma, Chiara Meiohas, Maher Sanalla,
Edward Srouji
In-Reply-To: <20260309-fw-lsm-hook-v1-3-4a6422e63725@nvidia.com>
On Mon, 9 Mar 2026 13:15:20 +0200
Leon Romanovsky <leon@kernel.org> wrote:
> From: Chiara Meiohas <cmeiohas@nvidia.com>
>
> fwctl is subsystem which exposes a firmware interface directly to
> userspace: it allows userspace to send device specific command
> buffers to firmware.
>
> Call security_fw_validate_cmd() before dispatching the user-provided
> firmware command.
>
> This allows security modules to implement custom policies and
> enforce per-command security policy on user-triggered firmware
> commands. For example, a BPF LSM program could filter firmware
> commands based on their opcode.
>
> Signed-off-by: Chiara Meiohas <cmeiohas@nvidia.com>
> Reviewed-by: Maher Sanalla <msanalla@nvidia.com>
> Signed-off-by: Edward Srouji <edwards@nvidia.com>
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
LGTM
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
^ permalink raw reply
* Re: Subject: x86/msr + lockdown: allow access to **documented** RAPL/TCC controls under Secure Boot
From: Rafael J. Wysocki @ 2026-03-09 15:13 UTC (permalink / raw)
To: Artem S. Tashkinov
Cc: x86, Linux Kernel Mailing List, linux-pm, linux-efi,
linux-security-module, Srinivas Pandruvada, Zhang, Rui
In-Reply-To: <2780abfc-39d1-4441-833c-65e66f747054@gmx.com>
On Mon, Mar 9, 2026 at 1:24 PM Artem S. Tashkinov <aros@gmx.com> wrote:
>
> Hello,
>
> When Secure Boot is enabled and kernel lockdown is active, the x86 MSR
> driver blocks all raw MSR access from user space via `/dev/cpu/*/msr`.
> This effectively prevents legitimate use of documented CPU power and
> thermal management interfaces such as RAPL power limits (PL1/PL2) and
> the TCC/TjOffset control. These registers are part of Intel’s
> **publicly** documented architectural interface and have been stable
> across many generations of processors.
There is a power capping RAPL driver. What's the problem with it with
Secure Boot enabled?
> As a result, under Secure Boot Linux users lose the ability to read or
> adjust **standard** power-management controls that remain available
> through equivalent tooling on other operating systems.
The power capping RAPL driver is there, please use it. It is documented even.
There is also a driver for TCC/TjOffset control, it is called intel_tcc_cooling.
And there are utilities in user space (for example, Intel thermald)
that use those interfaces.
> The current all-or-nothing restriction appears broader than necessary
> for the stated goal of protecting kernel integrity. MSRs associated with
> power limits and TCC offset are not privileged debugging or microcode
> interfaces but standard hardware configuration knobs intended for
> platform power and thermal management.
>
> It would be useful if the kernel either allowed access to a small
> whitelist of such documented registers under lockdown or exposed a
> mediated kernel interface for adjusting them. Without such a mechanism,
> Secure Boot effectively disables legitimate and widely used
> power/thermal tuning functionality on modern Intel laptops.
>
> Most (if not all) Intel laptops don't expose or allow to configure
> PL1/PL2 limits in BIOS/EFI either.
Because it is not necessary to do so.
^ permalink raw reply
* Re: [PATCH 1/3] lsm: add hook for firmware command validation
From: Leon Romanovsky @ 2026-03-09 15:25 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Paul Moore, James Morris, Serge E. Hallyn, Jason Gunthorpe,
Saeed Mahameed, Itay Avraham, Dave Jiang, linux-security-module,
linux-kernel, linux-rdma, Chiara Meiohas, Maher Sanalla,
Edward Srouji
In-Reply-To: <20260309150253.00001ec7@huawei.com>
On Mon, Mar 09, 2026 at 03:02:53PM +0000, Jonathan Cameron wrote:
> On Mon, 9 Mar 2026 13:15:18 +0200
> Leon Romanovsky <leon@kernel.org> wrote:
>
> > From: Chiara Meiohas <cmeiohas@nvidia.com>
> >
> > Drivers typically communicate with device firmware either via
> > register-based commands (writing parameters into device registers)
> > or by passing a command buffer using shared-memory mechanisms.
> >
> > This hook targets the command buffer mechanism, which is commonly
> > used on modern, complex devices.
> >
> > Add the LSM hook fw_validate_cmd. This hook allows inspecting
> > firmware command buffers before they are sent to the device.
> > The hook receives the command buffer, device, command class, and a
> > class-specific id:
> > - class_id (enum fw_cmd_class) allows security modules to
> > differentiate between classes of firmware commands.
> > In this series, class_id distinguishes between commands from the
> > RDMA uverbs interface and from fwctl.
> > - id is a class-specific device identifier. For uverbs, id is the
> > RDMA driver identifier (enum rdma_driver_id). For fwctl, id is the
> > device type (enum fwctl_device_type).
> >
> > Signed-off-by: Chiara Meiohas <cmeiohas@nvidia.com>
> > Reviewed-by: Maher Sanalla <msanalla@nvidia.com>
> > Signed-off-by: Edward Srouji <edwards@nvidia.com>
> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> Hi Leon,
>
> To me this seems sensible, but LSM isn't an area I know that much about.
>
> With that in mind:
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
>
> A few formatting related comments inline.
Thanks for the feedback. I’ve addressed all comments and will send a new
revision within the next few days.
Thanks
^ permalink raw reply
* Re: [PATCH 3/3] fwctl/mlx5: Invoke fw_validate_cmd LSM hook for fwctl commands
From: Dave Jiang @ 2026-03-09 16:57 UTC (permalink / raw)
To: Leon Romanovsky, Paul Moore, James Morris, Serge E. Hallyn,
Jason Gunthorpe, Saeed Mahameed, Itay Avraham, Jonathan Cameron
Cc: linux-security-module, linux-kernel, linux-rdma, Chiara Meiohas,
Maher Sanalla, Edward Srouji
In-Reply-To: <20260309-fw-lsm-hook-v1-3-4a6422e63725@nvidia.com>
On 3/9/26 4:15 AM, Leon Romanovsky wrote:
> From: Chiara Meiohas <cmeiohas@nvidia.com>
>
> fwctl is subsystem which exposes a firmware interface directly to
> userspace: it allows userspace to send device specific command
> buffers to firmware.
>
> Call security_fw_validate_cmd() before dispatching the user-provided
> firmware command.
>
> This allows security modules to implement custom policies and
> enforce per-command security policy on user-triggered firmware
> commands. For example, a BPF LSM program could filter firmware
> commands based on their opcode.
>
> Signed-off-by: Chiara Meiohas <cmeiohas@nvidia.com>
> Reviewed-by: Maher Sanalla <msanalla@nvidia.com>
> Signed-off-by: Edward Srouji <edwards@nvidia.com>
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> drivers/fwctl/mlx5/main.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/fwctl/mlx5/main.c b/drivers/fwctl/mlx5/main.c
> index e86ab703c767a..8ed17aaf48f1f 100644
> --- a/drivers/fwctl/mlx5/main.c
> +++ b/drivers/fwctl/mlx5/main.c
> @@ -7,6 +7,7 @@
> #include <linux/mlx5/device.h>
> #include <linux/mlx5/driver.h>
> #include <uapi/fwctl/mlx5.h>
> +#include <linux/security.h>
>
> #define mlx5ctl_err(mcdev, format, ...) \
> dev_err(&mcdev->fwctl.dev, format, ##__VA_ARGS__)
> @@ -324,6 +325,15 @@ static void *mlx5ctl_fw_rpc(struct fwctl_uctx *uctx, enum fwctl_rpc_scope scope,
> if (!mlx5ctl_validate_rpc(rpc_in, scope))
> return ERR_PTR(-EBADMSG);
>
> + /* Enforce the user context for the command */
> + MLX5_SET(mbox_in_hdr, rpc_in, uid, mfd->uctx_uid);
> +
> + ret = security_fw_validate_cmd(rpc_in, in_len, &mcdev->fwctl.dev,
> + FW_CMD_CLASS_FWCTL,
> + FWCTL_DEVICE_TYPE_MLX5);
> + if (ret)
> + return ERR_PTR(ret);
> +
> /*
> * mlx5_cmd_do() copies the input message to its own buffer before
> * executing it, so we can reuse the allocation for the output.
> @@ -336,8 +346,6 @@ static void *mlx5ctl_fw_rpc(struct fwctl_uctx *uctx, enum fwctl_rpc_scope scope,
> return ERR_PTR(-ENOMEM);
> }
>
> - /* Enforce the user context for the command */
> - MLX5_SET(mbox_in_hdr, rpc_in, uid, mfd->uctx_uid);
> ret = mlx5_cmd_do(mcdev->mdev, rpc_in, in_len, rpc_out, *out_len);
>
> mlx5ctl_dbg(mcdev,
>
^ permalink raw reply
* Re: [PATCH 2/3] RDMA/mlx5: Invoke fw_validate_cmd LSM hook for DEVX commands
From: Dave Jiang @ 2026-03-09 16:59 UTC (permalink / raw)
To: Leon Romanovsky, Paul Moore, James Morris, Serge E. Hallyn,
Jason Gunthorpe, Saeed Mahameed, Itay Avraham, Jonathan Cameron
Cc: linux-security-module, linux-kernel, linux-rdma, Chiara Meiohas,
Maher Sanalla, Edward Srouji
In-Reply-To: <20260309-fw-lsm-hook-v1-2-4a6422e63725@nvidia.com>
On 3/9/26 4:15 AM, Leon Romanovsky wrote:
> From: Chiara Meiohas <cmeiohas@nvidia.com>
>
> DEVX is an RDMA uverbs extension that allows userspace to submit
> firmware command buffers. The driver inspects the command and then
> passes the buffer through for firmware execution.
>
> Call security_fw_validate_cmd() before dispatching firmware commands
> through DEVX.
>
> This allows security modules to implement custom policies and
> enforce per-command security policy on user-triggered firmware
> commands. For example, a BPF LSM program could restrict specific
> firmware operations to privileged users.
>
> Signed-off-by: Chiara Meiohas <cmeiohas@nvidia.com>
> Reviewed-by: Maher Sanalla <msanalla@nvidia.com>
> Signed-off-by: Edward Srouji <edwards@nvidia.com>
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> drivers/infiniband/hw/mlx5/devx.c | 52 ++++++++++++++++++++++++++++++---------
> 1 file changed, 40 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/infiniband/hw/mlx5/devx.c b/drivers/infiniband/hw/mlx5/devx.c
> index 0066b2738ac89..48a2c4b4ad4eb 100644
> --- a/drivers/infiniband/hw/mlx5/devx.c
> +++ b/drivers/infiniband/hw/mlx5/devx.c
> @@ -18,6 +18,7 @@
> #include "devx.h"
> #include "qp.h"
> #include <linux/xarray.h>
> +#include <linux/security.h>
>
> #define UVERBS_MODULE_NAME mlx5_ib
> #include <rdma/uverbs_named_ioctl.h>
> @@ -1111,6 +1112,8 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OTHER)(
> struct mlx5_ib_dev *dev;
> void *cmd_in = uverbs_attr_get_alloced_ptr(
> attrs, MLX5_IB_ATTR_DEVX_OTHER_CMD_IN);
> + int cmd_in_len = uverbs_attr_get_len(attrs,
> + MLX5_IB_ATTR_DEVX_OTHER_CMD_IN);
> int cmd_out_len = uverbs_attr_get_len(attrs,
> MLX5_IB_ATTR_DEVX_OTHER_CMD_OUT);
> void *cmd_out;
> @@ -1135,9 +1138,12 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OTHER)(
> return PTR_ERR(cmd_out);
>
> MLX5_SET(general_obj_in_cmd_hdr, cmd_in, uid, uid);
> - err = mlx5_cmd_do(dev->mdev, cmd_in,
> - uverbs_attr_get_len(attrs, MLX5_IB_ATTR_DEVX_OTHER_CMD_IN),
> - cmd_out, cmd_out_len);
> + err = security_fw_validate_cmd(cmd_in, cmd_in_len, &dev->ib_dev.dev,
> + FW_CMD_CLASS_UVERBS, RDMA_DRIVER_MLX5);
> + if (err)
> + return err;
> +
> + err = mlx5_cmd_do(dev->mdev, cmd_in, cmd_in_len, cmd_out, cmd_out_len);
> if (err && err != -EREMOTEIO)
> return err;
>
> @@ -1570,6 +1576,12 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_CREATE)(
> devx_set_umem_valid(cmd_in);
> }
>
> + err = security_fw_validate_cmd(cmd_in, cmd_in_len,
> + &dev->ib_dev.dev,
> + FW_CMD_CLASS_UVERBS, RDMA_DRIVER_MLX5);
> + if (err)
> + goto obj_free;
> +
> if (opcode == MLX5_CMD_OP_CREATE_DCT) {
> obj->flags |= DEVX_OBJ_FLAGS_DCT;
> err = mlx5_core_create_dct(dev, &obj->core_dct, cmd_in,
> @@ -1582,8 +1594,8 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_CREATE)(
> cmd_in, cmd_in_len, cmd_out,
> cmd_out_len);
> } else {
> - err = mlx5_cmd_do(dev->mdev, cmd_in, cmd_in_len,
> - cmd_out, cmd_out_len);
> + err = mlx5_cmd_do(dev->mdev, cmd_in, cmd_in_len, cmd_out,
> + cmd_out_len);
> }
>
> if (err == -EREMOTEIO)
> @@ -1646,6 +1658,8 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_MODIFY)(
> struct uverbs_attr_bundle *attrs)
> {
> void *cmd_in = uverbs_attr_get_alloced_ptr(attrs, MLX5_IB_ATTR_DEVX_OBJ_MODIFY_CMD_IN);
> + int cmd_in_len = uverbs_attr_get_len(attrs,
> + MLX5_IB_ATTR_DEVX_OBJ_MODIFY_CMD_IN);
> int cmd_out_len = uverbs_attr_get_len(attrs,
> MLX5_IB_ATTR_DEVX_OBJ_MODIFY_CMD_OUT);
> struct ib_uobject *uobj = uverbs_attr_get_uobject(attrs,
> @@ -1676,9 +1690,12 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_MODIFY)(
>
> MLX5_SET(general_obj_in_cmd_hdr, cmd_in, uid, uid);
> devx_set_umem_valid(cmd_in);
> + err = security_fw_validate_cmd(cmd_in, cmd_in_len, &mdev->ib_dev.dev,
> + FW_CMD_CLASS_UVERBS, RDMA_DRIVER_MLX5);
> + if (err)
> + return err;
>
> - err = mlx5_cmd_do(mdev->mdev, cmd_in,
> - uverbs_attr_get_len(attrs, MLX5_IB_ATTR_DEVX_OBJ_MODIFY_CMD_IN),
> + err = mlx5_cmd_do(mdev->mdev, cmd_in, cmd_in_len,
> cmd_out, cmd_out_len);
> if (err && err != -EREMOTEIO)
> return err;
> @@ -1693,6 +1710,8 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_QUERY)(
> struct uverbs_attr_bundle *attrs)
> {
> void *cmd_in = uverbs_attr_get_alloced_ptr(attrs, MLX5_IB_ATTR_DEVX_OBJ_QUERY_CMD_IN);
> + int cmd_in_len = uverbs_attr_get_len(attrs,
> + MLX5_IB_ATTR_DEVX_OBJ_QUERY_CMD_IN);
> int cmd_out_len = uverbs_attr_get_len(attrs,
> MLX5_IB_ATTR_DEVX_OBJ_QUERY_CMD_OUT);
> struct ib_uobject *uobj = uverbs_attr_get_uobject(attrs,
> @@ -1722,8 +1741,12 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_QUERY)(
> return PTR_ERR(cmd_out);
>
> MLX5_SET(general_obj_in_cmd_hdr, cmd_in, uid, uid);
> - err = mlx5_cmd_do(mdev->mdev, cmd_in,
> - uverbs_attr_get_len(attrs, MLX5_IB_ATTR_DEVX_OBJ_QUERY_CMD_IN),
> + err = security_fw_validate_cmd(cmd_in, cmd_in_len, &mdev->ib_dev.dev,
> + FW_CMD_CLASS_UVERBS, RDMA_DRIVER_MLX5);
> + if (err)
> + return err;
> +
> + err = mlx5_cmd_do(mdev->mdev, cmd_in, cmd_in_len,
> cmd_out, cmd_out_len);
> if (err && err != -EREMOTEIO)
> return err;
> @@ -1832,6 +1855,8 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_ASYNC_QUERY)(
> {
> void *cmd_in = uverbs_attr_get_alloced_ptr(attrs,
> MLX5_IB_ATTR_DEVX_OBJ_QUERY_ASYNC_CMD_IN);
> + int cmd_in_len = uverbs_attr_get_len(attrs,
> + MLX5_IB_ATTR_DEVX_OBJ_QUERY_ASYNC_CMD_IN);
> struct ib_uobject *uobj = uverbs_attr_get_uobject(
> attrs,
> MLX5_IB_ATTR_DEVX_OBJ_QUERY_ASYNC_HANDLE);
> @@ -1894,9 +1919,12 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_ASYNC_QUERY)(
> async_data->ev_file = ev_file;
>
> MLX5_SET(general_obj_in_cmd_hdr, cmd_in, uid, uid);
> - err = mlx5_cmd_exec_cb(&ev_file->async_ctx, cmd_in,
> - uverbs_attr_get_len(attrs,
> - MLX5_IB_ATTR_DEVX_OBJ_QUERY_ASYNC_CMD_IN),
> + err = security_fw_validate_cmd(cmd_in, cmd_in_len, &mdev->ib_dev.dev,
> + FW_CMD_CLASS_UVERBS, RDMA_DRIVER_MLX5);
> + if (err)
> + goto free_async;
> +
> + err = mlx5_cmd_exec_cb(&ev_file->async_ctx, cmd_in, cmd_in_len,
> async_data->hdr.out_data,
> async_data->cmd_out_len,
> devx_query_callback, &async_data->cb_work);
>
^ permalink raw reply
* Re: [PATCH 1/3] lsm: add hook for firmware command validation
From: Dave Jiang @ 2026-03-09 17:00 UTC (permalink / raw)
To: Leon Romanovsky, Paul Moore, James Morris, Serge E. Hallyn,
Jason Gunthorpe, Saeed Mahameed, Itay Avraham, Jonathan Cameron
Cc: linux-security-module, linux-kernel, linux-rdma, Chiara Meiohas,
Maher Sanalla, Edward Srouji
In-Reply-To: <20260309-fw-lsm-hook-v1-1-4a6422e63725@nvidia.com>
On 3/9/26 4:15 AM, Leon Romanovsky wrote:
> From: Chiara Meiohas <cmeiohas@nvidia.com>
>
> Drivers typically communicate with device firmware either via
> register-based commands (writing parameters into device registers)
> or by passing a command buffer using shared-memory mechanisms.
>
> This hook targets the command buffer mechanism, which is commonly
> used on modern, complex devices.
>
> Add the LSM hook fw_validate_cmd. This hook allows inspecting
> firmware command buffers before they are sent to the device.
> The hook receives the command buffer, device, command class, and a
> class-specific id:
> - class_id (enum fw_cmd_class) allows security modules to
> differentiate between classes of firmware commands.
> In this series, class_id distinguishes between commands from the
> RDMA uverbs interface and from fwctl.
> - id is a class-specific device identifier. For uverbs, id is the
> RDMA driver identifier (enum rdma_driver_id). For fwctl, id is the
> device type (enum fwctl_device_type).
>
> Signed-off-by: Chiara Meiohas <cmeiohas@nvidia.com>
> Reviewed-by: Maher Sanalla <msanalla@nvidia.com>
> Signed-off-by: Edward Srouji <edwards@nvidia.com>
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> include/linux/lsm_hook_defs.h | 2 ++
> include/linux/security.h | 25 +++++++++++++++++++++++++
> security/security.c | 26 ++++++++++++++++++++++++++
> 3 files changed, 53 insertions(+)
>
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 8c42b4bde09c0..93da090384ea1 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -445,6 +445,8 @@ LSM_HOOK(int, 0, bpf_token_capable, const struct bpf_token *token, int cap)
> #endif /* CONFIG_BPF_SYSCALL */
>
> LSM_HOOK(int, 0, locked_down, enum lockdown_reason what)
> +LSM_HOOK(int, 0, fw_validate_cmd, const void *in, size_t in_len,
> + const struct device *dev, enum fw_cmd_class class_id, u32 id)
>
> #ifdef CONFIG_PERF_EVENTS
> LSM_HOOK(int, 0, perf_event_open, int type)
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 83a646d72f6f8..64786d013207a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -67,6 +67,7 @@ enum fs_value_type;
> struct watch;
> struct watch_notification;
> struct lsm_ctx;
> +struct device;
>
> /* Default (no) options for the capable function */
> #define CAP_OPT_NONE 0x0
> @@ -157,6 +158,21 @@ enum lockdown_reason {
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
>
> +/*
> + * enum fw_cmd_class - Class of the firmware command passed to
> + * security_fw_validate_cmd.
> + * This allows security modules to distinguish between different command
> + * classes.
> + *
> + * @FW_CMD_CLASS_UVERBS: Command originated from the RDMA uverbs interface
> + * @FW_CMD_CLASS_FWCTL: Command originated from the fwctl interface
> + */
> +enum fw_cmd_class {
> + FW_CMD_CLASS_UVERBS,
> + FW_CMD_CLASS_FWCTL,
> + FW_CMD_CLASS_MAX,
> +};
> +
> /*
> * Data exported by the security modules
> */
> @@ -575,6 +591,9 @@ int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
> int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
> int security_inode_getsecctx(struct inode *inode, struct lsm_context *cp);
> int security_locked_down(enum lockdown_reason what);
> +int security_fw_validate_cmd(const void *in, size_t in_len,
> + const struct device *dev,
> + enum fw_cmd_class class_id, u32 id);
> int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, u32 *uctx_len,
> void *val, size_t val_len, u64 id, u64 flags);
> int security_bdev_alloc(struct block_device *bdev);
> @@ -1589,6 +1608,12 @@ static inline int security_locked_down(enum lockdown_reason what)
> {
> return 0;
> }
> +static inline int security_fw_validate_cmd(const void *in, size_t in_len,
> + const struct device *dev,
> + enum fw_cmd_class class_id, u32 id)
> +{
> + return 0;
> +}
> static inline int lsm_fill_user_ctx(struct lsm_ctx __user *uctx,
> u32 *uctx_len, void *val, size_t val_len,
> u64 id, u64 flags)
> diff --git a/security/security.c b/security/security.c
> index 67af9228c4e94..d05941fe89a48 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -5373,6 +5373,32 @@ int security_locked_down(enum lockdown_reason what)
> }
> EXPORT_SYMBOL(security_locked_down);
>
> +/**
> + * security_fw_validate_cmd() - Validate a firmware command
> + * @in: pointer to the firmware command input buffer
> + * @in_len: length of the firmware command input buffer
> + * @dev: device associated with the command
> + * @class_id: class of the firmware command
> + * @id: device identifier, specific to the command @class_id
> + *
> + * Check permissions before sending a firmware command generated by
> + * userspace to the device.
> + *
> + * Return: Returns 0 if permission is granted.
> + */
> +int security_fw_validate_cmd(const void *in, size_t in_len,
> + const struct device *dev,
> + enum fw_cmd_class class_id,
> + u32 id)
> +{
> + if (class_id >= FW_CMD_CLASS_MAX)
> + return -EINVAL;
> +
> + return call_int_hook(fw_validate_cmd, in, in_len,
> + dev, class_id, id);
> +}
> +EXPORT_SYMBOL_GPL(security_fw_validate_cmd);
> +
> /**
> * security_bdev_alloc() - Allocate a block device LSM blob
> * @bdev: block device
>
^ permalink raw reply
* Re: [PATCH v1 3/4] landlock: Improve kernel-doc "Return:" section consistency
From: Mickaël Salaün @ 2026-03-09 17:34 UTC (permalink / raw)
To: Günther Noack; +Cc: Günther Noack, linux-security-module
In-Reply-To: <20260306.b502a795f389@gnoack.org>
On Fri, Mar 06, 2026 at 08:30:29AM +0100, Günther Noack wrote:
> Just a few comment phrasing nits below
>
> On Wed, Mar 04, 2026 at 08:31:26PM +0100, Mickaël Salaün wrote:
> > The canonical kernel-doc form is "Return:" (singular, without trailing
> > "s"). Normalize all existing "Returns:" occurrences across the Landlock
> > source tree to the canonical form.
> >
> > Also fix capitalization for consistency. Balance descriptions to
> > describe all possible returned values.
> >
> > Consolidate bullet-point return descriptions into inline text for
> > functions with simple two-value or three-value returns for consistency.
> >
> > Cc: Günther Noack <gnoack@google.com>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > ---
> > security/landlock/cred.h | 2 +-
> > security/landlock/domain.c | 4 ++--
> > security/landlock/fs.c | 26 +++++++++++---------------
> > security/landlock/id.c | 2 +-
> > security/landlock/ruleset.c | 2 +-
> > security/landlock/ruleset.h | 2 +-
> > security/landlock/task.c | 4 ++--
> > security/landlock/tsync.c | 17 ++++++-----------
> > 8 files changed, 25 insertions(+), 34 deletions(-)
> >
> > diff --git a/security/landlock/cred.h b/security/landlock/cred.h
> > index c10a06727eb1..f287c56b5fd4 100644
> > --- a/security/landlock/cred.h
> > +++ b/security/landlock/cred.h
> > @@ -115,7 +115,7 @@ static inline bool landlocked(const struct task_struct *const task)
> > * @handle_layer: returned youngest layer handling a subset of @masks. Not set
> > * if the function returns NULL.
> > *
> > - * Returns: landlock_cred(@cred) if any access rights specified in @masks is
> > + * Return: landlock_cred(@cred) if any access rights specified in @masks is
> > * handled, or NULL otherwise.
> > */
> > static inline const struct landlock_cred_security *
> > diff --git a/security/landlock/domain.c b/security/landlock/domain.c
> > index 343a1aabaac6..8b9939005aa8 100644
> > --- a/security/landlock/domain.c
> > +++ b/security/landlock/domain.c
> > @@ -34,7 +34,7 @@
> > * @exe_size: Returned size of @exe_str (including the trailing null
> > * character), if any.
> > *
> > - * Returns: A pointer to an allocated buffer where @exe_str point to, %NULL if
> > + * Return: A pointer to an allocated buffer where @exe_str point to, %NULL if
> > * there is no executable path, or an error otherwise.
> > */
> > static const void *get_current_exe(const char **const exe_str,
> > @@ -73,7 +73,7 @@ static const void *get_current_exe(const char **const exe_str,
> > }
> >
> > /*
> > - * Returns: A newly allocated object describing a domain, or an error
> > + * Return: A newly allocated object describing a domain, or an error
> > * otherwise.
> > */
> > static struct landlock_details *get_current_details(void)
> > diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> > index cfe69075bf4e..a03ec664c78e 100644
> > --- a/security/landlock/fs.c
> > +++ b/security/landlock/fs.c
> > @@ -119,8 +119,8 @@ static const struct landlock_object_underops landlock_fs_underops = {
> > * Any new IOCTL commands that are implemented in fs/ioctl.c's do_vfs_ioctl()
> > * should be considered for inclusion here.
> > *
> > - * Returns: true if the IOCTL @cmd can not be restricted with Landlock for
> > - * device files.
> > + * Return: True if the IOCTL @cmd can not be restricted with Landlock for
> > + * device files, false otherwise.
> > */
> > static __attribute_const__ bool is_masked_device_ioctl(const unsigned int cmd)
> > {
> > @@ -428,10 +428,10 @@ static bool may_refer(const struct layer_access_masks *const src_parent,
> > * Check that a destination file hierarchy has more restrictions than a source
> > * file hierarchy. This is only used for link and rename actions.
> > *
> > - * Returns: true if child1 may be moved from parent1 to parent2 without
> > - * increasing its access rights. If child2 is set, an additional condition is
> > + * Return: True if child1 may be moved from parent1 to parent2 without
> > + * increasing its access rights (if child2 is set, an additional condition is
> > * that child2 may be used from parent2 to parent1 without increasing its access
> > - * rights.
> > + * rights), false otherwise.
> > */
> > static bool no_more_access(const struct layer_access_masks *const parent1,
> > const struct layer_access_masks *const child1,
> > @@ -734,9 +734,7 @@ static void test_is_eacces_with_write(struct kunit *const test)
> > * checks that the collected accesses and the remaining ones are enough to
> > * allow the request.
> > *
> > - * Returns:
> > - * - true if the access request is granted;
> > - * - false otherwise.
> > + * Return: True if the access request is granted, false otherwise.
> > */
> > static bool
> > is_access_to_paths_allowed(const struct landlock_ruleset *const domain,
> > @@ -1022,9 +1020,8 @@ static access_mask_t maybe_remove(const struct dentry *const dentry)
> > * only handles walking on the same mount point and only checks one set of
> > * accesses.
> > *
> > - * Returns:
> > - * - true if all the domain access rights are allowed for @dir;
> > - * - false if the walk reached @mnt_root.
> > + * Return: True if all the domain access rights are allowed for @dir, false if
> > + * the walk reached @mnt_root.
> > */
> > static bool collect_domain_accesses(const struct landlock_ruleset *const domain,
> > const struct dentry *const mnt_root,
> > @@ -1120,10 +1117,9 @@ static bool collect_domain_accesses(const struct landlock_ruleset *const domain,
> > * ephemeral matrices take some space on the stack, which limits the number of
> > * layers to a deemed reasonable number: 16.
> > *
> > - * Returns:
> > - * - 0 if access is allowed;
> > - * - -EXDEV if @old_dentry would inherit new access rights from @new_dir;
> > - * - -EACCES if file removal or creation is denied.
> > + * Return: 0 if access is allowed, -EXDEV if @old_dentry would inherit new
> > + * access rights from @new_dir, or -EACCES if file removal or creation is
> > + * denied.
> > */
> > static int current_check_refer_path(struct dentry *const old_dentry,
> > const struct path *const new_dir,
> > diff --git a/security/landlock/id.c b/security/landlock/id.c
> > index 838c3ed7bb82..6c8769777fdc 100644
> > --- a/security/landlock/id.c
> > +++ b/security/landlock/id.c
> > @@ -258,7 +258,7 @@ static void test_range2_rand16(struct kunit *const test)
> > *
> > * @number_of_ids: Number of IDs to hold. Must be greater than one.
> > *
> > - * Returns: The first ID in the range.
> > + * Return: The first ID in the range.
> > */
> > u64 landlock_get_id_range(size_t number_of_ids)
> > {
> > diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> > index de8386af2f30..52e48ffcc3aa 100644
> > --- a/security/landlock/ruleset.c
> > +++ b/security/landlock/ruleset.c
> > @@ -675,7 +675,7 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset,
> > * @masks: Layer access masks to populate.
> > * @key_type: The key type to switch between access masks of different types.
> > *
> > - * Returns: An access mask where each access right bit is set which is handled
> > + * Return: An access mask where each access right bit is set which is handled
> > * in any of the active layers in @domain.
> > */
> > access_mask_t
> > diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> > index 87d52031fb5a..5e63f78f7e1a 100644
> > --- a/security/landlock/ruleset.h
> > +++ b/security/landlock/ruleset.h
> > @@ -232,7 +232,7 @@ static inline void landlock_get_ruleset(struct landlock_ruleset *const ruleset)
> > *
> > * @domain: Landlock ruleset (used as a domain)
> > *
> > - * Returns: an access_masks result of the OR of all the domain's access masks.
> > + * Return: An access_masks result of the OR of all the domain's access masks.
> > */
> > static inline struct access_masks
> > landlock_union_access_masks(const struct landlock_ruleset *const domain)
> > diff --git a/security/landlock/task.c b/security/landlock/task.c
> > index bf7c3db7ce46..f2dbdebf2770 100644
> > --- a/security/landlock/task.c
> > +++ b/security/landlock/task.c
> > @@ -174,8 +174,8 @@ static int hook_ptrace_traceme(struct task_struct *const parent)
> > * @server: IPC receiver domain.
> > * @scope: The scope restriction criteria.
> > *
> > - * Returns: True if @server is in a different domain from @client, and @client
> > - * is scoped to access @server (i.e. access should be denied).
> > + * Return: True if @server is in a different domain from @client and @client
> > + * is scoped to access @server (i.e. access should be denied), false otherwise.
> > */
> > static bool domain_is_scoped(const struct landlock_ruleset *const client,
> > const struct landlock_ruleset *const server,
> > diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> > index b06a0fa4cedb..359aecbb1e4b 100644
> > --- a/security/landlock/tsync.c
> > +++ b/security/landlock/tsync.c
> > @@ -183,10 +183,8 @@ struct tsync_works {
> > * capacity. This can legitimately happen if new threads get started after we
> > * grew the capacity.
> > *
> > - * Returns:
> > - * A pointer to the preallocated context struct, with task filled in.
> > - *
> > - * NULL, if we ran out of preallocated context structs.
> > + * Return: A pointer to the preallocated context struct with task filled in, or
> > + * NULL if preallocated context structs ran out.
> > */
> > static struct tsync_work *tsync_works_provide(struct tsync_works *s,
> > struct task_struct *task)
> > @@ -243,11 +241,8 @@ static void tsync_works_trim(struct tsync_works *s)
> > * On a successful return, the subsequent n calls to tsync_works_provide() are
> > * guaranteed to succeed. (size + n <= capacity)
> > *
> > - * Returns:
> > - * -ENOMEM if the (re)allocation fails
> > -
> > - * 0 if the allocation succeeds, partially succeeds, or no reallocation
> > - * was needed
> > + * Return: 0 on success or partial success, -ENOMEM if the (re)allocation
> > + * fails.
>
> tsync_works_grow_by:
>
> I don't know what I meant when I wrote "partially succeeds" here in
> the original patch. Would suggest this phrasing:
>
> Return: 0 if sufficient space for n more elements could be provided,
> -ENOMEM on allocation errors, -EOVERFLOW in case of integer
> overflow.
>
> With this function, the success criterium is that it can establish
> that invariant. We also don't return a success if we only could
> allocate space for fewer elements.
Good, I'll fold this in the commit.
>
> > */
> > static int tsync_works_grow_by(struct tsync_works *s, size_t n, gfp_t flags)
> > {
> > @@ -363,8 +358,8 @@ static size_t count_additional_threads(const struct tsync_works *works)
> > * For each added task_work, atomically increments shared_ctx->num_preparing and
> > * shared_ctx->num_unfinished.
> > *
> > - * Returns:
> > - * true, if at least one eligible sibling thread was found
> > + * Return: True if at least one eligible sibling thread was found, false
> > + * otherwise.
> > */
> > static bool schedule_task_work(struct tsync_works *works,
> > struct tsync_shared_context *shared_ctx)
> > --
> > 2.53.0
> >
>
> Reviewed-by: Günther Noack <gnoack3000@gmail.com>
>
^ permalink raw reply
* Re: [PATCH v1 4/4] landlock: Fix formatting in tsync.c
From: Mickaël Salaün @ 2026-03-09 17:35 UTC (permalink / raw)
To: Günther Noack; +Cc: Günther Noack, linux-security-module
In-Reply-To: <20260306.73159b8566b4@gnoack.org>
On Fri, Mar 06, 2026 at 08:13:59AM +0100, Günther Noack wrote:
> On Wed, Mar 04, 2026 at 08:31:27PM +0100, Mickaël Salaün wrote:
> > Fix comment formatting in tsync.c to fit in 80 columns.
> >
> > Cc: Günther Noack <gnoack@google.com>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > ---
> >
> > My previous squashed fix was wrong.
> > ---
> > security/landlock/tsync.c | 121 +++++++++++++++++++++-----------------
> > 1 file changed, 66 insertions(+), 55 deletions(-)
> >
> > diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> > index 359aecbb1e4b..50445ae167dd 100644
> > --- a/security/landlock/tsync.c
> > +++ b/security/landlock/tsync.c
> > @@ -85,12 +85,14 @@ static void restrict_one_thread(struct tsync_shared_context *ctx)
> > /*
> > * Switch out old_cred with new_cred, if possible.
> > *
> > - * In the common case, where all threads initially point to the same
> > - * struct cred, this optimization avoids creating separate redundant
> > - * credentials objects for each, which would all have the same contents.
> > + * In the common case, where all threads initially point to the
> > + * same struct cred, this optimization avoids creating separate
> > + * redundant credentials objects for each, which would all have
> > + * the same contents.
> > *
> > - * Note: We are intentionally dropping the const qualifier here, because
> > - * it is required by commit_creds() and abort_creds().
> > + * Note: We are intentionally dropping the const qualifier
> > + * here, because it is required by commit_creds() and
> > + * abort_creds().
> > */
> > cred = (struct cred *)get_cred(ctx->new_cred);
> > } else {
> > @@ -101,8 +103,8 @@ static void restrict_one_thread(struct tsync_shared_context *ctx)
> > atomic_set(&ctx->preparation_error, -ENOMEM);
> >
> > /*
> > - * Even on error, we need to adhere to the protocol and coordinate
> > - * with concurrently running invocations.
> > + * Even on error, we need to adhere to the protocol and
> > + * coordinate with concurrently running invocations.
> > */
> > if (atomic_dec_return(&ctx->num_preparing) == 0)
> > complete_all(&ctx->all_prepared);
> > @@ -135,9 +137,9 @@ static void restrict_one_thread(struct tsync_shared_context *ctx)
> > }
> >
> > /*
> > - * Make sure that all sibling tasks fulfill the no_new_privs prerequisite.
> > - * (This is in line with Seccomp's SECCOMP_FILTER_FLAG_TSYNC logic in
> > - * kernel/seccomp.c)
> > + * Make sure that all sibling tasks fulfill the no_new_privs
> > + * prerequisite. (This is in line with Seccomp's
> > + * SECCOMP_FILTER_FLAG_TSYNC logic in kernel/seccomp.c)
> > */
> > if (ctx->set_no_new_privs)
> > task_set_no_new_privs(current);
> > @@ -221,16 +223,17 @@ static void tsync_works_trim(struct tsync_works *s)
> > ctx = s->works[s->size - 1];
> >
> > /*
> > - * For consistency, remove the task from ctx so that it does not look like
> > - * we handed it a task_work.
> > + * For consistency, remove the task from ctx so that it does not look
> > + * like we handed it a task_work.
> > */
> > put_task_struct(ctx->task);
> > *ctx = (typeof(*ctx)){};
> >
> > /*
> > - * Cancel the tsync_works_provide() change to recycle the reserved memory
> > - * for the next thread, if any. This also ensures that cancel_tsync_works()
> > - * and tsync_works_release() do not see any NULL task pointers.
> > + * Cancel the tsync_works_provide() change to recycle the reserved
> > + * memory for the next thread, if any. This also ensures that
> > + * cancel_tsync_works() and tsync_works_release() do not see any NULL
> > + * task pointers.
> > */
> > s->size--;
> > }
> > @@ -388,17 +391,17 @@ static bool schedule_task_work(struct tsync_works *works,
> > continue;
> >
> > /*
> > - * We found a sibling thread that is not doing its task_work yet, and
> > - * which might spawn new threads before our task work runs, so we need
> > - * at least one more round in the outer loop.
> > + * We found a sibling thread that is not doing its task_work
> > + * yet, and which might spawn new threads before our task work
> > + * runs, so we need at least one more round in the outer loop.
> > */
> > found_more_threads = true;
> >
> > ctx = tsync_works_provide(works, thread);
> > if (!ctx) {
> > /*
> > - * We ran out of preallocated contexts -- we need to try again with
> > - * this thread at a later time!
> > + * We ran out of preallocated contexts -- we need to
> > + * try again with this thread at a later time!
> > * found_more_threads is already true at this point.
> > */
> > break;
> > @@ -413,10 +416,10 @@ static bool schedule_task_work(struct tsync_works *works,
> > err = task_work_add(thread, &ctx->work, TWA_SIGNAL);
> > if (unlikely(err)) {
> > /*
> > - * task_work_add() only fails if the task is about to exit. We
> > - * checked that earlier, but it can happen as a race. Resume
> > - * without setting an error, as the task is probably gone in the
> > - * next loop iteration.
> > + * task_work_add() only fails if the task is about to
> > + * exit. We checked that earlier, but it can happen as
> > + * a race. Resume without setting an error, as the
> > + * task is probably gone in the next loop iteration.
> > */
> > tsync_works_trim(works);
> >
> > @@ -497,24 +500,25 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> > * After this barrier is reached, it's safe to read
> > * shared_ctx.preparation_error.
> > *
> > - * 4) reads shared_ctx.preparation_error and then either does commit_creds()
> > - * or abort_creds().
> > + * 4) reads shared_ctx.preparation_error and then either does
> > + * commit_creds() or abort_creds().
> > *
> > * 5) signals that it's done altogether (barrier synchronization
> > * "all_finished")
> > *
> > - * Unlike seccomp, which modifies sibling tasks directly, we do not need to
> > - * acquire the cred_guard_mutex and sighand->siglock:
> > + * Unlike seccomp, which modifies sibling tasks directly, we do not
> > + * need to acquire the cred_guard_mutex and sighand->siglock:
> > *
> > - * - As in our case, all threads are themselves exchanging their own struct
> > - * cred through the credentials API, no locks are needed for that.
> > + * - As in our case, all threads are themselves exchanging their own
> > + * struct cred through the credentials API, no locks are needed for
> > + * that.
> > * - Our for_each_thread() loops are protected by RCU.
> > - * - We do not acquire a lock to keep the list of sibling threads stable
> > - * between our for_each_thread loops. If the list of available sibling
> > - * threads changes between these for_each_thread loops, we make up for
> > - * that by continuing to look for threads until they are all discovered
> > - * and have entered their task_work, where they are unable to spawn new
> > - * threads.
> > + * - We do not acquire a lock to keep the list of sibling threads
> > + * stable between our for_each_thread loops. If the list of
> > + * available sibling threads changes between these for_each_thread
> > + * loops, we make up for that by continuing to look for threads until
> > + * they are all discovered and have entered their task_work, where
> > + * they are unable to spawn new threads.
> > */
> > do {
> > /* In RCU read-lock, count the threads we need. */
> > @@ -531,43 +535,50 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> > }
> >
> > /*
> > - * The "all_prepared" barrier is used locally to the loop body, this use
> > - * of for_each_thread(). We can reset it on each loop iteration because
> > - * all previous loop iterations are done with it already.
> > + * The "all_prepared" barrier is used locally to the loop body,
> > + * this use of for_each_thread(). We can reset it on each loop
> > + * iteration because all previous loop iterations are done with
> > + * it already.
> > *
> > - * num_preparing is initialized to 1 so that the counter can not go to 0
> > - * and mark the completion as done before all task works are registered.
> > - * We decrement it at the end of the loop body.
> > + * num_preparing is initialized to 1 so that the counter can
> > + * not go to 0 and mark the completion as done before all task
> > + * works are registered. We decrement it at the end of the
> > + * loop body.
> > */
> > atomic_set(&shared_ctx.num_preparing, 1);
> > reinit_completion(&shared_ctx.all_prepared);
> >
> > /*
> > - * In RCU read-lock, schedule task work on newly discovered sibling
> > - * tasks.
> > + * In RCU read-lock, schedule task work on newly discovered
> > + * sibling tasks.
> > */
> > found_more_threads = schedule_task_work(&works, &shared_ctx);
> >
> > /*
> > - * Decrement num_preparing for current, to undo that we initialized it
> > - * to 1 a few lines above.
> > + * Decrement num_preparing for current, to undo that we
> > + * initialized it to 1 a few lines above.
> > */
> > if (atomic_dec_return(&shared_ctx.num_preparing) > 0) {
> > if (wait_for_completion_interruptible(
> > &shared_ctx.all_prepared)) {
> > - /* In case of interruption, we need to retry the system call. */
> > + /*
> > + * In case of interruption, we need to retry
> > + * the system call.
> > + */
> > atomic_set(&shared_ctx.preparation_error,
> > -ERESTARTNOINTR);
> >
> > /*
> > - * Cancel task works for tasks that did not start running yet,
> > - * and decrement all_prepared and num_unfinished accordingly.
> > + * Cancel task works for tasks that did not
> > + * start running yet, and decrement
> > + * all_prepared and num_unfinished accordingly.
> > */
> > cancel_tsync_works(&works, &shared_ctx);
> >
> > /*
> > - * The remaining task works have started running, so waiting for
> > - * their completion will finish.
> > + * The remaining task works have started
> > + * running, so waiting for their completion
> > + * will finish.
> > */
> > wait_for_completion(&shared_ctx.all_prepared);
> > }
> > @@ -576,14 +587,14 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> > !atomic_read(&shared_ctx.preparation_error));
> >
> > /*
> > - * We now have all sibling threads blocking and in "prepared" state in the
> > - * task work. Ask all threads to commit.
> > + * We now have all sibling threads blocking and in "prepared" state in
> > + * the task work. Ask all threads to commit.
> > */
> > complete_all(&shared_ctx.ready_to_commit);
> >
> > /*
> > - * Decrement num_unfinished for current, to undo that we initialized it to 1
> > - * at the beginning.
> > + * Decrement num_unfinished for current, to undo that we initialized it
> > + * to 1 at the beginning.
> > */
> > if (atomic_dec_return(&shared_ctx.num_unfinished) > 0)
> > wait_for_completion(&shared_ctx.all_finished);
> > --
> > 2.53.0
> >
>
> Reviewed-by: Günther Noack <gnoack3000@gmail.com>
>
> Thanks! (It's irritating that the default clang-format configuration
> does not fix these. Do you use a special tool for this?)
I mostly use vim's colorcolumn but I should update check-linux.sh with
such check.
I guess you're also OK with the first patch?
>
> –Günther
>
^ permalink raw reply
* Re: [PATCH v3 00/12] vfs: change inode->i_ino from unsigned long to u64
From: Mimi Zohar @ 2026-03-09 17:47 UTC (permalink / raw)
To: Jeff Layton
Cc: linux-fsdevel, linux-kernel, linux-trace-kernel, nvdimm, fsverity,
linux-mm, netfs, linux-ext4, linux-f2fs-devel, linux-nfs,
linux-cifs, samba-technical, linux-nilfs, v9fs, linux-afs, autofs,
ceph-devel, codalist, ecryptfs, linux-mtd, jfs-discussion, ntfs3,
ocfs2-devel, devel, linux-unionfs, apparmor,
linux-security-module, linux-integrity, selinux, amd-gfx,
dri-devel, linux-media, linaro-mm-sig, netdev, linux-perf-users,
linux-fscrypt, linux-xfs, linux-hams, linux-x25, audit,
linux-bluetooth, linux-can, linux-sctp, bpf
In-Reply-To: <20260304-iino-u64-v3-0-2257ad83d372@kernel.org>
[ I/O socket time out. Trimming the To list.]
On Wed, 2026-03-04 at 10:32 -0500, Jeff Layton wrote:
> This version squashes all of the format-string changes and the i_ino
> type change into the same patch. This results in a giant 600+ line patch
> at the end of the series, but it does remain bisectable. Because the
> patchset was reorganized (again) some of the R-b's and A-b's have been
> dropped.
>
> The entire pile is in the "iino-u64" branch of my tree, if anyone is
> interested in testing this.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/jlayton/linux.git/
>
> Original cover letter follows:
>
> ----------------------8<-----------------------
>
> Christian said [1] to "just do it" when I proposed this, so here we are!
>
> For historical reasons, the inode->i_ino field is an unsigned long,
> which means that it's 32 bits on 32 bit architectures. This has caused a
> number of filesystems to implement hacks to hash a 64-bit identifier
> into a 32-bit field, and deprives us of a universal identifier field for
> an inode.
>
> This patchset changes the inode->i_ino field from an unsigned long to a
> u64. This shouldn't make any material difference on 64-bit hosts, but
> 32-bit hosts will see struct inode grow by at least 4 bytes. This could
> have effects on slabcache sizes and field alignment.
>
> The bulk of the changes are to format strings and tracepoints, since the
> kernel itself doesn't care that much about the i_ino field. The first
> patch changes some vfs function arguments, so check that one out
> carefully.
>
> With this change, we may be able to shrink some inode structures. For
> instance, struct nfs_inode has a fileid field that holds the 64-bit
> inode number. With this set of changes, that field could be eliminated.
> I'd rather leave that sort of cleanups for later just to keep this
> simple.
>
> Much of this set was generated by LLM, but I attributed it to myself
> since I consider this to be in the "menial tasks" category of LLM usage.
>
> [1]: https://lore.kernel.org/linux-fsdevel/20260219-portrait-winkt-959070cee42f@brauner/
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
Jeff, missing from this patch set is EVM. In hmac_add_misc() EVM copies the
i_ino and calculates either an HMAC or file meta-data hash, which is then
signed.
Mimi
^ permalink raw reply
* Re: [PATCH v3 00/12] vfs: change inode->i_ino from unsigned long to u64
From: Jeff Layton @ 2026-03-09 17:59 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-fsdevel, linux-kernel, linux-trace-kernel, nvdimm, fsverity,
linux-mm, netfs, linux-ext4, linux-f2fs-devel, linux-nfs,
linux-cifs, samba-technical, linux-nilfs, v9fs, linux-afs, autofs,
ceph-devel, codalist, ecryptfs, linux-mtd, jfs-discussion, ntfs3,
ocfs2-devel, devel, linux-unionfs, apparmor,
linux-security-module, linux-integrity, selinux, amd-gfx,
dri-devel, linux-media, linaro-mm-sig, netdev, linux-perf-users,
linux-fscrypt, linux-xfs, linux-hams, linux-x25, audit,
linux-bluetooth, linux-can, linux-sctp, bpf
In-Reply-To: <05b5d55c49b5a1bbc43a5315e3c84872e7e634b3.camel@linux.ibm.com>
On Mon, 2026-03-09 at 13:47 -0400, Mimi Zohar wrote:
> [ I/O socket time out. Trimming the To list.]
>
> On Wed, 2026-03-04 at 10:32 -0500, Jeff Layton wrote:
> > This version squashes all of the format-string changes and the i_ino
> > type change into the same patch. This results in a giant 600+ line patch
> > at the end of the series, but it does remain bisectable. Because the
> > patchset was reorganized (again) some of the R-b's and A-b's have been
> > dropped.
> >
> > The entire pile is in the "iino-u64" branch of my tree, if anyone is
> > interested in testing this.
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/jlayton/linux.git/
> >
> > Original cover letter follows:
> >
> > ----------------------8<-----------------------
> >
> > Christian said [1] to "just do it" when I proposed this, so here we are!
> >
> > For historical reasons, the inode->i_ino field is an unsigned long,
> > which means that it's 32 bits on 32 bit architectures. This has caused a
> > number of filesystems to implement hacks to hash a 64-bit identifier
> > into a 32-bit field, and deprives us of a universal identifier field for
> > an inode.
> >
> > This patchset changes the inode->i_ino field from an unsigned long to a
> > u64. This shouldn't make any material difference on 64-bit hosts, but
> > 32-bit hosts will see struct inode grow by at least 4 bytes. This could
> > have effects on slabcache sizes and field alignment.
> >
> > The bulk of the changes are to format strings and tracepoints, since the
> > kernel itself doesn't care that much about the i_ino field. The first
> > patch changes some vfs function arguments, so check that one out
> > carefully.
> >
> > With this change, we may be able to shrink some inode structures. For
> > instance, struct nfs_inode has a fileid field that holds the 64-bit
> > inode number. With this set of changes, that field could be eliminated.
> > I'd rather leave that sort of cleanups for later just to keep this
> > simple.
> >
> > Much of this set was generated by LLM, but I attributed it to myself
> > since I consider this to be in the "menial tasks" category of LLM usage.
> >
> > [1]: https://lore.kernel.org/linux-fsdevel/20260219-portrait-winkt-959070cee42f@brauner/
> >
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
>
> Jeff, missing from this patch set is EVM. In hmac_add_misc() EVM copies the
> i_ino and calculates either an HMAC or file meta-data hash, which is then
> signed.
>
>
Thanks Mimi, good catch.
It looks like we should just be able to change the ino field to a u64
alongside everything else. Something like this:
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index c0ca4eedb0fe..77b6c2fa345e 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -144,7 +144,7 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
char type, char *digest)
{
struct h_misc {
- unsigned long ino;
+ u64 ino;
__u32 generation;
uid_t uid;
gid_t gid;
That should make no material difference on 64-bit hosts. What's the
effect on 32-bit? Will they just need to remeasure everything or would
the consequences be more dire? Do we have any clue whether anyone is
using EVM in 32-bit environments?
Thanks,
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply related
* Re: LSM namespacing API
From: Stephen Smalley @ 2026-03-09 18:15 UTC (permalink / raw)
To: Casey Schaufler
Cc: Dr. Greg, Paul Moore, Ondrej Mosnacek, linux-security-module,
selinux, John Johansen
In-Reply-To: <ae5e9c1b-d0cd-476b-87d2-70c0bc8ba09c@schaufler-ca.com>
On Fri, Mar 6, 2026 at 4:01 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 3/6/2026 9:48 AM, Dr. Greg wrote:
> > On Tue, Mar 03, 2026 at 11:46:53AM -0500, Paul Moore wrote:
> >
> > Good morning, I hope the week is winding down well for everyone.
> >
> >> On Tue, Mar 3, 2026 at 8:30???AM Stephen Smalley
> >>> I think my only caveat here is that your proposal is quite a bit more
> >>> complex than what I implemented here:
> >>> [1] https://lore.kernel.org/selinux/20251003190959.3288-2-stephen.smalley.work@gmail.com/
> >>> [2] https://lore.kernel.org/selinux/20251003191328.3605-1-stephen.smalley.work@gmail.com/
> >>> and I'm not sure the extra complexity is worth it.
> >>>
> >>> In particular:
> >>> 1. Immediately unsharing the namespace upon lsm_set_self_attr() allows
> >>> the caller to immediately and unambiguously know if the operation is
> >>> supported and allowed ...
> >> Performing the unshare operation immediately looks much less like a
> >> LSM attribute and more like its own syscall. That isn't a problem
> >> in my eyes, it just means if this is the direction we want to go we
> >> should implement a lsm_unshare(2) API, or something similar.
> > Stephen's take on this is correct, the least complicated path forward
> > is a simple call, presumably lsm_unshare(2), that instructs the LSM(s)
> > to carry out whatever is needed to create a new security namespace.
> >
> > There are only two public implementations of what can be referred to
> > as major security namespacing efforts; Stephen's work with SeLinux and
> > our TSEM implementation.
>
> Please be just a tiny bit careful before you make this sort of assertion:
>
> https://lwn.net/Articles/645403/
I believe both AppArmor and TOMOYO also have namespacing
implementations already upstream, so SELinux is certainly not the only
one. Looks like the Smack implementation you cited above was based on
extending user namespaces rather than purely Smack-internal like the
others; is that why it wasn't ultimately merged?
^ permalink raw reply
* Re: [PATCH 0/3] Firmware LSM hook
From: Paul Moore @ 2026-03-09 18:32 UTC (permalink / raw)
To: Leon Romanovsky
Cc: James Morris, Serge E. Hallyn, Jason Gunthorpe, Saeed Mahameed,
Itay Avraham, Dave Jiang, Jonathan Cameron, linux-security-module,
linux-kernel, linux-rdma, Chiara Meiohas, Maher Sanalla,
Edward Srouji
In-Reply-To: <20260309-fw-lsm-hook-v1-0-4a6422e63725@nvidia.com>
On Mon, Mar 9, 2026 at 7:15 AM Leon Romanovsky <leon@kernel.org> wrote:
>
> From Chiara:
>
> This patch set introduces a new LSM hook to validate firmware commands
> triggered by userspace before they are submitted to the device. The hook
> runs after the command buffer is constructed, right before it is sent
> to firmware.
>
> The goal is to allow a security module to allow or deny a given command
> before it is submitted to firmware. BPF LSM can attach to this hook
> to implement such policies. This allows fine-grained policies for different
> firmware commands.
>
> In this series, the new hook is called from RDMA uverbs and from the fwctl
> subsystem. Both the uverbs and fwctl interfaces use ioctl, so an obvious
> candidate would seem to be the file_ioctl hook. However, the userspace
> attributes used to build the firmware command buffer are copied from
> userspace (copy_from_user()) deep in the driver, depending on various
> conditions. As a result, file_ioctl does not have the information required
> to make a policy decision.
>
> This newly introduced hook provides the command buffer together with relevant
> metadata (device, command class, and a class-specific device identifier), so
> security modules can distinguish between different command classes and devices.
>
> The hook can be used by other drivers that submit firmware commands via a command
> buffer.
Hi Leon,
At the link below, you'll find guidance on submitting new LSM hooks.
Please take a look and let me know if you have any questions.
https://github.com/LinuxSecurityModule/kernel/blob/main/README.md#new-lsm-hooks
(If you lose the link, or simply for future reference, you can find it
in the "SECURITY SUBSYSTEM" MAINTAINERS entry.)
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v3 00/12] vfs: change inode->i_ino from unsigned long to u64
From: Mimi Zohar @ 2026-03-09 19:00 UTC (permalink / raw)
To: Jeff Layton
Cc: linux-fsdevel, linux-kernel, linux-trace-kernel, nvdimm, fsverity,
linux-mm, netfs, linux-ext4, linux-f2fs-devel, linux-nfs,
linux-cifs, samba-technical, linux-nilfs, v9fs, linux-afs, autofs,
ceph-devel, codalist, ecryptfs, linux-mtd, jfs-discussion, ntfs3,
ocfs2-devel, devel, linux-unionfs, apparmor,
linux-security-module, linux-integrity, selinux, amd-gfx,
dri-devel, linux-media, linaro-mm-sig, netdev, linux-perf-users,
linux-fscrypt, linux-xfs, linux-hams, linux-x25, audit,
linux-bluetooth, linux-can, linux-sctp, bpf
In-Reply-To: <f22758116dabd3c135a833bcb5cfcd2ea4f6ecf4.camel@kernel.org>
On Mon, 2026-03-09 at 13:59 -0400, Jeff Layton wrote:
> On Mon, 2026-03-09 at 13:47 -0400, Mimi Zohar wrote:
> > [ I/O socket time out. Trimming the To list.]
> >
> > On Wed, 2026-03-04 at 10:32 -0500, Jeff Layton wrote:
> > > This version squashes all of the format-string changes and the i_ino
> > > type change into the same patch. This results in a giant 600+ line patch
> > > at the end of the series, but it does remain bisectable. Because the
> > > patchset was reorganized (again) some of the R-b's and A-b's have been
> > > dropped.
> > >
> > > The entire pile is in the "iino-u64" branch of my tree, if anyone is
> > > interested in testing this.
> > >
> > > https://git.kernel.org/pub/scm/linux/kernel/git/jlayton/linux.git/
> > >
> > > Original cover letter follows:
> > >
> > > ----------------------8<-----------------------
> > >
> > > Christian said [1] to "just do it" when I proposed this, so here we are!
> > >
> > > For historical reasons, the inode->i_ino field is an unsigned long,
> > > which means that it's 32 bits on 32 bit architectures. This has caused a
> > > number of filesystems to implement hacks to hash a 64-bit identifier
> > > into a 32-bit field, and deprives us of a universal identifier field for
> > > an inode.
> > >
> > > This patchset changes the inode->i_ino field from an unsigned long to a
> > > u64. This shouldn't make any material difference on 64-bit hosts, but
> > > 32-bit hosts will see struct inode grow by at least 4 bytes. This could
> > > have effects on slabcache sizes and field alignment.
> > >
> > > The bulk of the changes are to format strings and tracepoints, since the
> > > kernel itself doesn't care that much about the i_ino field. The first
> > > patch changes some vfs function arguments, so check that one out
> > > carefully.
> > >
> > > With this change, we may be able to shrink some inode structures. For
> > > instance, struct nfs_inode has a fileid field that holds the 64-bit
> > > inode number. With this set of changes, that field could be eliminated.
> > > I'd rather leave that sort of cleanups for later just to keep this
> > > simple.
> > >
> > > Much of this set was generated by LLM, but I attributed it to myself
> > > since I consider this to be in the "menial tasks" category of LLM usage.
> > >
> > > [1]: https://lore.kernel.org/linux-fsdevel/20260219-portrait-winkt-959070cee42f@brauner/
> > >
> > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> >
> > Jeff, missing from this patch set is EVM. In hmac_add_misc() EVM copies the
> > i_ino and calculates either an HMAC or file meta-data hash, which is then
> > signed.
> >
> >
>
> Thanks Mimi, good catch.
>
> It looks like we should just be able to change the ino field to a u64
> alongside everything else. Something like this:
>
> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> index c0ca4eedb0fe..77b6c2fa345e 100644
> --- a/security/integrity/evm/evm_crypto.c
> +++ b/security/integrity/evm/evm_crypto.c
> @@ -144,7 +144,7 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
> char type, char *digest)
> {
> struct h_misc {
> - unsigned long ino;
> + u64 ino;
> __u32 generation;
> uid_t uid;
> gid_t gid;
>
Agreed.
>
> That should make no material difference on 64-bit hosts. What's the
> effect on 32-bit? Will they just need to remeasure everything or would
> the consequences be more dire? Do we have any clue whether anyone is
> using EVM in 32-bit environments?
All good questions. Unfortunately I don't know the answer to most of them. What
we do know: changing the size of the i_ino field would affect EVM file metadata
verification and would require relabeling the filesystem. Even packages
containing EVM portable signatures, which don't include or verify the i_ino
number, would be affected.
Mimi
^ permalink raw reply
* Re: [PATCH v3 00/12] vfs: change inode->i_ino from unsigned long to u64
From: Jeff Layton @ 2026-03-09 19:33 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-fsdevel, linux-kernel, linux-trace-kernel, nvdimm, fsverity,
linux-mm, netfs, linux-ext4, linux-f2fs-devel, linux-nfs,
linux-cifs, samba-technical, linux-nilfs, v9fs, linux-afs, autofs,
ceph-devel, codalist, ecryptfs, linux-mtd, jfs-discussion, ntfs3,
ocfs2-devel, devel, linux-unionfs, apparmor,
linux-security-module, linux-integrity, selinux, amd-gfx,
dri-devel, linux-media, linaro-mm-sig, netdev, linux-perf-users,
linux-fscrypt, linux-xfs, linux-hams, linux-x25, audit,
linux-bluetooth, linux-can, linux-sctp, bpf
In-Reply-To: <c9500adc562665d44feaca9206f23a5ba07432c1.camel@linux.ibm.com>
On Mon, 2026-03-09 at 15:00 -0400, Mimi Zohar wrote:
> On Mon, 2026-03-09 at 13:59 -0400, Jeff Layton wrote:
> > On Mon, 2026-03-09 at 13:47 -0400, Mimi Zohar wrote:
> > > [ I/O socket time out. Trimming the To list.]
> > >
> > > On Wed, 2026-03-04 at 10:32 -0500, Jeff Layton wrote:
> > > > This version squashes all of the format-string changes and the i_ino
> > > > type change into the same patch. This results in a giant 600+ line patch
> > > > at the end of the series, but it does remain bisectable. Because the
> > > > patchset was reorganized (again) some of the R-b's and A-b's have been
> > > > dropped.
> > > >
> > > > The entire pile is in the "iino-u64" branch of my tree, if anyone is
> > > > interested in testing this.
> > > >
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/jlayton/linux.git/
> > > >
> > > > Original cover letter follows:
> > > >
> > > > ----------------------8<-----------------------
> > > >
> > > > Christian said [1] to "just do it" when I proposed this, so here we are!
> > > >
> > > > For historical reasons, the inode->i_ino field is an unsigned long,
> > > > which means that it's 32 bits on 32 bit architectures. This has caused a
> > > > number of filesystems to implement hacks to hash a 64-bit identifier
> > > > into a 32-bit field, and deprives us of a universal identifier field for
> > > > an inode.
> > > >
> > > > This patchset changes the inode->i_ino field from an unsigned long to a
> > > > u64. This shouldn't make any material difference on 64-bit hosts, but
> > > > 32-bit hosts will see struct inode grow by at least 4 bytes. This could
> > > > have effects on slabcache sizes and field alignment.
> > > >
> > > > The bulk of the changes are to format strings and tracepoints, since the
> > > > kernel itself doesn't care that much about the i_ino field. The first
> > > > patch changes some vfs function arguments, so check that one out
> > > > carefully.
> > > >
> > > > With this change, we may be able to shrink some inode structures. For
> > > > instance, struct nfs_inode has a fileid field that holds the 64-bit
> > > > inode number. With this set of changes, that field could be eliminated.
> > > > I'd rather leave that sort of cleanups for later just to keep this
> > > > simple.
> > > >
> > > > Much of this set was generated by LLM, but I attributed it to myself
> > > > since I consider this to be in the "menial tasks" category of LLM usage.
> > > >
> > > > [1]: https://lore.kernel.org/linux-fsdevel/20260219-portrait-winkt-959070cee42f@brauner/
> > > >
> > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > >
> > > Jeff, missing from this patch set is EVM. In hmac_add_misc() EVM copies the
> > > i_ino and calculates either an HMAC or file meta-data hash, which is then
> > > signed.
> > >
> > >
> >
> > Thanks Mimi, good catch.
> >
> > It looks like we should just be able to change the ino field to a u64
> > alongside everything else. Something like this:
> >
> > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> > index c0ca4eedb0fe..77b6c2fa345e 100644
> > --- a/security/integrity/evm/evm_crypto.c
> > +++ b/security/integrity/evm/evm_crypto.c
> > @@ -144,7 +144,7 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
> > char type, char *digest)
> > {
> > struct h_misc {
> > - unsigned long ino;
> > + u64 ino;
> > __u32 generation;
> > uid_t uid;
> > gid_t gid;
> >
>
> Agreed.
>
> >
> > That should make no material difference on 64-bit hosts. What's the
> > effect on 32-bit? Will they just need to remeasure everything or would
> > the consequences be more dire? Do we have any clue whether anyone is
> > using EVM in 32-bit environments?
>
> All good questions. Unfortunately I don't know the answer to most of them. What
> we do know: changing the size of the i_ino field would affect EVM file metadata
> verification and would require relabeling the filesystem. Even packages
> containing EVM portable signatures, which don't include or verify the i_ino
> number, would be affected.
>
Ouch. Technically, I guess this is ABI...
While converting to u64 seems like the ideal thing to do, the other
option might be to just keep this as an unsigned long for now.
No effect on 64-bit, but that could keep things working 32-bit when the
i_ino casts properly to a u32. ext4 would be fine since they don't
issue inode numbers larger than UINT_MAX. xfs and btrfs are a bit more
iffy, but worst case they'd just need to be relabeled (which is what
they'll need to do anyway).
If we do that, then we should probably add a comment to this function
explaining why it's an unsigned long.
Thoughts?
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH 0/3] Firmware LSM hook
From: Leon Romanovsky @ 2026-03-09 19:37 UTC (permalink / raw)
To: Paul Moore
Cc: James Morris, Serge E. Hallyn, Jason Gunthorpe, Saeed Mahameed,
Itay Avraham, Dave Jiang, Jonathan Cameron, linux-security-module,
linux-kernel, linux-rdma, Chiara Meiohas, Maher Sanalla,
Edward Srouji
In-Reply-To: <CAHC9VhTR9CsBgxRCAHXm5T2NZ5tr+XfmA--zkt=udmk9hPRuZQ@mail.gmail.com>
On Mon, Mar 09, 2026 at 02:32:39PM -0400, Paul Moore wrote:
> On Mon, Mar 9, 2026 at 7:15 AM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > From Chiara:
> >
> > This patch set introduces a new LSM hook to validate firmware commands
> > triggered by userspace before they are submitted to the device. The hook
> > runs after the command buffer is constructed, right before it is sent
> > to firmware.
> >
> > The goal is to allow a security module to allow or deny a given command
> > before it is submitted to firmware. BPF LSM can attach to this hook
> > to implement such policies. This allows fine-grained policies for different
> > firmware commands.
> >
> > In this series, the new hook is called from RDMA uverbs and from the fwctl
> > subsystem. Both the uverbs and fwctl interfaces use ioctl, so an obvious
> > candidate would seem to be the file_ioctl hook. However, the userspace
> > attributes used to build the firmware command buffer are copied from
> > userspace (copy_from_user()) deep in the driver, depending on various
> > conditions. As a result, file_ioctl does not have the information required
> > to make a policy decision.
> >
> > This newly introduced hook provides the command buffer together with relevant
> > metadata (device, command class, and a class-specific device identifier), so
> > security modules can distinguish between different command classes and devices.
> >
> > The hook can be used by other drivers that submit firmware commands via a command
> > buffer.
>
> Hi Leon,
>
> At the link below, you'll find guidance on submitting new LSM hooks.
> Please take a look and let me know if you have any questions.
>
> https://github.com/LinuxSecurityModule/kernel/blob/main/README.md#new-lsm-hooks
I assume that you are referring to this part:
* New LSM hooks must demonstrate their usefulness by providing a meaningful
implementation for at least one in-kernel LSM. The goal is to demonstrate
the purpose and expected semantics of the hooks. Out of tree kernel code,
and pass through implementations, such as the BPF LSM, are not eligible
for LSM hook reference implementations.
The point is that we are not inspecting a kernel call, but the FW mailbox,
which has very little meaning to the kernel. From the kernel's perspective,
all relevant checks have already been performed, but the existing capability
granularity does not allow us to distinguish between FW_CMD1 and FW_CMD2.
Here we propose a generic interface that can be applied to all FWCTL
devices without out-of-tree kernel code at all.
Thanks
>
> (If you lose the link, or simply for future reference, you can find it
> in the "SECURITY SUBSYSTEM" MAINTAINERS entry.)
>
> --
> paul-moore.com
>
^ permalink raw reply
* Re: [PATCH v3 00/12] vfs: change inode->i_ino from unsigned long to u64
From: Mimi Zohar @ 2026-03-09 20:11 UTC (permalink / raw)
To: Jeff Layton
Cc: linux-fsdevel, linux-kernel, linux-trace-kernel, nvdimm, fsverity,
linux-mm, netfs, linux-ext4, linux-f2fs-devel, linux-nfs,
linux-cifs, samba-technical, linux-nilfs, v9fs, linux-afs, autofs,
ceph-devel, codalist, ecryptfs, linux-mtd, jfs-discussion, ntfs3,
ocfs2-devel, devel, linux-unionfs, apparmor,
linux-security-module, linux-integrity, selinux, amd-gfx,
dri-devel, linux-media, linaro-mm-sig, netdev, linux-perf-users,
linux-fscrypt, linux-xfs, linux-hams, linux-x25, audit,
linux-bluetooth, linux-can, linux-sctp, bpf
In-Reply-To: <dd3f9873c7939fba0ca2366effd24e4b6326f17b.camel@kernel.org>
On Mon, 2026-03-09 at 15:33 -0400, Jeff Layton wrote:
> On Mon, 2026-03-09 at 15:00 -0400, Mimi Zohar wrote:
> > On Mon, 2026-03-09 at 13:59 -0400, Jeff Layton wrote:
> > > On Mon, 2026-03-09 at 13:47 -0400, Mimi Zohar wrote:
> > > > [ I/O socket time out. Trimming the To list.]
> > > >
> > > > On Wed, 2026-03-04 at 10:32 -0500, Jeff Layton wrote:
> > > > > This version squashes all of the format-string changes and the i_ino
> > > > > type change into the same patch. This results in a giant 600+ line patch
> > > > > at the end of the series, but it does remain bisectable. Because the
> > > > > patchset was reorganized (again) some of the R-b's and A-b's have been
> > > > > dropped.
> > > > >
> > > > > The entire pile is in the "iino-u64" branch of my tree, if anyone is
> > > > > interested in testing this.
> > > > >
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/jlayton/linux.git/
> > > > >
> > > > > Original cover letter follows:
> > > > >
> > > > > ----------------------8<-----------------------
> > > > >
> > > > > Christian said [1] to "just do it" when I proposed this, so here we are!
> > > > >
> > > > > For historical reasons, the inode->i_ino field is an unsigned long,
> > > > > which means that it's 32 bits on 32 bit architectures. This has caused a
> > > > > number of filesystems to implement hacks to hash a 64-bit identifier
> > > > > into a 32-bit field, and deprives us of a universal identifier field for
> > > > > an inode.
> > > > >
> > > > > This patchset changes the inode->i_ino field from an unsigned long to a
> > > > > u64. This shouldn't make any material difference on 64-bit hosts, but
> > > > > 32-bit hosts will see struct inode grow by at least 4 bytes. This could
> > > > > have effects on slabcache sizes and field alignment.
> > > > >
> > > > > The bulk of the changes are to format strings and tracepoints, since the
> > > > > kernel itself doesn't care that much about the i_ino field. The first
> > > > > patch changes some vfs function arguments, so check that one out
> > > > > carefully.
> > > > >
> > > > > With this change, we may be able to shrink some inode structures. For
> > > > > instance, struct nfs_inode has a fileid field that holds the 64-bit
> > > > > inode number. With this set of changes, that field could be eliminated.
> > > > > I'd rather leave that sort of cleanups for later just to keep this
> > > > > simple.
> > > > >
> > > > > Much of this set was generated by LLM, but I attributed it to myself
> > > > > since I consider this to be in the "menial tasks" category of LLM usage.
> > > > >
> > > > > [1]: https://lore.kernel.org/linux-fsdevel/20260219-portrait-winkt-959070cee42f@brauner/
> > > > >
> > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > >
> > > > Jeff, missing from this patch set is EVM. In hmac_add_misc() EVM copies the
> > > > i_ino and calculates either an HMAC or file meta-data hash, which is then
> > > > signed.
> > > >
> > > >
> > >
> > > Thanks Mimi, good catch.
> > >
> > > It looks like we should just be able to change the ino field to a u64
> > > alongside everything else. Something like this:
> > >
> > > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> > > index c0ca4eedb0fe..77b6c2fa345e 100644
> > > --- a/security/integrity/evm/evm_crypto.c
> > > +++ b/security/integrity/evm/evm_crypto.c
> > > @@ -144,7 +144,7 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
> > > char type, char *digest)
> > > {
> > > struct h_misc {
> > > - unsigned long ino;
> > > + u64 ino;
> > > __u32 generation;
> > > uid_t uid;
> > > gid_t gid;
> > >
> >
> > Agreed.
> >
> > >
> > > That should make no material difference on 64-bit hosts. What's the
> > > effect on 32-bit? Will they just need to remeasure everything or would
> > > the consequences be more dire? Do we have any clue whether anyone is
> > > using EVM in 32-bit environments?
> >
> > All good questions. Unfortunately I don't know the answer to most of them. What
> > we do know: changing the size of the i_ino field would affect EVM file metadata
> > verification and would require relabeling the filesystem. Even packages
> > containing EVM portable signatures, which don't include or verify the i_ino
> > number, would be affected.
> >
>
> Ouch. Technically, I guess this is ABI...
>
> While converting to u64 seems like the ideal thing to do, the other
> option might be to just keep this as an unsigned long for now.
>
> No effect on 64-bit, but that could keep things working 32-bit when the
> i_ino casts properly to a u32. ext4 would be fine since they don't
> issue inode numbers larger than UINT_MAX. xfs and btrfs are a bit more
> iffy, but worst case they'd just need to be relabeled (which is what
> they'll need to do anyway).
>
> If we do that, then we should probably add a comment to this function
> explaining why it's an unsigned long.
Agreed.
>
> Thoughts?
My concern would be embedded/IoT devices, but I don't have any insight into who
might be using it on 32 bit.
Mimi
^ permalink raw reply
* [PATCH] integrity: Eliminate weak definition of arch_get_secureboot()
From: Nathan Chancellor @ 2026-03-09 20:37 UTC (permalink / raw)
To: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg
Cc: Arnd Bergmann, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Paul Moore, James Morris, Serge E. Hallyn,
Coiby Xu, linux-kernel, linuxppc-dev, linux-s390, linux-integrity,
linux-security-module, llvm, Nathan Chancellor
security/integrity/secure_boot.c contains a single __weak function,
which breaks recordmcount when building with clang:
$ make -skj"$(nproc)" ARCH=powerpc LLVM=1 ppc64_defconfig security/integrity/secure_boot.o
Cannot find symbol for section 2: .text.
security/integrity/secure_boot.o: failed
Introduce a Kconfig symbol, CONFIG_HAVE_ARCH_GET_SECUREBOOT, to indicate
that an architecture provides a definition of arch_get_secureboot().
Provide a static inline stub when this symbol is not defined to achieve
the same effect as the __weak function, allowing secure_boot.c to be
removed altogether. Move the s390 definition of arch_get_secureboot()
out of the CONFIG_KEXEC_FILE block to ensure it is always available, as
it does not actually depend on KEXEC_FILE.
Fixes: 31a6a07eefeb ("integrity: Make arch_ima_get_secureboot integrity-wide")
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
arch/Kconfig | 3 +++
arch/powerpc/Kconfig | 1 +
arch/s390/Kconfig | 1 +
arch/s390/kernel/ipl.c | 10 +++++-----
include/linux/secure_boot.h | 4 ++++
security/integrity/Makefile | 2 +-
security/integrity/secure_boot.c | 16 ----------------
7 files changed, 15 insertions(+), 22 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index 102ddbd4298e..a6d1c8cc1d64 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1841,4 +1841,7 @@ config ARCH_WANTS_PRE_LINK_VMLINUX
config ARCH_HAS_CPU_ATTACK_VECTORS
bool
+config HAVE_ARCH_GET_SECUREBOOT
+ def_bool EFI
+
endmenu
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index ad7a2fe63a2a..da1eafb64354 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -1061,6 +1061,7 @@ config PPC_SECURE_BOOT
depends on IMA_ARCH_POLICY
imply IMA_SECURE_AND_OR_TRUSTED_BOOT
select PSERIES_PLPKS if PPC_PSERIES
+ select HAVE_ARCH_GET_SECUREBOOT
help
Systems with firmware secure boot enabled need to define security
policies to extend secure boot to the OS. This config allows a user
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 2101cc738b5e..4197c20d34b4 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -181,6 +181,7 @@ config S390
select GENERIC_IOREMAP if PCI
select HAVE_ALIGNED_STRUCT_PAGE
select HAVE_ARCH_AUDITSYSCALL
+ select HAVE_ARCH_GET_SECUREBOOT
select HAVE_ARCH_JUMP_LABEL
select HAVE_ARCH_JUMP_LABEL_RELATIVE
select HAVE_ARCH_KASAN
diff --git a/arch/s390/kernel/ipl.c b/arch/s390/kernel/ipl.c
index 2d01a1713938..3c346b02ceb9 100644
--- a/arch/s390/kernel/ipl.c
+++ b/arch/s390/kernel/ipl.c
@@ -2388,6 +2388,11 @@ void __no_stack_protector s390_reset_system(void)
diag_amode31_ops.diag308_reset();
}
+bool arch_get_secureboot(void)
+{
+ return ipl_secure_flag;
+}
+
#ifdef CONFIG_KEXEC_FILE
int ipl_report_add_component(struct ipl_report *report, struct kexec_buf *kbuf,
@@ -2505,11 +2510,6 @@ void *ipl_report_finish(struct ipl_report *report)
return buf;
}
-bool arch_get_secureboot(void)
-{
- return ipl_secure_flag;
-}
-
int ipl_report_free(struct ipl_report *report)
{
struct ipl_report_component *comp, *ncomp;
diff --git a/include/linux/secure_boot.h b/include/linux/secure_boot.h
index 3ded3f03655c..d17e92351567 100644
--- a/include/linux/secure_boot.h
+++ b/include/linux/secure_boot.h
@@ -10,10 +10,14 @@
#include <linux/types.h>
+#ifdef CONFIG_HAVE_ARCH_GET_SECUREBOOT
/*
* Returns true if the platform secure boot is enabled.
* Returns false if disabled or not supported.
*/
bool arch_get_secureboot(void);
+#else
+static inline bool arch_get_secureboot(void) { return false; }
+#endif
#endif /* _LINUX_SECURE_BOOT_H */
diff --git a/security/integrity/Makefile b/security/integrity/Makefile
index 548665e2b702..45dfdedbdad4 100644
--- a/security/integrity/Makefile
+++ b/security/integrity/Makefile
@@ -5,7 +5,7 @@
obj-$(CONFIG_INTEGRITY) += integrity.o
-integrity-y := iint.o secure_boot.o
+integrity-y := iint.o
integrity-$(CONFIG_INTEGRITY_AUDIT) += integrity_audit.o
integrity-$(CONFIG_INTEGRITY_SIGNATURE) += digsig.o
integrity-$(CONFIG_INTEGRITY_ASYMMETRIC_KEYS) += digsig_asymmetric.o
diff --git a/security/integrity/secure_boot.c b/security/integrity/secure_boot.c
deleted file mode 100644
index fc2693c286f8..000000000000
--- a/security/integrity/secure_boot.c
+++ /dev/null
@@ -1,16 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Copyright (C) 2026 Red Hat, Inc. All Rights Reserved.
- *
- * Author: Coiby Xu <coxu@redhat.com>
- */
-#include <linux/secure_boot.h>
-
-/*
- * Default weak implementation.
- * Architectures that support secure boot must override this.
- */
-__weak bool arch_get_secureboot(void)
-{
- return false;
-}
---
base-commit: 870819434c8dfcc3158033b66e7851b81bb17e21
change-id: 20260309-integrity-drop-weak-arch-get-secureboot-cead298d493f
Best regards,
--
Nathan Chancellor <nathan@kernel.org>
^ permalink raw reply related
* Re: [PATCH v3 00/12] vfs: change inode->i_ino from unsigned long to u64
From: Jeff Layton @ 2026-03-09 20:50 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-fsdevel, linux-kernel, linux-trace-kernel, nvdimm, fsverity,
linux-mm, netfs, linux-ext4, linux-f2fs-devel, linux-nfs,
linux-cifs, samba-technical, linux-nilfs, v9fs, linux-afs, autofs,
ceph-devel, codalist, ecryptfs, linux-mtd, jfs-discussion, ntfs3,
ocfs2-devel, devel, linux-unionfs, apparmor,
linux-security-module, linux-integrity, selinux, amd-gfx,
dri-devel, linux-media, linaro-mm-sig, netdev, linux-perf-users,
linux-fscrypt, linux-xfs, linux-hams, linux-x25, audit,
linux-bluetooth, linux-can, linux-sctp, bpf
In-Reply-To: <0bd92b4fce00a6111a0fc7764904f7e6ae0ece3a.camel@linux.ibm.com>
On Mon, 2026-03-09 at 16:11 -0400, Mimi Zohar wrote:
> On Mon, 2026-03-09 at 15:33 -0400, Jeff Layton wrote:
> > On Mon, 2026-03-09 at 15:00 -0400, Mimi Zohar wrote:
> > > On Mon, 2026-03-09 at 13:59 -0400, Jeff Layton wrote:
> > > > On Mon, 2026-03-09 at 13:47 -0400, Mimi Zohar wrote:
> > > > > [ I/O socket time out. Trimming the To list.]
> > > > >
> > > > > On Wed, 2026-03-04 at 10:32 -0500, Jeff Layton wrote:
> > > > > > This version squashes all of the format-string changes and the i_ino
> > > > > > type change into the same patch. This results in a giant 600+ line patch
> > > > > > at the end of the series, but it does remain bisectable. Because the
> > > > > > patchset was reorganized (again) some of the R-b's and A-b's have been
> > > > > > dropped.
> > > > > >
> > > > > > The entire pile is in the "iino-u64" branch of my tree, if anyone is
> > > > > > interested in testing this.
> > > > > >
> > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/jlayton/linux.git/
> > > > > >
> > > > > > Original cover letter follows:
> > > > > >
> > > > > > ----------------------8<-----------------------
> > > > > >
> > > > > > Christian said [1] to "just do it" when I proposed this, so here we are!
> > > > > >
> > > > > > For historical reasons, the inode->i_ino field is an unsigned long,
> > > > > > which means that it's 32 bits on 32 bit architectures. This has caused a
> > > > > > number of filesystems to implement hacks to hash a 64-bit identifier
> > > > > > into a 32-bit field, and deprives us of a universal identifier field for
> > > > > > an inode.
> > > > > >
> > > > > > This patchset changes the inode->i_ino field from an unsigned long to a
> > > > > > u64. This shouldn't make any material difference on 64-bit hosts, but
> > > > > > 32-bit hosts will see struct inode grow by at least 4 bytes. This could
> > > > > > have effects on slabcache sizes and field alignment.
> > > > > >
> > > > > > The bulk of the changes are to format strings and tracepoints, since the
> > > > > > kernel itself doesn't care that much about the i_ino field. The first
> > > > > > patch changes some vfs function arguments, so check that one out
> > > > > > carefully.
> > > > > >
> > > > > > With this change, we may be able to shrink some inode structures. For
> > > > > > instance, struct nfs_inode has a fileid field that holds the 64-bit
> > > > > > inode number. With this set of changes, that field could be eliminated.
> > > > > > I'd rather leave that sort of cleanups for later just to keep this
> > > > > > simple.
> > > > > >
> > > > > > Much of this set was generated by LLM, but I attributed it to myself
> > > > > > since I consider this to be in the "menial tasks" category of LLM usage.
> > > > > >
> > > > > > [1]: https://lore.kernel.org/linux-fsdevel/20260219-portrait-winkt-959070cee42f@brauner/
> > > > > >
> > > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > > >
> > > > > Jeff, missing from this patch set is EVM. In hmac_add_misc() EVM copies the
> > > > > i_ino and calculates either an HMAC or file meta-data hash, which is then
> > > > > signed.
> > > > >
> > > > >
> > > >
> > > > Thanks Mimi, good catch.
> > > >
> > > > It looks like we should just be able to change the ino field to a u64
> > > > alongside everything else. Something like this:
> > > >
> > > > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> > > > index c0ca4eedb0fe..77b6c2fa345e 100644
> > > > --- a/security/integrity/evm/evm_crypto.c
> > > > +++ b/security/integrity/evm/evm_crypto.c
> > > > @@ -144,7 +144,7 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
> > > > char type, char *digest)
> > > > {
> > > > struct h_misc {
> > > > - unsigned long ino;
> > > > + u64 ino;
> > > > __u32 generation;
> > > > uid_t uid;
> > > > gid_t gid;
> > > >
> > >
> > > Agreed.
> > >
> > > >
> > > > That should make no material difference on 64-bit hosts. What's the
> > > > effect on 32-bit? Will they just need to remeasure everything or would
> > > > the consequences be more dire? Do we have any clue whether anyone is
> > > > using EVM in 32-bit environments?
> > >
> > > All good questions. Unfortunately I don't know the answer to most of them. What
> > > we do know: changing the size of the i_ino field would affect EVM file metadata
> > > verification and would require relabeling the filesystem. Even packages
> > > containing EVM portable signatures, which don't include or verify the i_ino
> > > number, would be affected.
> > >
> >
> > Ouch. Technically, I guess this is ABI...
> >
> > While converting to u64 seems like the ideal thing to do, the other
> > option might be to just keep this as an unsigned long for now.
> >
> > No effect on 64-bit, but that could keep things working 32-bit when the
> > i_ino casts properly to a u32. ext4 would be fine since they don't
> > issue inode numbers larger than UINT_MAX. xfs and btrfs are a bit more
> > iffy, but worst case they'd just need to be relabeled (which is what
> > they'll need to do anyway).
> >
> > If we do that, then we should probably add a comment to this function
> > explaining why it's an unsigned long.
>
> Agreed.
>
For now, I think that's the best approach. I'll spin up a patch to add
the comment.
> >
> > Thoughts?
>
> My concern would be embedded/IoT devices, but I don't have any insight into who
> might be using it on 32 bit.
>
Yep. This LWN article on Arnd's talk lays out the state of things:
https://lwn.net/Articles/1035727/
The upshot is that it's hard to buy 32-bit CPUs these days, and will
only get harder. But, there are still a fair number of 32-bit devices
out in the field and will be for some time.
The big question is how many of those EVM users that intend to run new
kernels. I have no idea how to answer that.
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH] integrity: avoid using __weak functions
From: Nathan Chancellor @ 2026-03-09 21:29 UTC (permalink / raw)
To: Mimi Zohar
Cc: Arnd Bergmann, Madhavan Srinivasan, Michael Ellerman,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Arnd Bergmann,
Roberto Sassu, Dmitry Kasatkin, Paul Moore, James Morris,
Serge E. Hallyn, Jarkko Sakkinen, Ard Biesheuvel, Coiby Xu,
Nicholas Piggin, Christophe Leroy (CS GROUP),
Christian Borntraeger, Sven Schnelle, Eric Snowberg,
Nick Desaulniers, Bill Wendling, Justin Stitt, Andrew Donnellan,
linuxppc-dev, linux-kernel, linux-s390, linux-arch,
linux-integrity, linux-security-module, keyrings, llvm
In-Reply-To: <abb4d186d1ca76c080d5e15bcd9e0019cce3af39.camel@linux.ibm.com>
On Sun, Mar 08, 2026 at 09:01:58PM -0400, Mimi Zohar wrote:
> Thanks Arnd, Nathan. LGTM. Nathan, could you send a patch with a proper patch
> description.
Done, thanks for the input!
https://lore.kernel.org/20260309-integrity-drop-weak-arch-get-secureboot-v1-1-6460d5c4bb89@kernel.org/
Cheers,
Nathan
^ permalink raw reply
* [PATCH RESEND] apparmor: Replace memcpy + NUL termination with kmemdup_nul in do_setattr
From: Thorsten Blum @ 2026-03-09 22:41 UTC (permalink / raw)
To: John Johansen, Paul Moore, James Morris, Serge E. Hallyn
Cc: Thorsten Blum, apparmor, linux-security-module, linux-kernel
Use kmemdup_nul() to copy 'value' instead of using memcpy() followed by
a manual NUL termination. No functional changes.
Reviewed-by: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
security/apparmor/lsm.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index c1d42fc72fdb..49aa6ad68838 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -858,12 +858,9 @@ static int do_setattr(u64 attr, void *value, size_t size)
/* AppArmor requires that the buffer must be null terminated atm */
if (args[size - 1] != '\0') {
- /* null terminate */
- largs = args = kmalloc(size + 1, GFP_KERNEL);
+ largs = args = kmemdup_nul(value, size, GFP_KERNEL);
if (!args)
return -ENOMEM;
- memcpy(args, value, size);
- args[size] = '\0';
}
error = -EINVAL;
^ permalink raw reply related
* Re: [PATCH v2 1/2] keys/trusted_keys: clean up debug message logging in the tpm backend
From: Nayna Jain @ 2026-03-09 22:45 UTC (permalink / raw)
To: Srish Srinivasan, linux-integrity, keyrings
Cc: James.Bottomley, jarkko, zohar, stefanb, linux-kernel,
linux-security-module
In-Reply-To: <20260220183426.80446-2-ssrish@linux.ibm.com>
On 2/20/26 1:34 PM, Srish Srinivasan wrote:
> The TPM trusted-keys backend uses a local TPM_DEBUG guard and pr_info()
> for logging debug information.
>
> Replace pr_info() with pr_debug(), and use KERN_DEBUG for print_hex_dump().
> Remove TPM_DEBUG.
>
> No functional change intended.
There is functional change here. This change allows secret and nonce in
the function dump_sess() to be logged to kernel logs when dynamic debug
is enabled. Previously, it was possible only in the debug builds and not
the production builds at runtime. With this change, it is always there
in production build. This can result in possible attack.
Instead of doing this change, I think add a comment to prevent this sort
of change in the future.
Thanks & Regards,
- Nayna
>
> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
> security/keys/trusted-keys/trusted_tpm1.c | 40 +++++++----------------
> 1 file changed, 12 insertions(+), 28 deletions(-)
>
> diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> index c865c97aa1b4..216caef97ffc 100644
> --- a/security/keys/trusted-keys/trusted_tpm1.c
> +++ b/security/keys/trusted-keys/trusted_tpm1.c
> @@ -46,28 +46,25 @@ enum {
> SRK_keytype = 4
> };
>
> -#define TPM_DEBUG 0
> -
> -#if TPM_DEBUG
> static inline void dump_options(struct trusted_key_options *o)
> {
> - pr_info("sealing key type %d\n", o->keytype);
> - pr_info("sealing key handle %0X\n", o->keyhandle);
> - pr_info("pcrlock %d\n", o->pcrlock);
> - pr_info("pcrinfo %d\n", o->pcrinfo_len);
> - print_hex_dump(KERN_INFO, "pcrinfo ", DUMP_PREFIX_NONE,
> + pr_debug("sealing key type %d\n", o->keytype);
> + pr_debug("sealing key handle %0X\n", o->keyhandle);
> + pr_debug("pcrlock %d\n", o->pcrlock);
> + pr_debug("pcrinfo %d\n", o->pcrinfo_len);
> + print_hex_dump(KERN_DEBUG, "pcrinfo ", DUMP_PREFIX_NONE,
> 16, 1, o->pcrinfo, o->pcrinfo_len, 0);
> }
>
> static inline void dump_sess(struct osapsess *s)
> {
> - print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
> + print_hex_dump(KERN_DEBUG, "trusted-key: handle ", DUMP_PREFIX_NONE,
> 16, 1, &s->handle, 4, 0);
> - pr_info("secret:\n");
> - print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
> + pr_debug("secret:\n");
> + print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
> 16, 1, &s->secret, SHA1_DIGEST_SIZE, 0);
> - pr_info("trusted-key: enonce:\n");
> - print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
> + pr_debug("trusted-key: enonce:\n");
> + print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
> 16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
> }
>
> @@ -75,23 +72,10 @@ static inline void dump_tpm_buf(unsigned char *buf)
> {
> int len;
>
> - pr_info("\ntpm buffer\n");
> + pr_debug("\ntpm buffer\n");
> len = LOAD32(buf, TPM_SIZE_OFFSET);
> - print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
> -}
> -#else
> -static inline void dump_options(struct trusted_key_options *o)
> -{
> -}
> -
> -static inline void dump_sess(struct osapsess *s)
> -{
> -}
> -
> -static inline void dump_tpm_buf(unsigned char *buf)
> -{
> + print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
> }
> -#endif
>
> static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
> unsigned int keylen, ...)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox