* Re: [PATCH 0/2] ima: add dont_audit and fs_subtype to policy language
From: Jann Horn @ 2025-10-14 15:55 UTC (permalink / raw)
To: Mimi Zohar
Cc: Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Frank Dinoff,
linux-kernel, linux-integrity
In-Reply-To: <ef7c07585e41c8afbb2b97df98fd47c9374b15cb.camel@linux.ibm.com>
Hi!
On Tue, Sep 30, 2025 at 12:23 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Fri, 2025-09-26 at 01:45 +0200, Jann Horn wrote:
> > This series adds a "dont_audit" action that cancels out following
> > "audit" actions (as we already have for other action types), and also
> > adds an "fs_subtype" that can be used to distinguish between FUSE
> > filesystems.
> >
> > With these two patches applied, as a toy example, you can use the
> > following policy:
> > ```
> > dont_audit fsname=fuse fs_subtype=sshfs
> > audit func=BPRM_CHECK fsname=fuse
> > ```
> >
> > I have tested that with this policy, executing a binary from a
> > "fuse-zip" FUSE filesystem results in an audit log entry:
> > ```
> > type=INTEGRITY_RULE msg=audit([...]): file="/home/user/ima/zipmount/usr/bin/echo" hash="sha256:1d82e8[...]
> > ```
> > while executing a binary from an "sshfs" FUSE filesystem does not
> > generate any audit log entries.
> >
> > Signed-off-by: Jann Horn <jannh@google.com>
>
>
> Thanks, Jann. The patches look fine.
What's the next step here - are the patches going to land in the
next-integrity branch, so that they will go into 6.19?
^ permalink raw reply
* [PATCH 1/2] vfs: Allow filesystems with foreign owner IDs to override UID checks
From: David Howells @ 2025-10-14 13:35 UTC (permalink / raw)
To: Al Viro, Christian Brauner
Cc: David Howells, Marc Dionne, Jeffrey Altman, Steve French,
linux-afs, openafs-devel, linux-cifs, linux-nfs, linux-fsdevel,
linux-kernel, Etienne Champetier, Chet Ramey, Cheyenne Wills,
Christian Brauner, Mimi Zohar, linux-integrity
In-Reply-To: <20251014133551.82642-1-dhowells@redhat.com>
A number of ownership checks made by the VFS make a number of assumptions:
(1) that it is meaningful to compare inode->i_uid to a second ->i_uid or
to current_fsuid(),
(2) that current_fsuid() represents the subject of the action,
(3) that the number in ->i_uid belong to the system's ID space and
(4) that the IDs can be represented by 32-bit integers.
Network filesystems, however, may violate all four of these assumptions.
Indeed, a network filesystem may not even have an actual concept of a UNIX
integer UID (cifs without POSIX extensions, for example). Plug-in block
filesystems (e.g. USB drives) may also violate this assumption.
In particular, AFS implements its own ACL security model with its own
per-cell user ID space with 64-bit IDs for some server variants. The
subject is represented by a token in a key, not current_fsuid(). The AFS
user IDs and the system user IDs for a cell may be numerically equivalent,
but that's matter of administrative policy and should perhaps be noted in
the cell definition or by mount option. A subsequent patch will address
AFS.
To help fix this, three functions are defined to perform UID comparison
within the VFS:
(1) vfs_inode_is_owned_by_me(). This defaults to comparing i_uid to
current_fsuid(), with appropriate namespace mapping, assuming that the
fsuid identifies the subject of the action. The filesystem may
override it by implementing an inode op:
int (*is_owned_by_me)(struct mnt_idmap *idmap, struct inode *inode);
This should return 0 if owned, 1 if not or an error if there's some
sort of lookup failure. It may use a means of identifying the subject
of the action other than fsuid, for example by using an authentication
token stored in a key.
(2) vfs_inodes_have_same_owner(). This defaults to comparing the i_uids
of two different inodes with appropriate namespace mapping. The
filesystem may override it by implementing another inode op:
int (*have_same_owner)(struct mnt_idmap *idmap, struct inode *inode1,
struct inode *inode2);
Again, this should return 0 if matching, 1 if not or an error if
there's some sort of lookup failure.
(3) vfs_inode_and_dir_have_same_owner(). This is similar to (2), but
assumes that the second inode is the parent directory to the first and
takes a nameidata struct instead of a second inode pointer.
Fix a number of places within the VFS where such UID checks are made that
should be deferring interpretation to the filesystem.
(*) chown_ok()
(*) chgrp_ok()
Call vfs_inode_is_owned_by_me(). Possibly these need to defer all
their checks to the network filesystem as the interpretation of the
new UID/GID depends on the netfs too, but the ->setattr() method gets
a chance to deal with that.
(*) coredump_file()
Call vfs_is_owned_by_me() to check that the file created is owned by
the caller - but the check that's there might be sufficient.
(*) inode_owner_or_capable()
Call vfs_is_owned_by_me(). I'm not sure whether the namespace mapping
makes sense in such a case, but it probably could be used.
(*) vfs_setlease()
Call vfs_is_owned_by_me(). Actually, it should query if leasing is
permitted.
Also, setting locks could perhaps do with a permission call to the
filesystem driver as AFS, for example, has a lock permission bit in
the ACL, but since the AFS server checks that when the RPC call is
made, it's probably unnecessary.
(*) acl_permission_check()
(*) posix_acl_permission()
Unchanged. These functions are only used by generic_permission()
which is overridden if ->permission() is supplied, and when evaluating
a POSIX ACL, it should arguably be checking the UID anyway.
AFS, for example, implements its own ACLs and evaluates them in
->permission() and on the server.
(*) may_follow_link()
Call vfs_inode_and_dir_have_same_owner() and vfs_is_owned_by_me() on
the the link and its parent dir.
(*) may_create_in_sticky()
Call vfs_is_owned_by_me() and also vfs_inode_and_dir_have_same_owner()
both.
[?] Should this return ok immediately if the open call we're in
created the file being checked.
(*) __check_sticky()
Call vfs_is_owned_by_me() on both the dir and the inode, but for AFS
vfs_is_owned_by_me() on a directory doesn't work, so call
vfs_inodes_have_same_owner() instead to check the directory (as is
done in may_create_in_sticky()).
(*) may_dedupe_file()
Call vfs_is_owned_by_me().
(*) IMA policy ops.
Unchanged for now. I'm not sure what the best way to deal with this
is - if, indeed, it needs any changes.
Note that wrapping stuff up into vfs_inode_is_owned_by_me() isn't
necessarily the most efficient as it means we may end up doing the uid
idmapping an extra time - though this is only done in three places, all to
do with world-writable sticky dir checks.
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Etienne Champetier <champetier.etienne@gmail.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Jeffrey Altman <jaltman@auristor.com>
cc: Chet Ramey <chet.ramey@case.edu>
cc: Cheyenne Wills <cwills@sinenomine.net>
cc: Alexander Viro <viro@zeniv.linux.org.uk>
cc: Christian Brauner <brauner@kernel.org>
cc: Steve French <sfrench@samba.org>
cc: Mimi Zohar <zohar@linux.ibm.com>
cc: linux-afs@lists.infradead.org
cc: openafs-devel@openafs.org
cc: linux-cifs@vger.kernel.org
cc: linux-fsdevel@vger.kernel.org
cc: linux-integrity@vger.kernel.org
Link: https://groups.google.com/g/gnu.bash.bug/c/6PPTfOgFdL4/m/2AQU-S1N76UJ
Link: https://git.savannah.gnu.org/cgit/bash.git/tree/redir.c?h=bash-5.3-rc1#n733
---
Documentation/filesystems/vfs.rst | 21 ++++
fs/attr.c | 58 ++++++-----
fs/coredump.c | 2 +-
fs/inode.c | 11 +-
fs/internal.h | 1 +
fs/locks.c | 7 +-
fs/namei.c | 161 ++++++++++++++++++++++++------
fs/remap_range.c | 20 ++--
include/linux/fs.h | 6 +-
9 files changed, 216 insertions(+), 71 deletions(-)
diff --git a/Documentation/filesystems/vfs.rst b/Documentation/filesystems/vfs.rst
index 4f13b01e42eb..5acbad3be4fd 100644
--- a/Documentation/filesystems/vfs.rst
+++ b/Documentation/filesystems/vfs.rst
@@ -495,6 +495,9 @@ As of kernel 2.6.22, the following members are defined:
struct dentry *dentry, struct file_kattr *fa);
int (*fileattr_get)(struct dentry *dentry, struct file_kattr *fa);
struct offset_ctx *(*get_offset_ctx)(struct inode *inode);
+ int (*is_owned_by_me)(struct mnt_idmap *idmap, struct inode *inode);
+ int (*have_same_owner)(struct mnt_idmap *idmap, struct inode *inode,
+ struct dentry *dentry);
};
Again, all methods are called without any locks being held, unless
@@ -679,6 +682,24 @@ otherwise noted.
filesystem must define this operation to use
simple_offset_dir_operations.
+``is_owned_by_me``
+ called to determine if the file can be considered to be 'owned' by
+ the owner of the process or if the process has a token that grants
+ it ownership privileges. If unset, the default is to compare i_uid
+ to current_fsuid() - but this may give incorrect results for some
+ network or plug-in block filesystems. For example, AFS determines
+ ownership entirely according to an obtained token and i_uid may not
+ even be from the same ID space as current_uid().
+
+``have_same_owner``
+ called to determine if an inode has the same owner as its immediate
+ parent on the path walked. If unset, the default is to simply
+ compare the i_uid of both. For example, AFS compares the owner IDs
+ of both - but these are a 64-bit values on some variants that might
+ not fit into a kuid_t and cifs has GUIDs that cannot be compared to
+ kuid_t.
+
+
The Address Space Object
========================
diff --git a/fs/attr.c b/fs/attr.c
index 795f231d00e8..096401a4815d 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -16,6 +16,7 @@
#include <linux/fcntl.h>
#include <linux/filelock.h>
#include <linux/security.h>
+#include "internal.h"
/**
* setattr_should_drop_sgid - determine whether the setgid bit needs to be
@@ -91,19 +92,21 @@ EXPORT_SYMBOL(setattr_should_drop_suidgid);
* permissions. On non-idmapped mounts or if permission checking is to be
* performed on the raw inode simply pass @nop_mnt_idmap.
*/
-static bool chown_ok(struct mnt_idmap *idmap,
- const struct inode *inode, vfsuid_t ia_vfsuid)
+static int chown_ok(struct mnt_idmap *idmap,
+ struct inode *inode, vfsuid_t ia_vfsuid)
{
vfsuid_t vfsuid = i_uid_into_vfsuid(idmap, inode);
- if (vfsuid_eq_kuid(vfsuid, current_fsuid()) &&
- vfsuid_eq(ia_vfsuid, vfsuid))
- return true;
+ int ret;
+
+ ret = vfs_inode_is_owned_by_me(idmap, inode);
+ if (ret <= 0)
+ return ret;
if (capable_wrt_inode_uidgid(idmap, inode, CAP_CHOWN))
- return true;
+ return 0;
if (!vfsuid_valid(vfsuid) &&
ns_capable(inode->i_sb->s_user_ns, CAP_CHOWN))
- return true;
- return false;
+ return 0;
+ return -EPERM;
}
/**
@@ -118,23 +121,27 @@ static bool chown_ok(struct mnt_idmap *idmap,
* permissions. On non-idmapped mounts or if permission checking is to be
* performed on the raw inode simply pass @nop_mnt_idmap.
*/
-static bool chgrp_ok(struct mnt_idmap *idmap,
- const struct inode *inode, vfsgid_t ia_vfsgid)
+static int chgrp_ok(struct mnt_idmap *idmap,
+ struct inode *inode, vfsgid_t ia_vfsgid)
{
vfsgid_t vfsgid = i_gid_into_vfsgid(idmap, inode);
- vfsuid_t vfsuid = i_uid_into_vfsuid(idmap, inode);
- if (vfsuid_eq_kuid(vfsuid, current_fsuid())) {
+ int ret;
+
+ ret = vfs_inode_is_owned_by_me(idmap, inode);
+ if (ret < 0)
+ return ret;
+ if (ret == 0) {
if (vfsgid_eq(ia_vfsgid, vfsgid))
- return true;
+ return 0;
if (vfsgid_in_group_p(ia_vfsgid))
- return true;
+ return 0;
}
if (capable_wrt_inode_uidgid(idmap, inode, CAP_CHOWN))
- return true;
+ return 0;
if (!vfsgid_valid(vfsgid) &&
ns_capable(inode->i_sb->s_user_ns, CAP_CHOWN))
- return true;
- return false;
+ return 0;
+ return -EPERM;
}
/**
@@ -163,6 +170,7 @@ int setattr_prepare(struct mnt_idmap *idmap, struct dentry *dentry,
{
struct inode *inode = d_inode(dentry);
unsigned int ia_valid = attr->ia_valid;
+ int ret;
/*
* First check size constraints. These can't be overriden using
@@ -179,14 +187,18 @@ int setattr_prepare(struct mnt_idmap *idmap, struct dentry *dentry,
goto kill_priv;
/* Make sure a caller can chown. */
- if ((ia_valid & ATTR_UID) &&
- !chown_ok(idmap, inode, attr->ia_vfsuid))
- return -EPERM;
+ if (ia_valid & ATTR_UID) {
+ ret = chown_ok(idmap, inode, attr->ia_vfsuid);
+ if (ret < 0)
+ return ret;
+ }
/* Make sure caller can chgrp. */
- if ((ia_valid & ATTR_GID) &&
- !chgrp_ok(idmap, inode, attr->ia_vfsgid))
- return -EPERM;
+ if (ia_valid & ATTR_GID) {
+ ret = chgrp_ok(idmap, inode, attr->ia_vfsgid);
+ if (ret < 0)
+ return ret;
+ }
/* Make sure a caller can chmod. */
if (ia_valid & ATTR_MODE) {
diff --git a/fs/coredump.c b/fs/coredump.c
index b5fc06a092a4..ac113e41d090 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -951,7 +951,7 @@ static bool coredump_file(struct core_name *cn, struct coredump_params *cprm,
* filesystem.
*/
idmap = file_mnt_idmap(file);
- if (!vfsuid_eq_kuid(i_uid_into_vfsuid(idmap, inode), current_fsuid())) {
+ if (vfs_inode_is_owned_by_me(idmap, inode) != 0) {
coredump_report_failure("Core dump to %s aborted: cannot preserve file owner", cn->corename);
return false;
}
diff --git a/fs/inode.c b/fs/inode.c
index ec9339024ac3..61e6b1d71e86 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2628,16 +2628,19 @@ EXPORT_SYMBOL(inode_init_owner);
* On non-idmapped mounts or if permission checking is to be performed on the
* raw inode simply pass @nop_mnt_idmap.
*/
-bool inode_owner_or_capable(struct mnt_idmap *idmap,
- const struct inode *inode)
+bool inode_owner_or_capable(struct mnt_idmap *idmap, struct inode *inode)
{
vfsuid_t vfsuid;
struct user_namespace *ns;
+ int ret;
- vfsuid = i_uid_into_vfsuid(idmap, inode);
- if (vfsuid_eq_kuid(vfsuid, current_fsuid()))
+ ret = vfs_inode_is_owned_by_me(idmap, inode);
+ if (ret == 0)
return true;
+ if (ret < 0)
+ return false;
+ vfsuid = i_uid_into_vfsuid(idmap, inode);
ns = current_user_ns();
if (vfsuid_has_mapping(ns, vfsuid) && ns_capable(ns, CAP_FOWNER))
return true;
diff --git a/fs/internal.h b/fs/internal.h
index 9b2b4d116880..29682c6edecd 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -52,6 +52,7 @@ extern int finish_clean_context(struct fs_context *fc);
/*
* namei.c
*/
+int vfs_inode_is_owned_by_me(struct mnt_idmap *idmap, struct inode *inode);
extern int filename_lookup(int dfd, struct filename *name, unsigned flags,
struct path *path, const struct path *root);
int do_rmdir(int dfd, struct filename *name);
diff --git a/fs/locks.c b/fs/locks.c
index 04a3f0e20724..b710bf0733b0 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -68,6 +68,7 @@
#include <trace/events/filelock.h>
#include <linux/uaccess.h>
+#include "internal.h"
static struct file_lock *file_lock(struct file_lock_core *flc)
{
@@ -2013,10 +2014,12 @@ int
vfs_setlease(struct file *filp, int arg, struct file_lease **lease, void **priv)
{
struct inode *inode = file_inode(filp);
- vfsuid_t vfsuid = i_uid_into_vfsuid(file_mnt_idmap(filp), inode);
int error;
- if ((!vfsuid_eq_kuid(vfsuid, current_fsuid())) && !capable(CAP_LEASE))
+ error = vfs_inode_is_owned_by_me(file_mnt_idmap(filp), inode);
+ if (error < 0)
+ return error;
+ if (error != 0 && !capable(CAP_LEASE))
return -EACCES;
if (!S_ISREG(inode->i_mode))
return -EINVAL;
diff --git a/fs/namei.c b/fs/namei.c
index 7377020a2cba..7dbcb5d50339 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -53,8 +53,8 @@
* The new code replaces the old recursive symlink resolution with
* an iterative one (in case of non-nested symlink chains). It does
* this with calls to <fs>_follow_link().
- * As a side effect, dir_namei(), _namei() and follow_link() are now
- * replaced with a single function lookup_dentry() that can handle all
+ * As a side effect, dir_namei(), _namei() and follow_link() are now
+ * replaced with a single function lookup_dentry() that can handle all
* the special cases of the former code.
*
* With the new dcache, the pathname is stored at each inode, at least as
@@ -1149,6 +1149,72 @@ fs_initcall(init_fs_namei_sysctls);
#endif /* CONFIG_SYSCTL */
+/*
+ * Determine if an inode is owned by the process (allowing for fsuid override),
+ * returning 0 if so, 1 if not and a negative error code if there was a problem
+ * making the determination.
+ */
+int vfs_inode_is_owned_by_me(struct mnt_idmap *idmap, struct inode *inode)
+{
+ if (unlikely(inode->i_op->is_owned_by_me))
+ return inode->i_op->is_owned_by_me(idmap, inode);
+ if (vfsuid_eq_kuid(i_uid_into_vfsuid(idmap, inode), current_fsuid()))
+ return 0;
+ return 1; /* Not same. */
+}
+
+/*
+ * Determine if an inode has the same owner as its parent, returning 0 if so, 1
+ * if not and a negative error code if there was a problem making the
+ * determination.
+ */
+static int vfs_inode_and_dir_have_same_owner(struct mnt_idmap *idmap, struct inode *inode,
+ const struct nameidata *nd)
+{
+ if (unlikely(inode->i_op->have_same_owner)) {
+ struct dentry *parent;
+ struct inode *dir;
+ int ret;
+
+ if (inode != nd->inode) {
+ dir = nd->inode;
+ ret = inode->i_op->have_same_owner(idmap, inode, dir);
+ } else if (nd->flags & LOOKUP_RCU) {
+ parent = READ_ONCE(nd->path.dentry);
+ dir = READ_ONCE(parent->d_inode);
+ if (!dir)
+ return -ECHILD;
+ ret = inode->i_op->have_same_owner(idmap, inode, dir);
+ } else {
+ parent = dget_parent(nd->path.dentry);
+ dir = parent->d_inode;
+ ret = inode->i_op->have_same_owner(idmap, inode, dir);
+ dput(parent);
+ }
+ return ret;
+ }
+
+ if (vfsuid_valid(nd->dir_vfsuid) &&
+ vfsuid_eq(i_uid_into_vfsuid(idmap, inode), nd->dir_vfsuid))
+ return 0;
+ return 1; /* Not same. */
+}
+
+/*
+ * Determine if two inodes have the same owner, returning 0 if so, 1 if not and
+ * a negative error code if there was a problem making the determination.
+ */
+static int vfs_inodes_have_same_owner(struct mnt_idmap *idmap, struct inode *inode,
+ struct inode *dir)
+{
+ if (unlikely(inode->i_op->have_same_owner))
+ return inode->i_op->have_same_owner(idmap, inode, dir);
+ if (vfsuid_eq(i_uid_into_vfsuid(idmap, inode),
+ i_uid_into_vfsuid(idmap, dir)))
+ return 0;
+ return 1; /* Not same. */
+}
+
/**
* may_follow_link - Check symlink following for unsafe situations
* @nd: nameidata pathwalk data
@@ -1165,27 +1231,28 @@ fs_initcall(init_fs_namei_sysctls);
*
* Returns 0 if following the symlink is allowed, -ve on error.
*/
-static inline int may_follow_link(struct nameidata *nd, const struct inode *inode)
+static inline int may_follow_link(struct nameidata *nd, struct inode *inode)
{
struct mnt_idmap *idmap;
- vfsuid_t vfsuid;
+ int ret;
if (!sysctl_protected_symlinks)
return 0;
- idmap = mnt_idmap(nd->path.mnt);
- vfsuid = i_uid_into_vfsuid(idmap, inode);
- /* Allowed if owner and follower match. */
- if (vfsuid_eq_kuid(vfsuid, current_fsuid()))
- return 0;
-
/* Allowed if parent directory not sticky and world-writable. */
if ((nd->dir_mode & (S_ISVTX|S_IWOTH)) != (S_ISVTX|S_IWOTH))
return 0;
+ idmap = mnt_idmap(nd->path.mnt);
+ /* Allowed if owner and follower match. */
+ ret = vfs_inode_is_owned_by_me(idmap, inode);
+ if (ret <= 0)
+ return ret;
+
/* Allowed if parent directory and link owner match. */
- if (vfsuid_valid(nd->dir_vfsuid) && vfsuid_eq(nd->dir_vfsuid, vfsuid))
- return 0;
+ ret = vfs_inode_and_dir_have_same_owner(idmap, inode, nd);
+ if (ret <= 0)
+ return ret;
if (nd->flags & LOOKUP_RCU)
return -ECHILD;
@@ -1283,12 +1350,12 @@ int may_linkat(struct mnt_idmap *idmap, const struct path *link)
* @inode: the inode of the file to open
*
* Block an O_CREAT open of a FIFO (or a regular file) when:
- * - sysctl_protected_fifos (or sysctl_protected_regular) is enabled
- * - the file already exists
- * - we are in a sticky directory
- * - we don't own the file
+ * - sysctl_protected_fifos (or sysctl_protected_regular) is enabled,
+ * - the file already exists,
+ * - we are in a sticky directory,
+ * - the directory is world writable,
+ * - we don't own the file and
* - the owner of the directory doesn't own the file
- * - the directory is world writable
* If the sysctl_protected_fifos (or sysctl_protected_regular) is set to 2
* the directory doesn't have to be world writable: being group writable will
* be enough.
@@ -1299,13 +1366,45 @@ int may_linkat(struct mnt_idmap *idmap, const struct path *link)
* On non-idmapped mounts or if permission checking is to be performed on the
* raw inode simply pass @nop_mnt_idmap.
*
+ * For a filesystem (e.g. a network filesystem) that has a separate ID space
+ * and has foreign IDs (maybe even non-integer IDs), i_uid cannot be compared
+ * to current_fsuid() and may not be directly comparable to another i_uid.
+ * Instead, the filesystem is asked to perform the comparisons. With network
+ * filesystems, there also exists the possibility of doing anonymous
+ * operations and having anonymously-owned objects.
+ *
+ * We have the following scenarios:
+ *
+ * USER DIR FILE FILE ALLOWED
+ * OWNER OWNER STATE
+ * ======= ======= ======= ======= =======
+ * A A - New Yes
+ * A A A Exists Yes
+ * A A C Exists No
+ * A B - New Yes
+ * A B A Exists Yes, FO==U
+ * A B B Exists Yes, FO==DO
+ * A B C Exists No
+ * A anon[1] - New Yes
+ * A anon[1] A Exists Yes
+ * A anon[1] C Exists No
+ * anon A - New Yes
+ * anon A A Exists Yes, FO==DO
+ * anon anon[1] - New Yes
+ * anon anon[1] - Exists No
+ * anon A A Exists Yes, FO==DO
+ * anon A C Exists No
+ * anon A anon Exists No
+ *
+ * [1] Can anonymously-owned dirs be sticky?
+ *
* Returns 0 if the open is allowed, -ve on error.
*/
static int may_create_in_sticky(struct mnt_idmap *idmap, struct nameidata *nd,
- struct inode *const inode)
+ struct inode *inode)
{
umode_t dir_mode = nd->dir_mode;
- vfsuid_t dir_vfsuid = nd->dir_vfsuid, i_vfsuid;
+ int ret;
if (likely(!(dir_mode & S_ISVTX)))
return 0;
@@ -1316,13 +1415,13 @@ static int may_create_in_sticky(struct mnt_idmap *idmap, struct nameidata *nd,
if (S_ISFIFO(inode->i_mode) && !sysctl_protected_fifos)
return 0;
- i_vfsuid = i_uid_into_vfsuid(idmap, inode);
-
- if (vfsuid_eq(i_vfsuid, dir_vfsuid))
- return 0;
+ ret = vfs_inode_and_dir_have_same_owner(idmap, inode, nd);
+ if (ret <= 0)
+ return ret;
- if (vfsuid_eq_kuid(i_vfsuid, current_fsuid()))
- return 0;
+ ret = vfs_inode_is_owned_by_me(idmap, inode);
+ if (ret <= 0)
+ return ret;
if (likely(dir_mode & 0002)) {
audit_log_path_denied(AUDIT_ANOM_CREAT, "sticky_create");
@@ -3222,12 +3321,14 @@ EXPORT_SYMBOL(user_path_at);
int __check_sticky(struct mnt_idmap *idmap, struct inode *dir,
struct inode *inode)
{
- kuid_t fsuid = current_fsuid();
+ int ret;
- if (vfsuid_eq_kuid(i_uid_into_vfsuid(idmap, inode), fsuid))
- return 0;
- if (vfsuid_eq_kuid(i_uid_into_vfsuid(idmap, dir), fsuid))
- return 0;
+ ret = vfs_inode_is_owned_by_me(idmap, inode);
+ if (ret <= 0)
+ return ret;
+ ret = vfs_inodes_have_same_owner(idmap, inode, dir);
+ if (ret <= 0)
+ return ret;
return !capable_wrt_inode_uidgid(idmap, inode, CAP_FOWNER);
}
EXPORT_SYMBOL(__check_sticky);
diff --git a/fs/remap_range.c b/fs/remap_range.c
index 26afbbbfb10c..9eee93c27001 100644
--- a/fs/remap_range.c
+++ b/fs/remap_range.c
@@ -413,20 +413,22 @@ loff_t vfs_clone_file_range(struct file *file_in, loff_t pos_in,
EXPORT_SYMBOL(vfs_clone_file_range);
/* Check whether we are allowed to dedupe the destination file */
-static bool may_dedupe_file(struct file *file)
+static int may_dedupe_file(struct file *file)
{
struct mnt_idmap *idmap = file_mnt_idmap(file);
struct inode *inode = file_inode(file);
+ int ret;
if (capable(CAP_SYS_ADMIN))
- return true;
+ return 0;
if (file->f_mode & FMODE_WRITE)
- return true;
- if (vfsuid_eq_kuid(i_uid_into_vfsuid(idmap, inode), current_fsuid()))
- return true;
+ return 0;
+ ret = vfs_inode_is_owned_by_me(idmap, inode);
+ if (ret <= 0)
+ return ret;
if (!inode_permission(idmap, inode, MAY_WRITE))
- return true;
- return false;
+ return 0;
+ return -EPERM;
}
loff_t vfs_dedupe_file_range_one(struct file *src_file, loff_t src_pos,
@@ -459,8 +461,8 @@ loff_t vfs_dedupe_file_range_one(struct file *src_file, loff_t src_pos,
if (ret)
return ret;
- ret = -EPERM;
- if (!may_dedupe_file(dst_file))
+ ret = may_dedupe_file(dst_file);
+ if (ret < 0)
goto out_drop_write;
ret = -EXDEV;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index c895146c1444..f59a7456852f 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2104,8 +2104,7 @@ static inline bool sb_start_intwrite_trylock(struct super_block *sb)
return __sb_start_write_trylock(sb, SB_FREEZE_FS);
}
-bool inode_owner_or_capable(struct mnt_idmap *idmap,
- const struct inode *inode);
+bool inode_owner_or_capable(struct mnt_idmap *idmap, struct inode *inode);
/*
* VFS helper functions..
@@ -2376,6 +2375,9 @@ struct inode_operations {
struct dentry *dentry, struct file_kattr *fa);
int (*fileattr_get)(struct dentry *dentry, struct file_kattr *fa);
struct offset_ctx *(*get_offset_ctx)(struct inode *inode);
+ int (*is_owned_by_me)(struct mnt_idmap *idmap, struct inode *inode);
+ int (*have_same_owner)(struct mnt_idmap *idmap, struct inode *inode1,
+ struct inode *inode2);
} ____cacheline_aligned;
/* Did the driver provide valid mmap hook configuration? */
^ permalink raw reply related
* Re: [PATCH] keys: Remove redundant less-than-zero checks
From: Mimi Zohar @ 2025-10-13 17:17 UTC (permalink / raw)
To: Thorsten Blum, David Howells, Jarkko Sakkinen, Paul Moore,
James Morris, Serge E. Hallyn, James Bottomley
Cc: keyrings, linux-security-module, linux-kernel, linux-integrity
In-Reply-To: <20251011144824.1257-2-thorsten.blum@linux.dev>
On Sat, 2025-10-11 at 16:48 +0200, Thorsten Blum wrote:
> The local variables 'size_t datalen' are unsigned and cannot be less
> than zero. Remove the redundant conditions.
>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH v3] keys: Replace deprecated strncpy in ecryptfs_fill_auth_tok
From: Paul Menzel @ 2025-10-13 16:11 UTC (permalink / raw)
To: Thorsten Blum
Cc: Mimi Zohar, David Howells, Jarkko Sakkinen, Paul Moore,
James Morris, Serge E. Hallyn, linux-hardening, Kees Cook,
linux-integrity, keyrings, linux-security-module, linux-kernel
In-Reply-To: <20251013152627.98231-2-thorsten.blum@linux.dev>
Dear Thorsten,
Thank you for the patch.
Am 13.10.25 um 17:26 schrieb Thorsten Blum:
> strncpy() is deprecated for NUL-terminated destination buffers; use
> strscpy_pad() instead to retain the NUL-padding behavior of strncpy().
>
> The destination buffer is initialized using kzalloc() with a 'signature'
> size of ECRYPTFS_PASSWORD_SIG_SIZE + 1. strncpy() then copies up to
> ECRYPTFS_PASSWORD_SIG_SIZE bytes from 'key_desc', NUL-padding any
> remaining bytes if needed, but expects the last byte to be zero.
>
> strscpy_pad() also copies the source string to 'signature', and NUL-pads
> the destination buffer if needed, but ensures it's always NUL-terminated
> without relying on it being zero-initialized.
>
> strscpy_pad() automatically determines the size of the fixed-length
> destination buffer via sizeof() when the optional size argument is
> omitted, making an explicit size unnecessary.
>
> In encrypted_init(), the source string 'key_desc' is validated by
> valid_ecryptfs_desc() before calling ecryptfs_fill_auth_tok(), and is
> therefore NUL-terminated and satisfies the __must_be_cstr() requirement
> of strscpy_pad().
>
> Link: https://github.com/KSPP/linux/issues/90
> Reviewed-by: Kees Cook <kees@kernel.org>
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
> Changes in v3:
> - Improve commit message
> - Link to v2: https://lore.kernel.org/lkml/20251010161340.458707-2-thorsten.blum@linux.dev/
>
> Changes in v2:
> - Improve commit message as suggested by Jarkko and Kees
> - Link to v1: https://lore.kernel.org/lkml/20251009180316.394708-3-thorsten.blum@linux.dev/
> ---
> security/keys/encrypted-keys/ecryptfs_format.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/security/keys/encrypted-keys/ecryptfs_format.c b/security/keys/encrypted-keys/ecryptfs_format.c
> index 8fdd76105ce3..2fc6f3a66135 100644
> --- a/security/keys/encrypted-keys/ecryptfs_format.c
> +++ b/security/keys/encrypted-keys/ecryptfs_format.c
> @@ -54,8 +54,7 @@ int ecryptfs_fill_auth_tok(struct ecryptfs_auth_tok *auth_tok,
> auth_tok->version = (((uint16_t)(major << 8) & 0xFF00)
> | ((uint16_t)minor & 0x00FF));
> auth_tok->token_type = ECRYPTFS_PASSWORD;
> - strncpy((char *)auth_tok->token.password.signature, key_desc,
> - ECRYPTFS_PASSWORD_SIG_SIZE);
> + strscpy_pad(auth_tok->token.password.signature, key_desc);
> auth_tok->token.password.session_key_encryption_key_bytes =
> ECRYPTFS_MAX_KEY_BYTES;
> /*
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Kind regards,
Paul
^ permalink raw reply
* [PATCH v3] keys: Replace deprecated strncpy in ecryptfs_fill_auth_tok
From: Thorsten Blum @ 2025-10-13 15:26 UTC (permalink / raw)
To: Mimi Zohar, David Howells, Jarkko Sakkinen, Paul Moore,
James Morris, Serge E. Hallyn
Cc: linux-hardening, Thorsten Blum, Kees Cook, linux-integrity,
keyrings, linux-security-module, linux-kernel
strncpy() is deprecated for NUL-terminated destination buffers; use
strscpy_pad() instead to retain the NUL-padding behavior of strncpy().
The destination buffer is initialized using kzalloc() with a 'signature'
size of ECRYPTFS_PASSWORD_SIG_SIZE + 1. strncpy() then copies up to
ECRYPTFS_PASSWORD_SIG_SIZE bytes from 'key_desc', NUL-padding any
remaining bytes if needed, but expects the last byte to be zero.
strscpy_pad() also copies the source string to 'signature', and NUL-pads
the destination buffer if needed, but ensures it's always NUL-terminated
without relying on it being zero-initialized.
strscpy_pad() automatically determines the size of the fixed-length
destination buffer via sizeof() when the optional size argument is
omitted, making an explicit size unnecessary.
In encrypted_init(), the source string 'key_desc' is validated by
valid_ecryptfs_desc() before calling ecryptfs_fill_auth_tok(), and is
therefore NUL-terminated and satisfies the __must_be_cstr() requirement
of strscpy_pad().
Link: https://github.com/KSPP/linux/issues/90
Reviewed-by: Kees Cook <kees@kernel.org>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
Changes in v3:
- Improve commit message
- Link to v2: https://lore.kernel.org/lkml/20251010161340.458707-2-thorsten.blum@linux.dev/
Changes in v2:
- Improve commit message as suggested by Jarkko and Kees
- Link to v1: https://lore.kernel.org/lkml/20251009180316.394708-3-thorsten.blum@linux.dev/
---
security/keys/encrypted-keys/ecryptfs_format.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/security/keys/encrypted-keys/ecryptfs_format.c b/security/keys/encrypted-keys/ecryptfs_format.c
index 8fdd76105ce3..2fc6f3a66135 100644
--- a/security/keys/encrypted-keys/ecryptfs_format.c
+++ b/security/keys/encrypted-keys/ecryptfs_format.c
@@ -54,8 +54,7 @@ int ecryptfs_fill_auth_tok(struct ecryptfs_auth_tok *auth_tok,
auth_tok->version = (((uint16_t)(major << 8) & 0xFF00)
| ((uint16_t)minor & 0x00FF));
auth_tok->token_type = ECRYPTFS_PASSWORD;
- strncpy((char *)auth_tok->token.password.signature, key_desc,
- ECRYPTFS_PASSWORD_SIG_SIZE);
+ strscpy_pad(auth_tok->token.password.signature, key_desc);
auth_tok->token.password.session_key_encryption_key_bytes =
ECRYPTFS_MAX_KEY_BYTES;
/*
--
2.51.0
^ permalink raw reply related
* [syzbot] Monthly integrity report (Oct 2025)
From: syzbot @ 2025-10-13 8:32 UTC (permalink / raw)
To: linux-integrity, linux-kernel, syzkaller-bugs
Hello integrity maintainers/developers,
This is a 31-day syzbot report for the integrity subsystem.
All related reports/information can be found at:
https://syzkaller.appspot.com/upstream/s/integrity
During the period, 0 new issues were detected and 0 were fixed.
In total, 3 issues are still open and 9 have already been fixed.
Some of the still happening issues:
Ref Crashes Repro Title
<1> 49 No possible deadlock in process_measurement (5)
https://syzkaller.appspot.com/bug?extid=6529afa25091aee8536c
<2> 44 Yes INFO: task hung in process_measurement (3)
https://syzkaller.appspot.com/bug?extid=cb9e66807bcb882cd0c5
<3> 14 Yes INFO: task hung in ima_file_free (4)
https://syzkaller.appspot.com/bug?extid=8036326eebe7d0140944
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
To disable reminders for individual bugs, reply with the following command:
#syz set <Ref> no-reminders
To change bug's subsystems, reply with:
#syz set <Ref> subsystems: new-subsystem
You may send multiple commands in a single email message.
^ permalink raw reply
* Re: [PATCH v2] keys: Replace deprecated strncpy in ecryptfs_fill_auth_tok
From: Jarkko Sakkinen @ 2025-10-13 7:22 UTC (permalink / raw)
To: Kees Cook
Cc: Thorsten Blum, Mimi Zohar, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, linux-hardening, linux-integrity,
keyrings, linux-security-module, linux-kernel
In-Reply-To: <202510101543.80A1D4E3@keescook>
On Fri, Oct 10, 2025 at 03:48:48PM -0700, Kees Cook wrote:
> On Fri, Oct 10, 2025 at 06:13:41PM +0200, Thorsten Blum wrote:
> > strncpy() is deprecated for NUL-terminated destination buffers; use
> > strscpy_pad() instead to retain the zero-padding behavior of strncpy().
> >
> > strscpy_pad() automatically determines the size of the fixed-length
> > destination buffer via sizeof() when the optional size argument is
> > omitted, making an explicit size unnecessary.
>
> I would explicitly say that the old code was NUL terminating the buffer
> due to it being "ECRYPTFS_PASSWORD_SIG_SIZE + 1" sized with strncpy
> left to fill ECRYPTFS_PASSWORD_SIG_SIZE. And then you have to answer the
> question, "how was this initialized?" and trace it back to:
>
> epayload = kzalloc(sizeof(*epayload) + payload_datalen +
> datablob_len + HASH_SIZE + 1, GFP_KERNEL);
>
> so the final byte was always being zeroed there, but now we're
> explicitly zeroing it (good). So there _is_ a functional change (we're
> writing 1 more byte here now), but it's more robust that way. There is
> no expected _logical_ change, though, yes.
Thanks for the remarks.
Thorsten, would you mind posting +1 with the commit message changes,
and reviewed-by tags (from me and Kees).
>
> >
> > In encrypted_init(), the source string 'key_desc' is validated by
> > valid_ecryptfs_desc() before calling ecryptfs_fill_auth_tok(), and is
> > therefore NUL-terminated and satisfies the __must_be_cstr() requirement
> > of strscpy_pad().
> >
> > No functional changes.
[just as reminder: removing this sentence was my earlier remark]
> >
> > Link: https://github.com/KSPP/linux/issues/90
> > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
>
> With "ECRYPTFS_PASSWORD_SIG_SIZE + 1" and tracing of the destination
> buffer initialization added to the commit log:
>
> Reviewed-by: Kees Cook <kees@kernel.org>
>
> -Kees
>
> --
> Kees Cook
BR, Jarkko
^ permalink raw reply
* [PATCH] keys: Remove redundant less-than-zero checks
From: Thorsten Blum @ 2025-10-11 14:48 UTC (permalink / raw)
To: David Howells, Jarkko Sakkinen, Paul Moore, James Morris,
Serge E. Hallyn, Mimi Zohar, James Bottomley
Cc: Thorsten Blum, keyrings, linux-security-module, linux-kernel,
linux-integrity
The local variables 'size_t datalen' are unsigned and cannot be less
than zero. Remove the redundant conditions.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
security/keys/big_key.c | 2 +-
security/keys/encrypted-keys/encrypted.c | 4 ++--
security/keys/trusted-keys/trusted_core.c | 4 ++--
security/keys/user_defined.c | 2 +-
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/security/keys/big_key.c b/security/keys/big_key.c
index c3367622c683..d46862ab90d6 100644
--- a/security/keys/big_key.c
+++ b/security/keys/big_key.c
@@ -66,7 +66,7 @@ int big_key_preparse(struct key_preparsed_payload *prep)
BUILD_BUG_ON(sizeof(*payload) != sizeof(prep->payload.data));
- if (datalen <= 0 || datalen > 1024 * 1024 || !prep->data)
+ if (datalen == 0 || datalen > 1024 * 1024 || !prep->data)
return -EINVAL;
/* Set an arbitrary quota */
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index d70f71d37f5f..57f88ae000ba 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -786,7 +786,7 @@ static int encrypted_instantiate(struct key *key,
size_t datalen = prep->datalen;
int ret;
- if (datalen <= 0 || datalen > 32767 || !prep->data)
+ if (datalen == 0 || datalen > 32767 || !prep->data)
return -EINVAL;
datablob = kmalloc(datalen + 1, GFP_KERNEL);
@@ -847,7 +847,7 @@ static int encrypted_update(struct key *key, struct key_preparsed_payload *prep)
if (key_is_negative(key))
return -ENOKEY;
- if (datalen <= 0 || datalen > 32767 || !prep->data)
+ if (datalen == 0 || datalen > 32767 || !prep->data)
return -EINVAL;
buf = kmalloc(datalen + 1, GFP_KERNEL);
diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
index e2d9644efde1..b1680ee53f86 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -157,7 +157,7 @@ static int trusted_instantiate(struct key *key,
int key_cmd;
size_t key_len;
- if (datalen <= 0 || datalen > 32767 || !prep->data)
+ if (datalen == 0 || datalen > 32767 || !prep->data)
return -EINVAL;
orig_datablob = datablob = kmalloc(datalen + 1, GFP_KERNEL);
@@ -240,7 +240,7 @@ static int trusted_update(struct key *key, struct key_preparsed_payload *prep)
p = key->payload.data[0];
if (!p->migratable)
return -EPERM;
- if (datalen <= 0 || datalen > 32767 || !prep->data)
+ if (datalen == 0 || datalen > 32767 || !prep->data)
return -EINVAL;
orig_datablob = datablob = kmalloc(datalen + 1, GFP_KERNEL);
diff --git a/security/keys/user_defined.c b/security/keys/user_defined.c
index 749e2a4dcb13..686d56e4cc85 100644
--- a/security/keys/user_defined.c
+++ b/security/keys/user_defined.c
@@ -61,7 +61,7 @@ int user_preparse(struct key_preparsed_payload *prep)
struct user_key_payload *upayload;
size_t datalen = prep->datalen;
- if (datalen <= 0 || datalen > 32767 || !prep->data)
+ if (datalen == 0 || datalen > 32767 || !prep->data)
return -EINVAL;
upayload = kmalloc(sizeof(*upayload) + datalen, GFP_KERNEL);
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v2] keys: Replace deprecated strncpy in ecryptfs_fill_auth_tok
From: Kees Cook @ 2025-10-10 22:48 UTC (permalink / raw)
To: Thorsten Blum
Cc: Mimi Zohar, David Howells, Jarkko Sakkinen, Paul Moore,
James Morris, Serge E. Hallyn, linux-hardening, linux-integrity,
keyrings, linux-security-module, linux-kernel
In-Reply-To: <20251010161340.458707-2-thorsten.blum@linux.dev>
On Fri, Oct 10, 2025 at 06:13:41PM +0200, Thorsten Blum wrote:
> strncpy() is deprecated for NUL-terminated destination buffers; use
> strscpy_pad() instead to retain the zero-padding behavior of strncpy().
>
> strscpy_pad() automatically determines the size of the fixed-length
> destination buffer via sizeof() when the optional size argument is
> omitted, making an explicit size unnecessary.
I would explicitly say that the old code was NUL terminating the buffer
due to it being "ECRYPTFS_PASSWORD_SIG_SIZE + 1" sized with strncpy
left to fill ECRYPTFS_PASSWORD_SIG_SIZE. And then you have to answer the
question, "how was this initialized?" and trace it back to:
epayload = kzalloc(sizeof(*epayload) + payload_datalen +
datablob_len + HASH_SIZE + 1, GFP_KERNEL);
so the final byte was always being zeroed there, but now we're
explicitly zeroing it (good). So there _is_ a functional change (we're
writing 1 more byte here now), but it's more robust that way. There is
no expected _logical_ change, though, yes.
>
> In encrypted_init(), the source string 'key_desc' is validated by
> valid_ecryptfs_desc() before calling ecryptfs_fill_auth_tok(), and is
> therefore NUL-terminated and satisfies the __must_be_cstr() requirement
> of strscpy_pad().
>
> No functional changes.
>
> Link: https://github.com/KSPP/linux/issues/90
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
With "ECRYPTFS_PASSWORD_SIG_SIZE + 1" and tracing of the destination
buffer initialization added to the commit log:
Reviewed-by: Kees Cook <kees@kernel.org>
-Kees
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v4 31/34] ima,evm: move initcalls to the LSM framework
From: Paul Moore @ 2025-10-10 19:21 UTC (permalink / raw)
To: Mimi Zohar
Cc: selinux, linux-integrity, linux-security-module, John Johansen,
Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <282070d5c0dd68140ae221833ea8c5ba4baada4f.camel@linux.ibm.com>
On Fri, Oct 10, 2025 at 12:54 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Tue, 2025-09-30 at 16:11 -0400, Paul Moore wrote:
> > On Tue, Sep 16, 2025 at 6:14 PM Paul Moore <paul@paul-moore.com> wrote:
> > >
> > > From: Roberto Sassu <roberto.sassu@huawei.com>
> > >
> > > This patch converts IMA and EVM to use the LSM frameworks's initcall
> > > mechanism. It moved the integrity_fs_init() call to ima_fs_init() and
> > > evm_init_secfs(), to work around the fact that there is no "integrity" LSM,
> > > and introduced integrity_fs_fini() to remove the integrity directory, if
> > > empty. Both integrity_fs_init() and integrity_fs_fini() support the
> > > scenario of being called by both the IMA and EVM LSMs.
> > >
> > > This patch does not touch any of the platform certificate code that
> > > lives under the security/integrity/platform_certs directory as the
> > > IMA/EVM developers would prefer to address that in a future patchset.
> > >
> > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > > [PM: adjust description as discussed over email]
> > > Signed-off-by: Paul Moore <paul@paul-moore.com>
> > > ---
> > > security/integrity/evm/evm_main.c | 3 +--
> > > security/integrity/evm/evm_secfs.c | 11 +++++++++--
> > > security/integrity/iint.c | 14 ++++++++++++--
> > > security/integrity/ima/ima_fs.c | 11 +++++++++--
> > > security/integrity/ima/ima_main.c | 4 ++--
> > > security/integrity/integrity.h | 2 ++
> > > 6 files changed, 35 insertions(+), 10 deletions(-)
> >
> > I appreciate you reviewing most (all?) of the other patches in this
> > patchset, but any chance you could review the IMA/EVM from Roberto?
> > This is the only patch that really needs your review ...
>
> Paul, I'm sorry for the long delay in reviewing and testing this patch set. It
> wasn't enough to just review this one patch, but it needed to be reviewed in
> context.
>
> The initcall ordering is extremely important for IMA. IMA-measurement needs to
> be initialized after the TPM, otherwise IMA goes into TPM-bypass mode. As
> expected, the initcall ordering seems to be fine. However this patch set
> modifies the initcall debugging.
>
> The kernel boot command line option "initcall_debug" outputs "entering initcall
> level:" messages for each of the initcall levels, and "calling ...." and
> "initcall ..." messages for the individual initcalls.
>
> For example,
> [ 0.896556] entering initcall level: arch
> [ 0.896556] calling report_snp_info+0x0/0xd0 @ 1
> [ 0.896556] initcall report_snp_info+0x0/0xd0 returned 0 after 0 usecs
>
> With this patch set, the "calling ..." and "initcall ..." messages will not be
> emitted for the LSMs. In lieu of these messages, the patch set defines a new
> boot command line option "lsm.debug" ...
This is not a new kernel command line option, oddly enough Kees wrote
the patch exactly seven years ago today and we first saw it released
in Linux v4.20.
> which outputs "LSM: entering ....
> initcall".
>
> For example,
> [ 2.225821] calling security_initcall_late+0x0/0xc0 @ 1
> [ 2.225825] LSM: running ima late initcall
>
> Regardless as to whether the performance information is actually necessary, the
> initcall debugging change should probably be documented. Maybe update
> initcall_debug to reference lsm.debug in Documentation/admin-guide/kernel-
> parameters.txt.
Thank you for the input, but I would just prefer to leave it as-is.
--
paul-moore.com
^ permalink raw reply
* Re: [GIT PULL] TPM DEVICE DRIVER: tpmdd-next-v6.18-2
From: pr-tracker-bot @ 2025-10-10 18:25 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Linus Torvalds, Peter Huewe, Jason Gunthorpe, David Howells,
keyrings, linux-integrity, linux-kernel
In-Reply-To: <aOibAOKu_lEsSlC8@kernel.org>
The pull request you sent on Fri, 10 Oct 2025 08:34:56 +0300:
> git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git tags/tpmdd-next-v6.18-2
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/84d4e8b613e073d9dfde782c471aedbcefdede6c
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [PATCH v2] keys: Replace deprecated strncpy in ecryptfs_fill_auth_tok
From: Jarkko Sakkinen @ 2025-10-10 17:44 UTC (permalink / raw)
To: Thorsten Blum
Cc: Kees Cook, Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, linux-hardening, linux-integrity, keyrings,
linux-security-module, linux-kernel
In-Reply-To: <20251010161340.458707-2-thorsten.blum@linux.dev>
On Fri, Oct 10, 2025 at 06:13:41PM +0200, Thorsten Blum wrote:
> strncpy() is deprecated for NUL-terminated destination buffers; use
> strscpy_pad() instead to retain the zero-padding behavior of strncpy().
>
> strscpy_pad() automatically determines the size of the fixed-length
> destination buffer via sizeof() when the optional size argument is
> omitted, making an explicit size unnecessary.
>
> In encrypted_init(), the source string 'key_desc' is validated by
> valid_ecryptfs_desc() before calling ecryptfs_fill_auth_tok(), and is
> therefore NUL-terminated and satisfies the __must_be_cstr() requirement
> of strscpy_pad().
>
> No functional changes.
It's a functional change (for better!) because it transforms to safer
semantics ;-) And yeah as years pass by commit messages like these
have more value than code changes themselves (as far backtracking
and bisecting is concerned).
So if you don't mind, I'll delete the very last one sentence paragraph,
and with that
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Thank you.
>
> Link: https://github.com/KSPP/linux/issues/90
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
> Changes in v2:
> - Improve commit message as suggested by Jarkko and Kees
> - Link to v1: https://lore.kernel.org/lkml/20251009180316.394708-3-thorsten.blum@linux.dev/
> ---
> security/keys/encrypted-keys/ecryptfs_format.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/security/keys/encrypted-keys/ecryptfs_format.c b/security/keys/encrypted-keys/ecryptfs_format.c
> index 8fdd76105ce3..2fc6f3a66135 100644
> --- a/security/keys/encrypted-keys/ecryptfs_format.c
> +++ b/security/keys/encrypted-keys/ecryptfs_format.c
> @@ -54,8 +54,7 @@ int ecryptfs_fill_auth_tok(struct ecryptfs_auth_tok *auth_tok,
> auth_tok->version = (((uint16_t)(major << 8) & 0xFF00)
> | ((uint16_t)minor & 0x00FF));
> auth_tok->token_type = ECRYPTFS_PASSWORD;
> - strncpy((char *)auth_tok->token.password.signature, key_desc,
> - ECRYPTFS_PASSWORD_SIG_SIZE);
> + strscpy_pad(auth_tok->token.password.signature, key_desc);
> auth_tok->token.password.session_key_encryption_key_bytes =
> ECRYPTFS_MAX_KEY_BYTES;
> /*
> --
> 2.51.0
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v2] tpm: infineon: add bounds check in tpm_inf_recv
From: Jarkko Sakkinen @ 2025-10-10 17:40 UTC (permalink / raw)
To: Shahriyar Jalayeri; +Cc: peterhuewe, jgg, linux-integrity, linux-kernel
In-Reply-To: <20251010065252.4377-1-shahriyar@posteo.de>
On Fri, Oct 10, 2025 at 06:52:55AM +0000, Shahriyar Jalayeri wrote:
> Add two buffer size validations to prevent buffer overflows in
> tpm_inf_recv():
>
> 1. Validate that the provided buffer can hold at least the 4-byte header
> before attempting to read it.
> 2. Validate that the buffer is large enough to hold the data size reported
> by the TPM before reading the payload.
>
> Without these checks, a malicious or malfunctioning TPM could cause buffer
> overflows by reporting data sizes larger than the provided buffer, leading
> to memory corruption.
>
> Fixes: ebb81fdb3dd0 ("[PATCH] tpm: Support for Infineon TPM")
> Signed-off-by: Shahriyar Jalayeri <shahriyar@posteo.de>
> ---
> drivers/char/tpm/tpm_infineon.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
> index 7638b65b8..0fe4193a3 100644
> --- a/drivers/char/tpm/tpm_infineon.c
> +++ b/drivers/char/tpm/tpm_infineon.c
> @@ -250,6 +250,10 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count)
> number_of_wtx = 0;
>
> recv_begin:
> + /* expect at least 1-byte VL header, 1-byte ctrl-tag, 2-byte data size */
This is definitely good enough :-)
But is that comment misaligned? Does VL come from "VLAN"?
> + if (count < 4)
> + return -EIO;
> +
> /* start receiving header */
> for (i = 0; i < 4; i++) {
> ret = wait(chip, STAT_RDA);
> @@ -268,6 +272,9 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count)
> /* size of the data received */
I'd delete the above comment.
> size = ((buf[2] << 8) | buf[3]);
And use here:
https://elixir.bootlin.com/linux/v6.17.1/source/include/linux/byteorder/generic.h#L108
Not exactly in scope but it would be good convention and make
the check after it more readable.
>
> + if (size + 6 > count)
> + return -EIO;
> +
> for (i = 0; i < size; i++) {
> wait(chip, STAT_RDA);
> buf[i] = tpm_data_in(RDFIFO);
> --
> 2.43.0
>
BR, Jarkko
^ permalink raw reply
* Re: [GIT PULL] TPM DEVICE DRIVER: tpmdd-next-v6.18-2
From: Jarkko Sakkinen @ 2025-10-10 17:26 UTC (permalink / raw)
To: Linus Torvalds
Cc: Peter Huewe, Jason Gunthorpe, David Howells, keyrings,
linux-integrity, linux-kernel
In-Reply-To: <CAHk-=whUUZpENHKMrrVQwqfBgP9Lm=SxW+a3WmoxZR3JObdrUA@mail.gmail.com>
On Fri, Oct 10, 2025 at 08:51:09AM -0700, Linus Torvalds wrote:
> On Thu, 9 Oct 2025 at 22:35, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git tags/tpmdd-next-v6.18-2
>
> So I've pulled this, but I'm still unhappy about the explanation.
>
> You tried to explain a one-line single-character change in that pull
> request, and even in that explanation you spent most effort on
> dismissing other peoples concerns.
For what it is, most of it comes from:
1. "tpm: use a map for tpm2_calc_ordinal_duration()"
Flattened out timeout calculations to a table and increase timeout
for TPM2_SelfTest, which addresses longer timeout on Raspeberry Pi.
2. "tpm: Prevent local DOS via tpm/tpm0/ppi/*operations"
Caches TPM physical presence interface ACPI functions on first run
instead of requesting for every read.
Also:
1. I went through Chris' email because you asked to refer to it.
2. I also spent time re-testing O_EXCL change throughly once more. From
my subjective perspective I was exactly trying to address other people's
concerns.
That said, I fell off the track and yeah not well delivered agreed.
> That one-liner would have been - and is - sufficiently explained by
> "it performs badly and breaks some configurations". There's absolutely
> no reason to then go on to describe how *you* don't care about those
> configurations.
Maybe I had a bad choice of words but there's no configuration that
breaks with anything sold as discrete TPM chips, embedded SoC, fTPM's
or anything really. I got the impression of a bug in the wild, other
than the perf regression.
>
> But lookie here:
>
> 8 files changed, 137 insertions(+), 199 deletions(-)
>
> that's the actual meat of the pull request, and it gets not a peep of
> commentary.
>
> I'd also like to point out that Microsoft spent *years* trying to do
> the "we require certain typical TPM setups", and people complained
> about their idiocy.
>
> For all I know, they still push that crap.
>
> I would certainly are *NOT* that stupid, and we are not going down that path.
>
> So when it comes to TPM, the rule is not "typical cases work".
>
> The rule is "if it causes problems, we acknowledge them and we avoid them".
I deeply care anything that can be bought with money or even anything
that drifts away from a spec manageable amount.
> Thus the whole "disable TCG_TPM2_HMAC" really doesn't merit this kind
> of long explanation.
>
> In contrast, the *other* changes are probably much more interesting than that.
Very true :-)
> Linus
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v4 31/34] ima,evm: move initcalls to the LSM framework
From: Mimi Zohar @ 2025-10-10 16:53 UTC (permalink / raw)
To: Paul Moore
Cc: selinux, linux-integrity, linux-security-module, John Johansen,
Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <CAHC9VhQCmFJQ1=Eyu1D+Mcg2FVDByrk8QcwV5HaZdB95esiA7Q@mail.gmail.com>
On Tue, 2025-09-30 at 16:11 -0400, Paul Moore wrote:
> On Tue, Sep 16, 2025 at 6:14 PM Paul Moore <paul@paul-moore.com> wrote:
> >
> > From: Roberto Sassu <roberto.sassu@huawei.com>
> >
> > This patch converts IMA and EVM to use the LSM frameworks's initcall
> > mechanism. It moved the integrity_fs_init() call to ima_fs_init() and
> > evm_init_secfs(), to work around the fact that there is no "integrity" LSM,
> > and introduced integrity_fs_fini() to remove the integrity directory, if
> > empty. Both integrity_fs_init() and integrity_fs_fini() support the
> > scenario of being called by both the IMA and EVM LSMs.
> >
> > This patch does not touch any of the platform certificate code that
> > lives under the security/integrity/platform_certs directory as the
> > IMA/EVM developers would prefer to address that in a future patchset.
> >
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > [PM: adjust description as discussed over email]
> > Signed-off-by: Paul Moore <paul@paul-moore.com>
> > ---
> > security/integrity/evm/evm_main.c | 3 +--
> > security/integrity/evm/evm_secfs.c | 11 +++++++++--
> > security/integrity/iint.c | 14 ++++++++++++--
> > security/integrity/ima/ima_fs.c | 11 +++++++++--
> > security/integrity/ima/ima_main.c | 4 ++--
> > security/integrity/integrity.h | 2 ++
> > 6 files changed, 35 insertions(+), 10 deletions(-)
>
> I appreciate you reviewing most (all?) of the other patches in this
> patchset, but any chance you could review the IMA/EVM from Roberto?
> This is the only patch that really needs your review ...
Paul, I'm sorry for the long delay in reviewing and testing this patch set. It
wasn't enough to just review this one patch, but it needed to be reviewed in
context.
The initcall ordering is extremely important for IMA. IMA-measurement needs to
be initialized after the TPM, otherwise IMA goes into TPM-bypass mode. As
expected, the initcall ordering seems to be fine. However this patch set
modifies the initcall debugging.
The kernel boot command line option "initcall_debug" outputs "entering initcall
level:" messages for each of the initcall levels, and "calling ...." and
"initcall ..." messages for the individual initcalls.
For example,
[ 0.896556] entering initcall level: arch
[ 0.896556] calling report_snp_info+0x0/0xd0 @ 1
[ 0.896556] initcall report_snp_info+0x0/0xd0 returned 0 after 0 usecs
With this patch set, the "calling ..." and "initcall ..." messages will not be
emitted for the LSMs. In lieu of these messages, the patch set defines a new
boot command line option "lsm.debug", which outputs "LSM: entering ....
initcall".
For example,
[ 2.225821] calling security_initcall_late+0x0/0xc0 @ 1
[ 2.225825] LSM: running ima late initcall
Regardless as to whether the performance information is actually necessary, the
initcall debugging change should probably be documented. Maybe update
initcall_debug to reference lsm.debug in Documentation/admin-guide/kernel-
parameters.txt.
Mimi
^ permalink raw reply
* [PATCH v2] keys: Replace deprecated strncpy in ecryptfs_fill_auth_tok
From: Thorsten Blum @ 2025-10-10 16:13 UTC (permalink / raw)
To: Kees Cook, Mimi Zohar, David Howells, Jarkko Sakkinen, Paul Moore,
James Morris, Serge E. Hallyn
Cc: linux-hardening, Thorsten Blum, linux-integrity, keyrings,
linux-security-module, linux-kernel
strncpy() is deprecated for NUL-terminated destination buffers; use
strscpy_pad() instead to retain the zero-padding behavior of strncpy().
strscpy_pad() automatically determines the size of the fixed-length
destination buffer via sizeof() when the optional size argument is
omitted, making an explicit size unnecessary.
In encrypted_init(), the source string 'key_desc' is validated by
valid_ecryptfs_desc() before calling ecryptfs_fill_auth_tok(), and is
therefore NUL-terminated and satisfies the __must_be_cstr() requirement
of strscpy_pad().
No functional changes.
Link: https://github.com/KSPP/linux/issues/90
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
Changes in v2:
- Improve commit message as suggested by Jarkko and Kees
- Link to v1: https://lore.kernel.org/lkml/20251009180316.394708-3-thorsten.blum@linux.dev/
---
security/keys/encrypted-keys/ecryptfs_format.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/security/keys/encrypted-keys/ecryptfs_format.c b/security/keys/encrypted-keys/ecryptfs_format.c
index 8fdd76105ce3..2fc6f3a66135 100644
--- a/security/keys/encrypted-keys/ecryptfs_format.c
+++ b/security/keys/encrypted-keys/ecryptfs_format.c
@@ -54,8 +54,7 @@ int ecryptfs_fill_auth_tok(struct ecryptfs_auth_tok *auth_tok,
auth_tok->version = (((uint16_t)(major << 8) & 0xFF00)
| ((uint16_t)minor & 0x00FF));
auth_tok->token_type = ECRYPTFS_PASSWORD;
- strncpy((char *)auth_tok->token.password.signature, key_desc,
- ECRYPTFS_PASSWORD_SIG_SIZE);
+ strscpy_pad(auth_tok->token.password.signature, key_desc);
auth_tok->token.password.session_key_encryption_key_bytes =
ECRYPTFS_MAX_KEY_BYTES;
/*
--
2.51.0
^ permalink raw reply related
* Re: [GIT PULL] TPM DEVICE DRIVER: tpmdd-next-v6.18-2
From: Linus Torvalds @ 2025-10-10 15:51 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Peter Huewe, Jason Gunthorpe, David Howells, keyrings,
linux-integrity, linux-kernel
In-Reply-To: <aOibAOKu_lEsSlC8@kernel.org>
On Thu, 9 Oct 2025 at 22:35, Jarkko Sakkinen <jarkko@kernel.org> wrote:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git tags/tpmdd-next-v6.18-2
So I've pulled this, but I'm still unhappy about the explanation.
You tried to explain a one-line single-character change in that pull
request, and even in that explanation you spent most effort on
dismissing other peoples concerns.
That one-liner would have been - and is - sufficiently explained by
"it performs badly and breaks some configurations". There's absolutely
no reason to then go on to describe how *you* don't care about those
configurations.
But lookie here:
8 files changed, 137 insertions(+), 199 deletions(-)
that's the actual meat of the pull request, and it gets not a peep of
commentary.
I'd also like to point out that Microsoft spent *years* trying to do
the "we require certain typical TPM setups", and people complained
about their idiocy.
For all I know, they still push that crap.
I would certainly are *NOT* that stupid, and we are not going down that path.
So when it comes to TPM, the rule is not "typical cases work".
The rule is "if it causes problems, we acknowledge them and we avoid them".
Thus the whole "disable TCG_TPM2_HMAC" really doesn't merit this kind
of long explanation.
In contrast, the *other* changes are probably much more interesting than that.
Linus
^ permalink raw reply
* Re: [PATCH] keys: Replace deprecated strncpy in ecryptfs_fill_auth_tok
From: Kees Cook @ 2025-10-10 15:12 UTC (permalink / raw)
To: Thorsten Blum
Cc: Mimi Zohar, David Howells, Jarkko Sakkinen, Paul Moore,
James Morris, Serge E. Hallyn, linux-hardening, linux-integrity,
keyrings, linux-security-module, linux-kernel
In-Reply-To: <20251009180316.394708-3-thorsten.blum@linux.dev>
On Thu, Oct 09, 2025 at 08:03:17PM +0200, Thorsten Blum wrote:
> strncpy() is deprecated for NUL-terminated destination buffers; use
> strscpy_pad() instead.
Remember for strncpy->strscpy conversions, the commit message needs
to include:
- how did you determine this was a NUL-terminated destination?
- how did you determine the need for padding or not?
- how do you know that final byte truncation is not a problem?
-Kees
>
> Link: https://github.com/KSPP/linux/issues/90
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
> security/keys/encrypted-keys/ecryptfs_format.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/security/keys/encrypted-keys/ecryptfs_format.c b/security/keys/encrypted-keys/ecryptfs_format.c
> index 8fdd76105ce3..2fc6f3a66135 100644
> --- a/security/keys/encrypted-keys/ecryptfs_format.c
> +++ b/security/keys/encrypted-keys/ecryptfs_format.c
> @@ -54,8 +54,7 @@ int ecryptfs_fill_auth_tok(struct ecryptfs_auth_tok *auth_tok,
> auth_tok->version = (((uint16_t)(major << 8) & 0xFF00)
> | ((uint16_t)minor & 0x00FF));
> auth_tok->token_type = ECRYPTFS_PASSWORD;
> - strncpy((char *)auth_tok->token.password.signature, key_desc,
> - ECRYPTFS_PASSWORD_SIG_SIZE);
> + strscpy_pad(auth_tok->token.password.signature, key_desc);
> auth_tok->token.password.session_key_encryption_key_bytes =
> ECRYPTFS_MAX_KEY_BYTES;
> /*
> --
> 2.51.0
>
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v4 31/34] ima,evm: move initcalls to the LSM framework
From: Mimi Zohar @ 2025-10-10 10:19 UTC (permalink / raw)
To: Paul Moore, linux-security-module, linux-integrity, selinux
Cc: John Johansen, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <20250916220355.252592-67-paul@paul-moore.com>
On Tue, 2025-09-16 at 18:03 -0400, Paul Moore wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> This patch converts IMA and EVM to use the LSM frameworks's initcall
> mechanism. It moved the integrity_fs_init() call to ima_fs_init() and
> evm_init_secfs(), to work around the fact that there is no "integrity" LSM,
> and introduced integrity_fs_fini() to remove the integrity directory, if
> empty. Both integrity_fs_init() and integrity_fs_fini() support the
> scenario of being called by both the IMA and EVM LSMs.
>
> This patch does not touch any of the platform certificate code that
> lives under the security/integrity/platform_certs directory as the
> IMA/EVM developers would prefer to address that in a future patchset.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> [PM: adjust description as discussed over email]
> Signed-off-by: Paul Moore <paul@paul-moore.com>
Acked-by: Mimi Zohar <zohar@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH v5] tpm: infineon: add bounds check in tpm_inf_recv
From: Shahriyar @ 2025-10-10 8:05 UTC (permalink / raw)
To: Paul Menzel; +Cc: jarkko, peterhuewe, jgg, linux-integrity, linux-kernel
In-Reply-To: <b1fbadb2-235e-4b08-afba-e397c6d076df@molgen.mpg.de>
On 10/10/25 10:02 AM, Paul Menzel wrote:
> Dear Shahriyar,
>
>
> Thank you for the improved version.
>
> Am 10.10.25 um 09:49 schrieb Shahriyar Jalayeri:
>> Add two buffer size validations to prevent buffer overflows in
>> tpm_inf_recv():
>>
>> 1. Validate that the provided buffer can hold at least the 4-byte header
>> before attempting to read it.
>> 2. Validate that the buffer is large enough to hold the data size
>> reported
>> by the TPM before reading the payload.
>>
>> Without these checks, a malicious or malfunctioning TPM could cause
>> buffer
>> overflows by reporting data sizes larger than the provided buffer,
>> leading
>> to memory corruption.
>>
>> Fixes: ebb81fdb3dd0 ("[PATCH] tpm: Support for Infineon TPM")
>> Signed-off-by: Shahriyar Jalayeri <shahriyar@posteo.de>
>> ---
>> Changelog:
>> v5:
>> - replaced space indentation with tabs before the header comment
>> v4:
>> - removed the curly braces around one line statements
>> v3:
>> - removed dev_err() logs
>> - added missing "fixes" tag
>> - described header structure
>> - fixed commit message length
>> - removed use of local variable as constant
>> - fixed check to account for off-by-six bytes
>> v2:
>> - added complete changes in the commit message
>> - dev_err() logged expected sizes and stated operation is aborted
>>
>> drivers/char/tpm/tpm_infineon.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/
>> tpm_infineon.c
>> index 7638b65b8..d76aae08b 100644
>> --- a/drivers/char/tpm/tpm_infineon.c
>> +++ b/drivers/char/tpm/tpm_infineon.c
>> @@ -250,6 +250,10 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8
>> * buf, size_t count)
>> number_of_wtx = 0;
>> recv_begin:
>> + /* expect at least 1-byte VL header, 1-byte ctrl-tag, 2-byte data
>> size */
>> + if (count < 4)
>> + return -EIO;
>> +
>> /* start receiving header */
>> for (i = 0; i < 4; i++) {
>> ret = wait(chip, STAT_RDA);
>> @@ -268,6 +272,9 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8
>> * buf, size_t count)
>> /* size of the data received */
>> size = ((buf[2] << 8) | buf[3]);
>> + if (size + 6 > count)
>> + return -EIO;
>> +
>> for (i = 0; i < size; i++) {
>> wait(chip, STAT_RDA);
>> buf[i] = tpm_data_in(RDFIFO);
>
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
>
>
> Kind regards,
>
> Paul
Thank you for your patience with newcomers.
--
BR.
/shj
^ permalink raw reply
* Re: [PATCH v5] tpm: infineon: add bounds check in tpm_inf_recv
From: Paul Menzel @ 2025-10-10 8:02 UTC (permalink / raw)
To: Shahriyar Jalayeri; +Cc: jarkko, peterhuewe, jgg, linux-integrity, linux-kernel
In-Reply-To: <20251010074956.6862-1-shahriyar@posteo.de>
Dear Shahriyar,
Thank you for the improved version.
Am 10.10.25 um 09:49 schrieb Shahriyar Jalayeri:
> Add two buffer size validations to prevent buffer overflows in
> tpm_inf_recv():
>
> 1. Validate that the provided buffer can hold at least the 4-byte header
> before attempting to read it.
> 2. Validate that the buffer is large enough to hold the data size reported
> by the TPM before reading the payload.
>
> Without these checks, a malicious or malfunctioning TPM could cause buffer
> overflows by reporting data sizes larger than the provided buffer, leading
> to memory corruption.
>
> Fixes: ebb81fdb3dd0 ("[PATCH] tpm: Support for Infineon TPM")
> Signed-off-by: Shahriyar Jalayeri <shahriyar@posteo.de>
> ---
> Changelog:
> v5:
> - replaced space indentation with tabs before the header comment
> v4:
> - removed the curly braces around one line statements
> v3:
> - removed dev_err() logs
> - added missing "fixes" tag
> - described header structure
> - fixed commit message length
> - removed use of local variable as constant
> - fixed check to account for off-by-six bytes
> v2:
> - added complete changes in the commit message
> - dev_err() logged expected sizes and stated operation is aborted
>
> drivers/char/tpm/tpm_infineon.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
> index 7638b65b8..d76aae08b 100644
> --- a/drivers/char/tpm/tpm_infineon.c
> +++ b/drivers/char/tpm/tpm_infineon.c
> @@ -250,6 +250,10 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count)
> number_of_wtx = 0;
>
> recv_begin:
> + /* expect at least 1-byte VL header, 1-byte ctrl-tag, 2-byte data size */
> + if (count < 4)
> + return -EIO;
> +
> /* start receiving header */
> for (i = 0; i < 4; i++) {
> ret = wait(chip, STAT_RDA);
> @@ -268,6 +272,9 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count)
> /* size of the data received */
> size = ((buf[2] << 8) | buf[3]);
>
> + if (size + 6 > count)
> + return -EIO;
> +
> for (i = 0; i < size; i++) {
> wait(chip, STAT_RDA);
> buf[i] = tpm_data_in(RDFIFO);
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Kind regards,
Paul
^ permalink raw reply
* [PATCH v5] tpm: infineon: add bounds check in tpm_inf_recv
From: Shahriyar Jalayeri @ 2025-10-10 7:49 UTC (permalink / raw)
To: jarkko; +Cc: shahriyar, peterhuewe, jgg, linux-integrity, linux-kernel
Add two buffer size validations to prevent buffer overflows in
tpm_inf_recv():
1. Validate that the provided buffer can hold at least the 4-byte header
before attempting to read it.
2. Validate that the buffer is large enough to hold the data size reported
by the TPM before reading the payload.
Without these checks, a malicious or malfunctioning TPM could cause buffer
overflows by reporting data sizes larger than the provided buffer, leading
to memory corruption.
Fixes: ebb81fdb3dd0 ("[PATCH] tpm: Support for Infineon TPM")
Signed-off-by: Shahriyar Jalayeri <shahriyar@posteo.de>
---
Changelog:
v5:
- replaced space indentation with tabs before the header comment
v4:
- removed the curly braces around one line statements
v3:
- removed dev_err() logs
- added missing "fixes" tag
- described header structure
- fixed commit message length
- removed use of local variable as constant
- fixed check to account for off-by-six bytes
v2:
- added complete changes in the commit message
- dev_err() logged expected sizes and stated operation is aborted
drivers/char/tpm/tpm_infineon.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
index 7638b65b8..d76aae08b 100644
--- a/drivers/char/tpm/tpm_infineon.c
+++ b/drivers/char/tpm/tpm_infineon.c
@@ -250,6 +250,10 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count)
number_of_wtx = 0;
recv_begin:
+ /* expect at least 1-byte VL header, 1-byte ctrl-tag, 2-byte data size */
+ if (count < 4)
+ return -EIO;
+
/* start receiving header */
for (i = 0; i < 4; i++) {
ret = wait(chip, STAT_RDA);
@@ -268,6 +272,9 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count)
/* size of the data received */
size = ((buf[2] << 8) | buf[3]);
+ if (size + 6 > count)
+ return -EIO;
+
for (i = 0; i < size; i++) {
wait(chip, STAT_RDA);
buf[i] = tpm_data_in(RDFIFO);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2] tpm: infineon: add bounds check in tpm_inf_recv
From: Paul Menzel @ 2025-10-10 7:11 UTC (permalink / raw)
To: Shahriyar Jalayeri; +Cc: jarkko, peterhuewe, jgg, linux-integrity, linux-kernel
In-Reply-To: <20251010065252.4377-1-shahriyar@posteo.de>
Dear Shahriyar,
Thank you for the improved version.
Am 10.10.25 um 08:52 schrieb Shahriyar Jalayeri:
> Add two buffer size validations to prevent buffer overflows in
> tpm_inf_recv():
>
> 1. Validate that the provided buffer can hold at least the 4-byte header
> before attempting to read it.
> 2. Validate that the buffer is large enough to hold the data size reported
> by the TPM before reading the payload.
>
> Without these checks, a malicious or malfunctioning TPM could cause buffer
> overflows by reporting data sizes larger than the provided buffer, leading
> to memory corruption.
>
> Fixes: ebb81fdb3dd0 ("[PATCH] tpm: Support for Infineon TPM")
> Signed-off-by: Shahriyar Jalayeri <shahriyar@posteo.de>
> ---
A changelog would be nice to have here.
> drivers/char/tpm/tpm_infineon.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
> index 7638b65b8..0fe4193a3 100644
> --- a/drivers/char/tpm/tpm_infineon.c
> +++ b/drivers/char/tpm/tpm_infineon.c
> @@ -250,6 +250,10 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count)
> number_of_wtx = 0;
>
> recv_begin:
> + /* expect at least 1-byte VL header, 1-byte ctrl-tag, 2-byte data size */
`scripts/checkpatch.pl` should have complained about using spaces
instead of tabs for indentation.
> + if (count < 4)
> + return -EIO;
> +
> /* start receiving header */
> for (i = 0; i < 4; i++) {
> ret = wait(chip, STAT_RDA);
> @@ -268,6 +272,9 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count)
> /* size of the data received */
> size = ((buf[2] << 8) | buf[3]);
>
> + if (size + 6 > count)
> + return -EIO;
> +
> for (i = 0; i < size; i++) {
> wait(chip, STAT_RDA);
> buf[i] = tpm_data_in(RDFIFO);
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Kind regards,
Paul
^ permalink raw reply
* [PATCH v2] tpm: infineon: add bounds check in tpm_inf_recv
From: Shahriyar Jalayeri @ 2025-10-10 6:52 UTC (permalink / raw)
To: jarkko; +Cc: shahriyar, peterhuewe, jgg, linux-integrity, linux-kernel
Add two buffer size validations to prevent buffer overflows in
tpm_inf_recv():
1. Validate that the provided buffer can hold at least the 4-byte header
before attempting to read it.
2. Validate that the buffer is large enough to hold the data size reported
by the TPM before reading the payload.
Without these checks, a malicious or malfunctioning TPM could cause buffer
overflows by reporting data sizes larger than the provided buffer, leading
to memory corruption.
Fixes: ebb81fdb3dd0 ("[PATCH] tpm: Support for Infineon TPM")
Signed-off-by: Shahriyar Jalayeri <shahriyar@posteo.de>
---
drivers/char/tpm/tpm_infineon.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
index 7638b65b8..0fe4193a3 100644
--- a/drivers/char/tpm/tpm_infineon.c
+++ b/drivers/char/tpm/tpm_infineon.c
@@ -250,6 +250,10 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count)
number_of_wtx = 0;
recv_begin:
+ /* expect at least 1-byte VL header, 1-byte ctrl-tag, 2-byte data size */
+ if (count < 4)
+ return -EIO;
+
/* start receiving header */
for (i = 0; i < 4; i++) {
ret = wait(chip, STAT_RDA);
@@ -268,6 +272,9 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count)
/* size of the data received */
size = ((buf[2] << 8) | buf[3]);
+ if (size + 6 > count)
+ return -EIO;
+
for (i = 0; i < size; i++) {
wait(chip, STAT_RDA);
buf[i] = tpm_data_in(RDFIFO);
--
2.43.0
^ permalink raw reply related
* Re: [GIT PULL] TPM DEVICE DRIVER: tpmdd-next-v6.18-2
From: Jarkko Sakkinen @ 2025-10-10 5:38 UTC (permalink / raw)
To: Linus Torvalds
Cc: Peter Huewe, Jason Gunthorpe, David Howells, keyrings,
linux-integrity, linux-kernel
In-Reply-To: <aOibAOKu_lEsSlC8@kernel.org>
On Fri, Oct 10, 2025 at 08:35:00AM +0300, Jarkko Sakkinen wrote:
> As per Chris' feedback, commands fail because it is based on Google's a
> non-standard proprietary TPM alike implementation. And the issue is not
> PC Client Profile specific. "typical profiles" are fine when they become
"atypical profiles"
> "typical profiles".
BR, Jarkko
^ 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