* Re: [PATCH v3] memfd,selinux: call security_inode_init_security_anon
From: Paul Moore @ 2025-09-21 18:31 UTC (permalink / raw)
To: Thiébaud Weksteen, Hugh Dickins
Cc: James Morris, Stephen Smalley, Jeff Vander Stoep, Nick Kralevich,
Jeff Xu, Baolin Wang, Isaac Manjarres, linux-kernel,
linux-security-module, selinux, linux-mm
In-Reply-To: <20250918020434.1612137-1-tweek@google.com>
On Wed, Sep 17, 2025 at 10:04 PM Thiébaud Weksteen <tweek@google.com> wrote:
>
> Prior to this change, no security hooks were called at the creation of a
> memfd file. It means that, for SELinux as an example, it will receive
> the default type of the filesystem that backs the in-memory inode. In
> most cases, that would be tmpfs, but if MFD_HUGETLB is passed, it will
> be hugetlbfs. Both can be considered implementation details of memfd.
>
> It also means that it is not possible to differentiate between a file
> coming from memfd_create and a file coming from a standard tmpfs mount
> point.
>
> Additionally, no permission is validated at creation, which differs from
> the similar memfd_secret syscall.
>
> Call security_inode_init_security_anon during creation. This ensures
> that the file is setup similarly to other anonymous inodes. On SELinux,
> it means that the file will receive the security context of its task.
>
> The ability to limit fexecve on memfd has been of interest to avoid
> potential pitfalls where /proc/self/exe or similar would be executed
> [1][2]. Reuse the "execute_no_trans" and "entrypoint" access vectors,
> similarly to the file class. These access vectors may not make sense for
> the existing "anon_inode" class. Therefore, define and assign a new
> class "memfd_file" to support such access vectors.
>
> Guard these changes behind a new policy capability named "memfd_class".
>
> [1] https://crbug.com/1305267
> [2] https://lore.kernel.org/lkml/20221215001205.51969-1-jeffxu@google.com/
>
> Signed-off-by: Thiébaud Weksteen <tweek@google.com>
> ---
> Changes since v2:
> - Add WARN_ON when using unexpected class. Return -EACCES instead
> of -EPERM
> - Remove extra new line
> - Rebase on selinux/dev
>
> Changes since v1:
> - Move test of class earlier in selinux_bprm_creds_for_exec
> - Remove duplicate call to security_transition_sid
>
> Changes since RFC:
> - Remove enum argument, simply compare the anon inode name
> - Introduce a policy capability for compatility
> - Add validation of class in selinux_bprm_creds_for_exec
> include/linux/memfd.h | 2 ++
> mm/memfd.c | 14 ++++++++++--
> security/selinux/hooks.c | 26 +++++++++++++++++-----
> security/selinux/include/classmap.h | 2 ++
> security/selinux/include/policycap.h | 1 +
> security/selinux/include/policycap_names.h | 1 +
> security/selinux/include/security.h | 5 +++++
> 7 files changed, 44 insertions(+), 7 deletions(-)
Thanks Thiébaud, I'm going to merge this into selinux/dev-staging now
with the plan to move it over to selinux/dev after the upcoming merge
window closes.
Hugh, since the changes between this patch and the v2 you ACK'd are
minimal and limited to the SELinux error handling code (see diff
below), I'm going to carry over your ACK, but if you have any concerns
or objections please let us know.
Thanks everyone!
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v4 11/34] lsm: get rid of the lsm_names list and do some cleanup
From: Paul Moore @ 2025-09-21 19:23 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-security-module, linux-integrity, selinux, 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: <d514db2f7c1de9b6d9092ff2ad1ce4cdba286e83.camel@linux.ibm.com>
On Fri, Sep 19, 2025 at 3:16 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Tue, 2025-09-16 at 18:03 -0400, Paul Moore wrote:
> > The LSM currently has a lot of code to maintain a list of the currently
> > active LSMs in a human readable string, with the only user being the
> > "/sys/kernel/security/lsm" code. Let's drop all of that code and
> > generate the string on first use and then cache it for subsequent use.
> >
> > Signed-off-by: Paul Moore <paul@paul-moore.com>
>
> FYI, checkpatch.pl complains of unbalanced braces, otherwise
Looks good to me?
% stg export --stdout lsm-lsm_names_cleanup | ./scripts/checkpatch.pl -
total: 0 errors, 0 warnings, 139 lines checked
Your patch has no obvious style problems and is ready for submission.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH 31/39] convert selinuxfs
From: Al Viro @ 2025-09-21 21:41 UTC (permalink / raw)
To: Paul Moore
Cc: linux-fsdevel, torvalds, brauner, jack, kees, casey,
linux-security-module, john.johansen
In-Reply-To: <CAHC9VhTRsQtncKx4bkbkSqVXpZyQLHbvKkcaVO-ss21Fq36r+Q@mail.gmail.com>
On Sun, Sep 21, 2025 at 04:44:28PM -0400, Paul Moore wrote:
> > + dput(dentry);
> > + return dentry; // borrowed
> > }
>
> Prefer C style comments on their own line:
>
> dput(dentry);
> /* borrowed dentry */
> return dentry;
Umm... IMO that's more of an annotation along the lines of "fallthru"...
> > @@ -2079,15 +2088,14 @@ static int sel_fill_super(struct super_block *sb, struct fs_context *fc)
> > goto err;
> > }
> >
> > - fsi->policycap_dir = sel_make_dir(sb->s_root, POLICYCAP_DIR_NAME,
> > + dentry = sel_make_dir(sb->s_root, POLICYCAP_DIR_NAME,
> > &fsi->last_ino);
>
> I'd probably keep fsi->policycap_dir in this patch simply to limit the
> scope of this patch to just the DCACHE_PERSISTENT related changes, but
> I'm not going to make a big fuss about that.
Not hard to split that way. Will do...
BTW, an unrelated question: does userland care about selinuxfs /null being
called that (and being on selinuxfs, for that matter)? Same for the
apparmor's securityfs /apparmor/.null...
If nobody cares, I would rather add an internal-only filesystem with
root being a character device (1,3) and whatever markings selinux et.al.
need for it. With open_devnull(creds) provided for selinux,
apparmor and whoever else wants to play with neutering descriptors
on exec...
^ permalink raw reply
* Re: [RFC PATCH 1/6] landlock: Add a place for flags to layer rules
From: Tingmao Wang @ 2025-09-21 23:52 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, Jan Kara, linux-security-module
In-Reply-To: <20250919.oanahnoon2Za@digikod.net>
On 9/19/25 17:02, Mickaël Salaün wrote:
> On Tue, Sep 09, 2025 at 01:06:35AM +0100, Tingmao Wang wrote:
>> To avoid unnecessarily increasing the size of struct landlock_layer, we
>> make the layer level a u8 and use the space to store the flags struct.
>>
>> Signed-off-by: Tingmao Wang <m@maowtm.org>
>> ---
>> security/landlock/fs.c | 75 ++++++++++++++++++++++++-------------
>> security/landlock/net.c | 3 +-
>> security/landlock/ruleset.c | 9 ++++-
>> security/landlock/ruleset.h | 27 ++++++++++++-
>> 4 files changed, 83 insertions(+), 31 deletions(-)
>>
>> [...]
>> @@ -643,6 +644,12 @@ bool landlock_unmask_layers(const struct landlock_rule *const rule,
>> unsigned long access_bit;
>> bool is_empty;
>>
>> + if (rule_flags) {
>> + /* Collect rule flags for each layer */
>> + if (layer->flags.quiet)
>> + rule_flags->quiet_masks |= layer_bit;
>
> This patch makes quiet flags related to on object, but not tied to the
> rule access rights (as explained in the next patch's doc). To tie it to
> rule access rights would require to duplicate the access bits for each
> rule (because multiple rules can be tied to the same object for the same
> layer/ruleset).
(imo the use of "rule" here as a terminology is a bit confusing, I would
have thought that a "rule" is a collection of access rights associated
with an object, and therefore you can of course only have one rule per
object in a ruleset. Otherwise landlock_add_rule should really have been
called landlock_add_rules...?)
>
> So, the question is, what do we really need to mute?
>
> I think the current approach is enough. We could still add a new flag in
> the future, or maybe even a new field to each rule type. However, we
> should rename the flag to make it clear that the it's related to the
> rule's object which is muted instead of the whole rule. Maybe something
> like LANDLOCK_ADD_RULE_QUIET_OBJECT?
I don't see what benefit a new field to each rule type would bring, since
different rule types targets different objects and live in different
rbtrees, and so they are already separable.
>
> If we want to have a more fine-grained control, a complementary patch
> could add a bitfield for each access right type to quiet a denied access
> right iff the object is also quiet (where rules are possible). That
> could be a follow up to complete this quiet feature, but this patch
> series is good on its own.
Worth noting that if one really wants to suppress logging for only some
access bits and we do not add support for it (due to the extra overhead
etc), that is still doable with just this patch by using two layers - the
outer one would contain the intended rules but not have any quiet flags,
whereas the inner one would contain the same set of rules but with quiet
flags set, except that for access bits which the sandbox does not want to
be quiet, it would also "allow" them in the second layer (access would
still be denied by the first layer, but would get audit logged due to
quiet flag not applying when the younger layer allows the access).
But this gets very tricky in the context of mutable domains, and does not
work at all for the purpose of controlling whether supervisor mode would
delegate to the supervisor or deny outright, since supervisors are
"accumulative". Therefore if this (different "quietness" for different
access bits) becomes a strong need, we should probably consider some way
of implementing it, rather than expecting a sandboxer to do this two-layer
hack. (But implementing this does have the potential to result in needing
to have a (number of access bits) x (number of layers) matrix...)
> [...]
Will add suggested changes in v2. Thanks for the review :)
Kind regards,
Tingmao
^ permalink raw reply
* Re: [RFC PATCH 5/6] landlock/access: Improve explanation on the deny_masks_t
From: Tingmao Wang @ 2025-09-21 23:52 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, Jan Kara, linux-security-module
In-Reply-To: <20250919.loh1Oor4busa@digikod.net>
On 9/19/25 17:04, Mickaël Salaün wrote:
> Looks good, I'll take it.
Thanks, will skip this in the next version.
>
> On Tue, Sep 09, 2025 at 01:06:39AM +0100, Tingmao Wang wrote:
>> Not really related to this series, but just something which took me a
>> while to realize, and would probably be helpful as a comment.
>
> Please just describe the change in the main commit message and move this
> kind of explanation bellow a "---", just after your SoB. This is useful
> for review and avoid unrelated information when picking a patch out of
> this context.
Got it :)
>
>>
>> Signed-off-by: Tingmao Wang <m@maowtm.org>
>> ---
>> security/landlock/access.h | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/security/landlock/access.h b/security/landlock/access.h
>> index 7961c6630a2d..5e2285575479 100644
>> --- a/security/landlock/access.h
>> +++ b/security/landlock/access.h
>> @@ -67,8 +67,10 @@ typedef u16 layer_mask_t;
>> static_assert(BITS_PER_TYPE(layer_mask_t) >= LANDLOCK_MAX_NUM_LAYERS);
>>
>> /*
>> - * Tracks domains responsible of a denied access. This is required to avoid
>> - * storing in each object the full layer_masks[] required by update_request().
>> + * Tracks domains responsible of a denied access, stored in the form of
>> + * two 4-bit layer numbers packed into a byte (one for each optional
>> + * access). This is required to avoid storing in each object the full
>> + * layer_masks[] required by update_request().
>> */
>> typedef u8 deny_masks_t;
>>
>> --
>> 2.51.0
>>
>>
^ permalink raw reply
* Re: [PATCH 31/39] convert selinuxfs
From: Paul Moore @ 2025-09-22 2:45 UTC (permalink / raw)
To: Al Viro, selinux
Cc: linux-fsdevel, torvalds, brauner, jack, kees, casey,
linux-security-module, john.johansen
In-Reply-To: <20250921214110.GN39973@ZenIV>
On Sun, Sep 21, 2025 at 5:41 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Sun, Sep 21, 2025 at 04:44:28PM -0400, Paul Moore wrote:
> > > + dput(dentry);
> > > + return dentry; // borrowed
> > > }
> >
> > Prefer C style comments on their own line:
> >
> > dput(dentry);
> > /* borrowed dentry */
> > return dentry;
>
> Umm... IMO that's more of an annotation along the lines of "fallthru"...
Maybe, I still prefer the example provided above. The heart wants
what the heart wants I guess.
> > > @@ -2079,15 +2088,14 @@ static int sel_fill_super(struct super_block *sb, struct fs_context *fc)
> > > goto err;
> > > }
> > >
> > > - fsi->policycap_dir = sel_make_dir(sb->s_root, POLICYCAP_DIR_NAME,
> > > + dentry = sel_make_dir(sb->s_root, POLICYCAP_DIR_NAME,
> > > &fsi->last_ino);
> >
> > I'd probably keep fsi->policycap_dir in this patch simply to limit the
> > scope of this patch to just the DCACHE_PERSISTENT related changes, but
> > I'm not going to make a big fuss about that.
>
> Not hard to split that way. Will do...
Thanks.
> BTW, an unrelated question: does userland care about selinuxfs /null being
> called that (and being on selinuxfs, for that matter)? Same for the
> apparmor's securityfs /apparmor/.null...
That's an interesting question. The kernel really only references it
in one place after creation, and as you've already seen, that's easily
changed. It's more important that it can be uniquely labeled such
that most any process can open it, otherwise we run into problems when
trying to replace fds when another file that the process can't open.
I'm adding the SELinux list to tap into the folks that play with
userland more than I do, but off the top of my head I can't think of
why userspace would need to do anything directly with
/sys/fs/selinux/null. There are some comments in the userland code
about not being able to mount selinuxfs with MS_NODEV due to the null
device, but that's the only obvious thing I see while quickly
searching through the code tonight.
> If nobody cares, I would rather add an internal-only filesystem with
> root being a character device (1,3) and whatever markings selinux et.al.
> need for it. With open_devnull(creds) provided for selinux,
> apparmor and whoever else wants to play with neutering descriptors
> on exec...
With the ongoing efforts to push towards better support for multiple
simultaneous LSMs, we would likely need to make sure there each LSM
that currently has their own null device would continue to have their
own, otherwise we potentially run into permission conflicts between
LSMs where one could end up blocking another and then we're back to
not having a file to use as a replacement. Not sure if that is what
you had in mind with your proposal, but just wanted to make sure that
was factored into the idea.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v3] memfd,selinux: call security_inode_init_security_anon
From: Hugh Dickins @ 2025-09-22 3:23 UTC (permalink / raw)
To: Paul Moore
Cc: Thiébaud Weksteen, Hugh Dickins, James Morris,
Stephen Smalley, Jeff Vander Stoep, Nick Kralevich, Jeff Xu,
Baolin Wang, Isaac Manjarres, linux-kernel, linux-security-module,
selinux, linux-mm
In-Reply-To: <CAHC9VhSbWJ-8tj5BxSTxznGO8zraKRSE31a+tqdfMHB53ef-MQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3224 bytes --]
On Sun, 21 Sep 2025, Paul Moore wrote:
> On Wed, Sep 17, 2025 at 10:04 PM Thiébaud Weksteen <tweek@google.com> wrote:
> >
> > Prior to this change, no security hooks were called at the creation of a
> > memfd file. It means that, for SELinux as an example, it will receive
> > the default type of the filesystem that backs the in-memory inode. In
> > most cases, that would be tmpfs, but if MFD_HUGETLB is passed, it will
> > be hugetlbfs. Both can be considered implementation details of memfd.
> >
> > It also means that it is not possible to differentiate between a file
> > coming from memfd_create and a file coming from a standard tmpfs mount
> > point.
> >
> > Additionally, no permission is validated at creation, which differs from
> > the similar memfd_secret syscall.
> >
> > Call security_inode_init_security_anon during creation. This ensures
> > that the file is setup similarly to other anonymous inodes. On SELinux,
> > it means that the file will receive the security context of its task.
> >
> > The ability to limit fexecve on memfd has been of interest to avoid
> > potential pitfalls where /proc/self/exe or similar would be executed
> > [1][2]. Reuse the "execute_no_trans" and "entrypoint" access vectors,
> > similarly to the file class. These access vectors may not make sense for
> > the existing "anon_inode" class. Therefore, define and assign a new
> > class "memfd_file" to support such access vectors.
> >
> > Guard these changes behind a new policy capability named "memfd_class".
> >
> > [1] https://crbug.com/1305267
> > [2] https://lore.kernel.org/lkml/20221215001205.51969-1-jeffxu@google.com/
> >
> > Signed-off-by: Thiébaud Weksteen <tweek@google.com>
> > ---
> > Changes since v2:
> > - Add WARN_ON when using unexpected class. Return -EACCES instead
> > of -EPERM
> > - Remove extra new line
> > - Rebase on selinux/dev
> >
> > Changes since v1:
> > - Move test of class earlier in selinux_bprm_creds_for_exec
> > - Remove duplicate call to security_transition_sid
> >
> > Changes since RFC:
> > - Remove enum argument, simply compare the anon inode name
> > - Introduce a policy capability for compatility
> > - Add validation of class in selinux_bprm_creds_for_exec
> > include/linux/memfd.h | 2 ++
> > mm/memfd.c | 14 ++++++++++--
> > security/selinux/hooks.c | 26 +++++++++++++++++-----
> > security/selinux/include/classmap.h | 2 ++
> > security/selinux/include/policycap.h | 1 +
> > security/selinux/include/policycap_names.h | 1 +
> > security/selinux/include/security.h | 5 +++++
> > 7 files changed, 44 insertions(+), 7 deletions(-)
>
> Thanks Thiébaud, I'm going to merge this into selinux/dev-staging now
> with the plan to move it over to selinux/dev after the upcoming merge
> window closes.
>
> Hugh, since the changes between this patch and the v2 you ACK'd are
> minimal and limited to the SELinux error handling code (see diff
> below), I'm going to carry over your ACK, but if you have any concerns
> or objections please let us know.
Sure, please do carry over my ACK - thanks.
Hugh
^ permalink raw reply
* [PATCH] tpm: Use -EPERM as fallback error code in tpm_ret_to_err
From: Jarkko Sakkinen @ 2025-09-22 7:23 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, stable, Peter Huewe, Jarkko Sakkinen,
Jason Gunthorpe, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, James Bottomley, Mimi Zohar, Stefano Garzarella,
open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM
From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Using -EFAULT here was not the best idea for tpm_ret_to_err as the fallback
error code as it is no concise with trusted keys.
Change the fallback as -EPERM, process TPM_RC_HASH also in tpm_ret_to_err,
and by these changes make the helper applicable for trusted keys.
Cc: stable@vger.kernel.org # v6.15+
Fixes: 539fbab37881 ("tpm: Mask TPM RC in tpm2_start_auth_session()")
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
---
include/linux/tpm.h | 9 +++++---
security/keys/trusted-keys/trusted_tpm2.c | 26 ++++++-----------------
2 files changed, 13 insertions(+), 22 deletions(-)
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index dc0338a783f3..667d290789ca 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -449,13 +449,16 @@ static inline ssize_t tpm_ret_to_err(ssize_t ret)
if (ret < 0)
return ret;
- switch (tpm2_rc_value(ret)) {
- case TPM2_RC_SUCCESS:
+ if (!ret)
return 0;
+
+ switch (tpm2_rc_value(ret)) {
case TPM2_RC_SESSION_MEMORY:
return -ENOMEM;
+ case TPM2_RC_HASH:
+ return -EINVAL;
default:
- return -EFAULT;
+ return -EPERM;
}
}
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 024be262702f..e165b117bbca 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -348,25 +348,19 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
}
blob_len = tpm2_key_encode(payload, options, &buf.data[offset], blob_len);
+ if (blob_len < 0)
+ rc = blob_len;
out:
tpm_buf_destroy(&sized);
tpm_buf_destroy(&buf);
- if (rc > 0) {
- if (tpm2_rc_value(rc) == TPM2_RC_HASH)
- rc = -EINVAL;
- else
- rc = -EPERM;
- }
- if (blob_len < 0)
- rc = blob_len;
- else
+ if (!rc)
payload->blob_len = blob_len;
out_put:
tpm_put_ops(chip);
- return rc;
+ return tpm_ret_to_err(rc);
}
/**
@@ -468,10 +462,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
kfree(blob);
tpm_buf_destroy(&buf);
- if (rc > 0)
- rc = -EPERM;
-
- return rc;
+ return tpm_ret_to_err(rc);
}
/**
@@ -534,8 +525,6 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
tpm_buf_fill_hmac_session(chip, &buf);
rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
rc = tpm_buf_check_hmac_response(chip, &buf, rc);
- if (rc > 0)
- rc = -EPERM;
if (!rc) {
data_len = be16_to_cpup(
@@ -568,7 +557,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
out:
tpm_buf_destroy(&buf);
- return rc;
+ return tpm_ret_to_err(rc);
}
/**
@@ -600,6 +589,5 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
out:
tpm_put_ops(chip);
-
- return rc;
+ return tpm_ret_to_err(rc);
}
--
2.39.5
^ permalink raw reply related
* Re: [PATCH v7] tpm: Make TPM buffer allocations more robust
From: Stefano Garzarella @ 2025-09-22 7:58 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, Stefan Berger, Jarkko Sakkinen, Peter Huewe,
Jason Gunthorpe, James Bottomley, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, open list,
open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM
In-Reply-To: <aM13m-CYijlu6Pf6@kernel.org>
On Fri, Sep 19, 2025 at 06:32:43PM +0300, Jarkko Sakkinen wrote:
>On Fri, Sep 19, 2025 at 03:35:43PM +0200, Stefano Garzarella wrote:
>> On Fri, Sep 19, 2025 at 02:24:47PM +0300, Jarkko Sakkinen wrote:
>> > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
>> >
>> > Drop 'tpm_buf_init', 'tpm_buf_init_sized' and 'tpm_buf_free'. Refine
>> > 'struct tpm_buf' to hold capacity in order to enable stack allocation and
>> > sizes other than page size.
>> >
>> > The updated 'struct tpm_buf' can be allocated either from stack or heap.
>> >
>> > The contract is the following:
>> >
>> > 1. 'tpm_buf_reset' and 'tpm_buf_reset_size' expect that on the first run
>> > the passed buffer is zeroed by the caller (e.g. via memset or kzalloc).
>> > 2. The same buffer can be reused. On the second and subsequent resets the
>> > aforementioned functions verify that 'buf_size' has the same value, and
>> > emits warning if not.
>> >
>> > As a consequence 'struct tpm_buf' instance can be easily wrapped into
>> > managed allocation:
>> >
>> > struct tpm_buf *buf __free(kfree) buf = kzalloc(PAGE_SIZE,
>> > GFP_KERNEL);
>> >
>> > Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
>> > ---
>> > v7:
>> > - Additional function comments and invariant was unfortunately left to
>> > my staging area so here's the addition (does not affect functionality).
>> > v6:
>> > - Removed two empty lines as requested by Stefan:
>> > https://lore.kernel.org/linux-integrity/be1c5bef-7c97-4173-b417-986dc90d779c@linux.ibm.com/
>> > - Add 'capacity' field as this makes easy to stretch tpm_buf into stack
>> > allocation.
>> > v5:
>> > - I tested this version also with TPM 1.2 by booting up and checking
>> > that sysfs attributes work.
>> > - Fixed the length check against capacity (James) with TPM_BUF_CAPACITY.
>> > - Fixed declaration style: do it at the site (Jason).
>> > - Improved commit message (Stefan).
>> > - Removed "out" label from tpm2_pcr_read() (Stefan).
>> > - Removed spurious "return rc;" from tpm2_get_pcr_allocation() (Stefan).
>> > v4:
>> > - Wrote a more a descriptive short summary and improved description.
>> > - Fixed the error in documentation: there is 4090 bytes of space left
>> > for the payload - not 4088 bytes.
>> > - Turned tpm_buf_alloc() into inline function.
>> > v3:
>> > - Removed the cleanup class and moved on using __free(kfree) instead.
>> > - Removed `buf_size` (James).
>> > - I'm open for the idea of splitting still (Jason) but I'll hold
>> > at least this revision just to check that my core idea here
>> > is correct.
>> > v2:
>> > - Implement also memory allocation using the cleanup class.
>> > - Rewrote the commit message.
>> > - Implemented CLASS_TPM_BUF() helper macro.
>> > ---
>> > drivers/char/tpm/tpm-buf.c | 68 ++----
>> > drivers/char/tpm/tpm-sysfs.c | 19 +-
>> > drivers/char/tpm/tpm1-cmd.c | 143 ++++++------
>> > drivers/char/tpm/tpm2-cmd.c | 270 ++++++++++------------
>> > drivers/char/tpm/tpm2-sessions.c | 118 +++++-----
>> > drivers/char/tpm/tpm2-space.c | 42 ++--
>> > drivers/char/tpm/tpm_vtpm_proxy.c | 29 +--
>> > include/linux/tpm.h | 17 +-
>> > security/keys/trusted-keys/trusted_tpm1.c | 40 ++--
>> > security/keys/trusted-keys/trusted_tpm2.c | 153 ++++++------
>> > 10 files changed, 390 insertions(+), 509 deletions(-)
>> >
>> > diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
>> > index dc882fc9fa9e..19774bc5786f 100644
>> > --- a/drivers/char/tpm/tpm-buf.c
>> > +++ b/drivers/char/tpm/tpm-buf.c
>> > @@ -7,83 +7,57 @@
>> > #include <linux/module.h>
>> > #include <linux/tpm.h>
>> >
>> > -/**
>> > - * tpm_buf_init() - Allocate and initialize a TPM command
>> > - * @buf: A &tpm_buf
>> > - * @tag: TPM_TAG_RQU_COMMAND, TPM2_ST_NO_SESSIONS or TPM2_ST_SESSIONS
>> > - * @ordinal: A command ordinal
>> > - *
>> > - * Return: 0 or -ENOMEM
>> > - */
>> > -int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
>> > -{
>> > - buf->data = (u8 *)__get_free_page(GFP_KERNEL);
>> > - if (!buf->data)
>> > - return -ENOMEM;
>> > -
>> > - tpm_buf_reset(buf, tag, ordinal);
>> > - return 0;
>> > -}
>> > -EXPORT_SYMBOL_GPL(tpm_buf_init);
>> > -
>> > /**
>> > * tpm_buf_reset() - Initialize a TPM command
>> > * @buf: A &tpm_buf
>> > + * @buf_size: Size of the buffer.
>> > * @tag: TPM_TAG_RQU_COMMAND, TPM2_ST_NO_SESSIONS or TPM2_ST_SESSIONS
>> > * @ordinal: A command ordinal
>> > + *
>> > + * 1. Expects that on the first run the passed buffer is zeroed by the caller.
>> > + * 2. Old buffer can be reused. On the second and subsequent resets @buf_size is
>> > + * verified to be equal to the previous value.
>> > */
>> > -void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
>> > +void tpm_buf_reset(struct tpm_buf *buf, u16 buf_size, u16 tag, u32 ordinal)
>> > {
>> > struct tpm_header *head = (struct tpm_header *)buf->data;
>> >
>> > + WARN_ON(buf->capacity != 0 && buf_size != (buf->capacity + sizeof(*buf)));
>> > WARN_ON(tag != TPM_TAG_RQU_COMMAND && tag != TPM2_ST_NO_SESSIONS &&
>> > tag != TPM2_ST_SESSIONS && tag != 0);
>> >
>> > buf->flags = 0;
>> > buf->length = sizeof(*head);
>> > + buf->capacity = buf_size - sizeof(*buf);
>> > + buf->handles = 0;
>> > head->tag = cpu_to_be16(tag);
>> > head->length = cpu_to_be32(sizeof(*head));
>> > head->ordinal = cpu_to_be32(ordinal);
>> > - buf->handles = 0;
>> > }
>> > EXPORT_SYMBOL_GPL(tpm_buf_reset);
>> >
>> > -/**
>> > - * tpm_buf_init_sized() - Allocate and initialize a sized (TPM2B) buffer
>> > - * @buf: A @tpm_buf
>> > - *
>> > - * Return: 0 or -ENOMEM
>> > - */
>> > -int tpm_buf_init_sized(struct tpm_buf *buf)
>> > -{
>> > - buf->data = (u8 *)__get_free_page(GFP_KERNEL);
>> > - if (!buf->data)
>> > - return -ENOMEM;
>> > -
>> > - tpm_buf_reset_sized(buf);
>> > - return 0;
>> > -}
>> > -EXPORT_SYMBOL_GPL(tpm_buf_init_sized);
>> > -
>> > /**
>> > * tpm_buf_reset_sized() - Initialize a sized buffer
>> > * @buf: A &tpm_buf
>> > + * @buf_size: Size of the buffer.
>> > + *
>> > + * 1. Expects that on the first run the passed buffer is zeroed by the caller.
>> > + * 2. Old buffer can be reused. On the second and subsequent resets @buf_size is
>> > + * verified to be equal to the previous value.
>> > */
>> > -void tpm_buf_reset_sized(struct tpm_buf *buf)
>> > +void tpm_buf_reset_sized(struct tpm_buf *buf, u16 buf_size)
>> > {
>> > + WARN_ON(buf->capacity != 0 && buf_size != (buf->capacity + sizeof(*buf)));
>> > +
>> > buf->flags = TPM_BUF_TPM2B;
>> > buf->length = 2;
>> > + buf->capacity = buf_size - sizeof(*buf);
>> > + buf->handles = 0;
>> > buf->data[0] = 0;
>> > buf->data[1] = 0;
>> > }
>> > EXPORT_SYMBOL_GPL(tpm_buf_reset_sized);
>> >
>> > -void tpm_buf_destroy(struct tpm_buf *buf)
>> > -{
>> > - free_page((unsigned long)buf->data);
>> > -}
>> > -EXPORT_SYMBOL_GPL(tpm_buf_destroy);
>> > -
>> > /**
>> > * tpm_buf_length() - Return the number of bytes consumed by the data
>> > * @buf: A &tpm_buf
>> *
>> * Return: The number of bytes consumed by the buffer
>> */
>> u32 tpm_buf_length(struct tpm_buf *buf)
>> {
>> return buf->length;
>> }
>> EXPORT_SYMBOL_GPL(tpm_buf_length);
>>
>> Should we update the return type (u16) on this function?
>
>Yes we should. Thanks for catching this.
>
>>
>> > @@ -108,7 +82,7 @@ void tpm_buf_append(struct tpm_buf *buf, const u8 *new_data, u16 new_length)
>> > if (buf->flags & TPM_BUF_OVERFLOW)
>> > return;
>> >
>> > - if ((buf->length + new_length) > PAGE_SIZE) {
>> > + if ((buf->length + new_length) > buf->capacity) {
>>
>> IIUC all of these are u16, so there could be an overflow when we do
>> `buf->length + new_length` ?
>>
>> Should we cast to u32 or just change the expression in something like
>> `new_length > (buf->capacity - buf->length)`
>
>Thanks, these really appropriate comments.
>
>I think best measure here would have hard constant cap of PAGE_SIZE
>for buf_size. That's how the specs limit anyhow. I'll extend the
>invariant.
Oh I see, that makes sense, thanks :-)
Stefano
^ permalink raw reply
* Re: [PATCH v10 1/4] tpm: Make TPM buffer allocations more robust
From: Stefano Garzarella @ 2025-09-22 8:44 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, Jarkko Sakkinen, Stefan Berger, Peter Huewe,
Jason Gunthorpe, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, James Bottomley, Mimi Zohar, open list,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM
In-Reply-To: <20250921020804.1088824-2-jarkko@kernel.org>
On Sun, Sep 21, 2025 at 05:08:01AM +0300, Jarkko Sakkinen wrote:
>From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
>
>Create more ergonomic primitives to work with TPM buffers, where
>'tpm_buf_init*' initialize the memory and 'tpm_buf_reset*' (re)set a buffer
>for a particular use and purpose. The new primitives are ubiquitos when
>it comes to heap and stack usage.
>
>Given that the kzalloc is decoupled, a managed allocation can be done
>trivially:
>
> struct tpm_buf *buf __free(kfree) buf = kzalloc(TPM_BUF_MAX_SIZE,
> GFP_KERNEL);
>
>This effectively zeros out the odds having any memory leaks with TPM
>buffers. The new structures can be later used to widen the use of stack
>over heap in the subsystem in the critical code paths..
>
>Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
>---
>v10:
>- No changes.
>v9:
>- Rename pre-existing TPM_BUFSIZE as TPM_BUF_MAX_SIZE and redeclare
> it in include/linux/tpm.h, and use this constant instead of
> PAGE_SIZE in tpm_buf_init*. Define TPM_BUF_MIN_SIZE to be a
> sane placeholder value for stack allocations.
>- Fix and restructure invariant for the sake of clarity.
>- memset buffers to zero in tpm_buf_init* in order to guarantee a legit
> initial state. This does not cause any regressions but once tpm_buf
> is allocaed from stack at some site, this will zero out chance of
> corrupted state.
>v8:
>- Decouple memory init i.e. bring tpm_init* back but with new fresh
> form.
>- Cap buf_size to page size and also make checks in tpm_buf_append
> safe:
> https://lore.kernel.org/linux-integrity/hwsx2t2tkbos3g7k2syemxbvc6sfrsbviwm64ubcdl6ms7ljvo@toetomkhheht/
>- Fix trusted_tpm_send() and simplify the flow.
>v7:
>- Additional function comments and invariant was unfortunately left to
> my staging area so here's the addition (does not affect functionality).
>v6:
>- Removed two empty lines as requested by Stefan:
> https://lore.kernel.org/linux-integrity/be1c5bef-7c97-4173-b417-986dc90d779c@linux.ibm.com/
>- Add 'capacity' field as this makes easy to stretch tpm_buf into stack
> allocation.
>v5:
>- I tested this version also with TPM 1.2 by booting up and checking
> that sysfs attributes work.
>- Fixed the length check against capacity (James) with TPM_BUF_CAPACITY.
>- Fixed declaration style: do it at the site (Jason).
>- Improved commit message (Stefan).
>- Removed "out" label from tpm2_pcr_read() (Stefan).
>- Removed spurious "return rc;" from tpm2_get_pcr_allocation() (Stefan).
>v4:
>- Wrote a more a descriptive short summary and improved description.
>- Fixed the error in documentation: there is 4090 bytes of space left
> for the payload - not 4088 bytes.
>- Turned tpm_buf_alloc() into inline function.
>v3:
>- Removed the cleanup class and moved on using __free(kfree) instead.
>- Removed `buf_size` (James).
>- I'm open for the idea of splitting still (Jason) but I'll hold
> at least this revision just to check that my core idea here
> is correct.
>v2:
>- Implement also memory allocation using the cleanup class.
>- Rewrote the commit message.
>- Implemented CLASS_TPM_BUF() helper macro.
>---
> drivers/char/tpm/tpm-buf.c | 137 +++++++----
> drivers/char/tpm/tpm-dev-common.c | 4 +-
> drivers/char/tpm/tpm-dev.h | 2 +-
> drivers/char/tpm/tpm-interface.c | 4 +-
> drivers/char/tpm/tpm-sysfs.c | 20 +-
> drivers/char/tpm/tpm.h | 3 +-
> drivers/char/tpm/tpm1-cmd.c | 149 ++++++------
> drivers/char/tpm/tpm2-cmd.c | 282 ++++++++++------------
> drivers/char/tpm/tpm2-sessions.c | 121 +++++-----
> drivers/char/tpm/tpm2-space.c | 44 ++--
> drivers/char/tpm/tpm_tis_i2c.c | 4 +-
> drivers/char/tpm/tpm_vtpm_proxy.c | 32 ++-
> include/linux/tpm.h | 28 ++-
> security/keys/trusted-keys/trusted_tpm1.c | 34 ++-
> security/keys/trusted-keys/trusted_tpm2.c | 156 ++++++------
> 15 files changed, 493 insertions(+), 527 deletions(-)
>
>diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
>index dc882fc9fa9e..82dce0350a41 100644
>--- a/drivers/char/tpm/tpm-buf.c
>+++ b/drivers/char/tpm/tpm-buf.c
>@@ -7,82 +7,107 @@
> #include <linux/module.h>
> #include <linux/tpm.h>
>
>-/**
>- * tpm_buf_init() - Allocate and initialize a TPM command
>- * @buf: A &tpm_buf
>- * @tag: TPM_TAG_RQU_COMMAND, TPM2_ST_NO_SESSIONS or TPM2_ST_SESSIONS
>- * @ordinal: A command ordinal
>- *
>- * Return: 0 or -ENOMEM
>- */
>-int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
>+static void __tpm_buf_size_invariant(struct tpm_buf *buf, u16 buf_size)
> {
>- buf->data = (u8 *)__get_free_page(GFP_KERNEL);
>- if (!buf->data)
>- return -ENOMEM;
>-
>- tpm_buf_reset(buf, tag, ordinal);
>- return 0;
>+ if (!buf->capacity) {
>+ if (buf_size > TPM_BUF_MAX_SIZE) {
>+ WARN(1, "%s: size overflow: %u\n", __func__, buf_size);
IIUC here we will always print something like:
"__tpm_buf_size_invariant: size overflow: XXX"
So, should we just remove `__func__` or maybe use a macro to print the
name of the caller?
>+ buf->flags |= TPM_BUF_ERROR;
>+ }
>+ } else {
>+ if (buf_size != buf->capacity + sizeof(*buf)) {
>+ WARN(1, "%s: size mismatch: %u != %u\n", __func__, buf_size,
>+ buf->capacity + sizeof(*buf));
>+ buf->flags |= TPM_BUF_ERROR;
>+ }
>+ }
> }
>-EXPORT_SYMBOL_GPL(tpm_buf_init);
>
>-/**
>- * tpm_buf_reset() - Initialize a TPM command
>- * @buf: A &tpm_buf
>- * @tag: TPM_TAG_RQU_COMMAND, TPM2_ST_NO_SESSIONS or TPM2_ST_SESSIONS
>- * @ordinal: A command ordinal
>- */
>-void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
>+static void __tpm_buf_reset(struct tpm_buf *buf, u16 buf_size, u16 tag, u32 ordinal)
> {
> struct tpm_header *head = (struct tpm_header *)buf->data;
>
>+ __tpm_buf_size_invariant(buf, buf_size);
>+
>+ if (buf->flags & TPM_BUF_ERROR)
>+ return;
>+
> WARN_ON(tag != TPM_TAG_RQU_COMMAND && tag != TPM2_ST_NO_SESSIONS &&
> tag != TPM2_ST_SESSIONS && tag != 0);
>
> buf->flags = 0;
> buf->length = sizeof(*head);
>+ buf->capacity = buf_size - sizeof(*buf);
>+ buf->handles = 0;
> head->tag = cpu_to_be16(tag);
> head->length = cpu_to_be32(sizeof(*head));
> head->ordinal = cpu_to_be32(ordinal);
>+}
>+
>+static void __tpm_buf_reset_sized(struct tpm_buf *buf, u16 buf_size)
>+{
>+ __tpm_buf_size_invariant(buf, buf_size);
>+
>+ if (buf->flags & TPM_BUF_ERROR)
>+ return;
>+
>+ buf->flags = TPM_BUF_TPM2B;
>+ buf->length = 2;
>+ buf->capacity = buf_size - sizeof(*buf);
> buf->handles = 0;
>+ buf->data[0] = 0;
>+ buf->data[1] = 0;
> }
>-EXPORT_SYMBOL_GPL(tpm_buf_reset);
>
> /**
>- * tpm_buf_init_sized() - Allocate and initialize a sized (TPM2B) buffer
>- * @buf: A @tpm_buf
>- *
>- * Return: 0 or -ENOMEM
>+ * tpm_buf_init() - Initialize a TPM command
>+ * @buf: A &tpm_buf
>+ * @buf_size: Size of the buffer.
> */
>-int tpm_buf_init_sized(struct tpm_buf *buf)
>+void tpm_buf_init(struct tpm_buf *buf, u16 buf_size)
> {
>- buf->data = (u8 *)__get_free_page(GFP_KERNEL);
>- if (!buf->data)
>- return -ENOMEM;
>+ memset(buf, 0, buf_size);
>+ __tpm_buf_reset(buf, buf_size, TPM_TAG_RQU_COMMAND, 0);
>+}
>+EXPORT_SYMBOL_GPL(tpm_buf_init);
>
>- tpm_buf_reset_sized(buf);
>- return 0;
>+/**
>+ * tpm_buf_init_sized() - Initialize a sized buffer
>+ * @buf: A &tpm_buf
>+ * @buf_size: Size of the buffer.
>+ */
>+void tpm_buf_init_sized(struct tpm_buf *buf, u16 buf_size)
>+{
>+ memset(buf, 0, buf_size);
>+ __tpm_buf_reset_sized(buf, buf_size);
> }
> EXPORT_SYMBOL_GPL(tpm_buf_init_sized);
>
> /**
>- * tpm_buf_reset_sized() - Initialize a sized buffer
>+ * tpm_buf_reset() - Re-initialize a TPM command
> * @buf: A &tpm_buf
>+ * @tag: TPM_TAG_RQU_COMMAND, TPM2_ST_NO_SESSIONS or TPM2_ST_SESSIONS
>+ * @ordinal: A command ordinal
> */
>-void tpm_buf_reset_sized(struct tpm_buf *buf)
>+void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
> {
>- buf->flags = TPM_BUF_TPM2B;
>- buf->length = 2;
>- buf->data[0] = 0;
>- buf->data[1] = 0;
>+ u16 buf_size = buf->capacity + sizeof(*buf);
>+
>+ __tpm_buf_reset(buf, buf_size, tag, ordinal);
> }
>-EXPORT_SYMBOL_GPL(tpm_buf_reset_sized);
>+EXPORT_SYMBOL_GPL(tpm_buf_reset);
>
>-void tpm_buf_destroy(struct tpm_buf *buf)
>+/**
>+ * tpm_buf_reset_sized() - Re-initialize a sized buffer
>+ * @buf: A &tpm_buf
>+ */
>+void tpm_buf_reset_sized(struct tpm_buf *buf)
> {
>- free_page((unsigned long)buf->data);
>+ u16 buf_size = buf->capacity + sizeof(*buf);
>+
>+ __tpm_buf_reset_sized(buf, buf_size);
> }
>-EXPORT_SYMBOL_GPL(tpm_buf_destroy);
>+EXPORT_SYMBOL_GPL(tpm_buf_reset_sized);
>
> /**
> * tpm_buf_length() - Return the number of bytes consumed by the data
>@@ -92,6 +117,9 @@ EXPORT_SYMBOL_GPL(tpm_buf_destroy);
> */
> u32 tpm_buf_length(struct tpm_buf *buf)
> {
>+ if (buf->flags & TPM_BUF_ERROR)
>+ return 0;
>+
> return buf->length;
> }
> EXPORT_SYMBOL_GPL(tpm_buf_length);
>@@ -104,13 +132,14 @@ EXPORT_SYMBOL_GPL(tpm_buf_length);
> */
> void tpm_buf_append(struct tpm_buf *buf, const u8 *new_data, u16 new_length)
> {
>- /* Return silently if overflow has already happened. */
>- if (buf->flags & TPM_BUF_OVERFLOW)
>+ u32 total_length = (u32)buf->length + (u32)new_length;
>+
>+ if (buf->flags & TPM_BUF_ERROR)
> return;
>
>- if ((buf->length + new_length) > PAGE_SIZE) {
>+ if (total_length > (u32)buf->capacity) {
> WARN(1, "tpm_buf: write overflow\n");
>- buf->flags |= TPM_BUF_OVERFLOW;
>+ buf->flags |= TPM_BUF_ERROR;
> return;
> }
>
>@@ -157,8 +186,12 @@ EXPORT_SYMBOL_GPL(tpm_buf_append_u32);
> */
> void tpm_buf_append_handle(struct tpm_chip *chip, struct tpm_buf *buf, u32 handle)
> {
>+ if (buf->flags & TPM_BUF_ERROR)
>+ return;
>+
> if (buf->flags & TPM_BUF_TPM2B) {
>- dev_err(&chip->dev, "Invalid buffer type (TPM2B)\n");
>+ dev_err(&chip->dev, "%s: invalid for buffer type: TPM2B\n", __func__);
>+ buf->flags |= TPM_BUF_ERROR;
> return;
> }
>
>@@ -178,13 +211,13 @@ static void tpm_buf_read(struct tpm_buf *buf, off_t *offset, size_t count, void
> off_t next_offset;
>
> /* Return silently if overflow has already happened. */
>- if (buf->flags & TPM_BUF_BOUNDARY_ERROR)
>+ if (buf->flags & TPM_BUF_ERROR)
> return;
>
> next_offset = *offset + count;
> if (next_offset > buf->length) {
> WARN(1, "tpm_buf: read out of boundary\n");
>- buf->flags |= TPM_BUF_BOUNDARY_ERROR;
>+ buf->flags |= TPM_BUF_ERROR;
> return;
> }
>
>@@ -242,5 +275,3 @@ u32 tpm_buf_read_u32(struct tpm_buf *buf, off_t *offset)
> return be32_to_cpu(value);
> }
> EXPORT_SYMBOL_GPL(tpm_buf_read_u32);
>-
>-
>diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
>index f2a5e09257dd..4f5893555fb7 100644
>--- a/drivers/char/tpm/tpm-dev-common.c
>+++ b/drivers/char/tpm/tpm-dev-common.c
>@@ -147,7 +147,7 @@ ssize_t tpm_common_read(struct file *file, char __user *buf,
>
> rc = copy_to_user(buf, priv->data_buffer + *off,
> ret_size);
> if (rc) {
>- memset(priv->data_buffer, 0, TPM_BUFSIZE);
>+ memset(priv->data_buffer, 0, TPM_BUF_MAX_SIZE);
> priv->response_length = 0;
> ret_size = -EFAULT;
> } else {
>@@ -173,7 +173,7 @@ ssize_t tpm_common_write(struct file *file, const char __user *buf,
> struct file_priv *priv = file->private_data;
> int ret = 0;
>
>- if (size > TPM_BUFSIZE)
>+ if (size > TPM_BUF_MAX_SIZE)
> return -E2BIG;
>
> mutex_lock(&priv->buffer_mutex);
>diff --git a/drivers/char/tpm/tpm-dev.h b/drivers/char/tpm/tpm-dev.h
>index f3742bcc73e3..700e3d9d8b64 100644
>--- a/drivers/char/tpm/tpm-dev.h
>+++ b/drivers/char/tpm/tpm-dev.h
>@@ -18,7 +18,7 @@ struct file_priv {
> bool response_read;
> bool command_enqueued;
>
>- u8 data_buffer[TPM_BUFSIZE];
>+ u8 data_buffer[TPM_BUF_MAX_SIZE];
> };
>
> void tpm_common_open(struct file *file, struct tpm_chip *chip,
>diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
>index c9f173001d0e..b0d5098fb92b 100644
>--- a/drivers/char/tpm/tpm-interface.c
>+++ b/drivers/char/tpm/tpm-interface.c
>@@ -100,8 +100,8 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz)
> if (bufsiz < TPM_HEADER_SIZE)
> return -EINVAL;
>
>- if (bufsiz > TPM_BUFSIZE)
>- bufsiz = TPM_BUFSIZE;
>+ if (bufsiz > TPM_BUF_MAX_SIZE)
>+ bufsiz = TPM_BUF_MAX_SIZE;
>
> count = be32_to_cpu(header->length);
> ordinal = be32_to_cpu(header->ordinal);
>diff --git a/drivers/char/tpm/tpm-sysfs.c
>b/drivers/char/tpm/tpm-sysfs.c
>index 94231f052ea7..4213a8285ed0 100644
>--- a/drivers/char/tpm/tpm-sysfs.c
>+++ b/drivers/char/tpm/tpm-sysfs.c
>@@ -32,28 +32,30 @@ struct tpm_readpubek_out {
> static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
>- struct tpm_buf tpm_buf;
> struct tpm_readpubek_out *out;
> int i;
> char *str = buf;
> struct tpm_chip *chip = to_tpm_chip(dev);
> char anti_replay[20];
>
>+ struct tpm_buf *tpm_buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL);
We're using PAGE_SIZE instead of TPM_BUF_MAX_SIZE to reduce the pressure
on the allocator, right?
I was wondering if we could create an inline function or a macro that
calls kzalloc() and tpm_buf_init().
Just because we do it often, it's not a strong opinion, just something
that came to mind while doing the review.
I mean something like this (untested):
struct tpm_buf *tpm_buf_alloc_max(void) {
struct tpm_buf *buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
if (!buf)
return NULL;
tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
return buf;
}
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH v10 4/4] tpm_vpm_proxy: Use stack for TPM_CC_SET_LOCALITY
From: Stefano Garzarella @ 2025-09-22 8:46 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM
In-Reply-To: <20250921020804.1088824-5-jarkko@kernel.org>
On Sun, Sep 21, 2025 at 05:08:04AM +0300, Jarkko Sakkinen wrote:
>From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
>
>Use stack allocation for TPM_CC_SET_LOCALITY, as it has known fixed size.
>
>Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
>---
> drivers/char/tpm/tpm1-cmd.c | 2 +-
> drivers/char/tpm/tpm_vtpm_proxy.c | 12 +++++-------
> 2 files changed, 6 insertions(+), 8 deletions(-)
>
>diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
>index 11c16ad9b2a7..433908cfb4a9 100644
>--- a/drivers/char/tpm/tpm1-cmd.c
>+++ b/drivers/char/tpm/tpm1-cmd.c
>@@ -328,7 +328,7 @@ static int tpm1_startup(struct tpm_chip *chip)
> int rc;
>
> dev_info(&chip->dev, "TPM_Startup\n");
>- tpm_buf_init(buf, TPM_BUF_INT_SIZE);
>+ tpm_buf_init(buf, TPM_BUF_MIN_SIZE);
This change should be squashed in patch 2, right?
Thanks,
Stefano
> tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_STARTUP);
> tpm_buf_append_u16(buf, TPM_ST_CLEAR);
> rc = tpm_transmit_cmd(chip, buf, 0, "TPM_Startup");
>diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
>index e5de14379eb2..0f1b1b67ed4e 100644
>--- a/drivers/char/tpm/tpm_vtpm_proxy.c
>+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
>@@ -395,15 +395,13 @@ static bool vtpm_proxy_tpm_req_canceled(struct tpm_chip *chip, u8 status)
>
> static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
> {
>- int rc;
>- const struct tpm_header *header;
> struct proxy_dev *proxy_dev = dev_get_drvdata(&chip->dev);
>+ u8 buf_data[TPM_BUF_MIN_SIZE];
>+ struct tpm_buf *buf = (struct tpm_buf *)buf_data;
>+ const struct tpm_header *header;
>+ int rc;
>
>- struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL);
>- if (!buf)
>- return -ENOMEM;
>-
>- tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
>+ tpm_buf_init(buf, TPM_BUF_MIN_SIZE);
> if (chip->flags & TPM_CHIP_FLAG_TPM2)
> tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_SET_LOCALITY);
> else
>--
>2.39.5
>
^ permalink raw reply
* Re: [PATCH] tpm: Use -EPERM as fallback error code in tpm_ret_to_err
From: Stefano Garzarella @ 2025-09-22 9:25 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, Jarkko Sakkinen, stable, Peter Huewe,
Jason Gunthorpe, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, James Bottomley, Mimi Zohar, open list,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM
In-Reply-To: <20250922072332.2649135-1-jarkko@kernel.org>
On Mon, Sep 22, 2025 at 10:23:32AM +0300, Jarkko Sakkinen wrote:
>From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
>
>Using -EFAULT here was not the best idea for tpm_ret_to_err as the fallback
>error code as it is no concise with trusted keys.
>
>Change the fallback as -EPERM, process TPM_RC_HASH also in tpm_ret_to_err,
>and by these changes make the helper applicable for trusted keys.
>
>Cc: stable@vger.kernel.org # v6.15+
>Fixes: 539fbab37881 ("tpm: Mask TPM RC in tpm2_start_auth_session()")
>Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
>---
> include/linux/tpm.h | 9 +++++---
> security/keys/trusted-keys/trusted_tpm2.c | 26 ++++++-----------------
> 2 files changed, 13 insertions(+), 22 deletions(-)
>
>diff --git a/include/linux/tpm.h b/include/linux/tpm.h
>index dc0338a783f3..667d290789ca 100644
>--- a/include/linux/tpm.h
>+++ b/include/linux/tpm.h
>@@ -449,13 +449,16 @@ static inline ssize_t tpm_ret_to_err(ssize_t ret)
> if (ret < 0)
> return ret;
>
>- switch (tpm2_rc_value(ret)) {
>- case TPM2_RC_SUCCESS:
I slightly prefer the `case TPM2_RC_SUCCESS` but I don't have a strong
opinion.
>+ if (!ret)
> return 0;
If we want to remove the `case TPM2_RC_SUCCESS`, can we just merge this
condition with the if on top, I mean:
if (ret <= 0)
return ret;
>+
>+ switch (tpm2_rc_value(ret)) {
> case TPM2_RC_SESSION_MEMORY:
> return -ENOMEM;
>+ case TPM2_RC_HASH:
>+ return -EINVAL;
> default:
>- return -EFAULT;
>+ return -EPERM;
> }
> }
>
>diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
>index 024be262702f..e165b117bbca 100644
>--- a/security/keys/trusted-keys/trusted_tpm2.c
>+++ b/security/keys/trusted-keys/trusted_tpm2.c
>@@ -348,25 +348,19 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> }
>
> blob_len = tpm2_key_encode(payload, options, &buf.data[offset], blob_len);
>+ if (blob_len < 0)
>+ rc = blob_len;
>
> out:
> tpm_buf_destroy(&sized);
> tpm_buf_destroy(&buf);
>
>- if (rc > 0) {
>- if (tpm2_rc_value(rc) == TPM2_RC_HASH)
>- rc = -EINVAL;
>- else
>- rc = -EPERM;
>- }
>- if (blob_len < 0)
nit: since `blob_len` is not accessed anymore in the error path, can we
avoid to set it to 0 when declaring it?
Thanks,
Stefano
>- rc = blob_len;
>- else
>+ if (!rc)
> payload->blob_len = blob_len;
>
> out_put:
> tpm_put_ops(chip);
>- return rc;
>+ return tpm_ret_to_err(rc);
> }
>
> /**
>@@ -468,10 +462,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> kfree(blob);
> tpm_buf_destroy(&buf);
>
>- if (rc > 0)
>- rc = -EPERM;
>-
>- return rc;
>+ return tpm_ret_to_err(rc);
> }
>
> /**
>@@ -534,8 +525,6 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
> tpm_buf_fill_hmac_session(chip, &buf);
> rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
> rc = tpm_buf_check_hmac_response(chip, &buf, rc);
>- if (rc > 0)
>- rc = -EPERM;
>
> if (!rc) {
> data_len = be16_to_cpup(
>@@ -568,7 +557,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>
> out:
> tpm_buf_destroy(&buf);
>- return rc;
>+ return tpm_ret_to_err(rc);
> }
>
> /**
>@@ -600,6 +589,5 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
>
> out:
> tpm_put_ops(chip);
>-
>- return rc;
>+ return tpm_ret_to_err(rc);
> }
>--
>2.39.5
>
^ permalink raw reply
* Re: [PATCH v3 00/35] Compiler-Based Capability- and Locking-Analysis
From: Marco Elver @ 2025-09-22 9:33 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Nathan Chancellor, Peter Zijlstra, Boqun Feng, Ingo Molnar,
Will Deacon, David S. Miller, Luc Van Oostenryck,
Paul E. McKenney, Alexander Potapenko, Arnd Bergmann,
Bart Van Assche, Bill Wendling, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Jonathan Corbet, Josh Triplett,
Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Neeraj Upadhyay,
Nick Desaulniers, Steven Rostedt, Tetsuo Handa, Thomas Gleixner,
Thomas Graf, Uladzislau Rezki, Waiman Long, kasan-dev,
linux-crypto, linux-doc, linux-kbuild, linux-kernel, linux-mm,
linux-security-module, linux-sparse, llvm, rcu
In-Reply-To: <20250919140954.GA24160@lst.de>
On Fri, Sep 19, 2025 at 04:09PM +0200, Christoph Hellwig wrote:
> On Fri, Sep 19, 2025 at 04:08:03PM +0200, Christoph Hellwig wrote:
> > I started to play around with that. For the nvme code adding the
> > annotations was very simply, and I also started adding trivial
> > __guarded_by which instantly found issues.
> >
> > For XFS it was a lot more work and I still see tons of compiler
> > warnings, which I'm not entirely sure how to address. Right now I
> > see three major classes:
>
> And in case anyone cares, here are my patches for that:
>
> https://git.infradead.org/?p=users/hch/misc.git;a=shortlog;h=refs/heads/cap-analysis
>
> git://git.infradead.org/users/hch/misc.git cap-analysis
I gave this a try, and with the below patch and the Clang fix [1],
fs/xfs compiles cleanly. I think the fundamental limitation are the
conditional locking wrappers. I suspect it's possible to do better than
disabling the analysis here, by overapproximating the lock set taken
(like you did elsewhere), so that at least the callers are checked, but
when I tried it showed lots of callers need annotating as well, so I
gave up at that point. Still, it might be better than no checking at
all.
[1] https://github.com/llvm/llvm-project/pull/159921
Thanks,
-- Marco
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 9c39251961a3..f371a08e5d44 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -140,6 +140,7 @@ void
xfs_ilock(
xfs_inode_t *ip,
uint lock_flags)
+ __capability_unsafe(/* conditional locking */)
{
trace_xfs_ilock(ip, lock_flags, _RET_IP_);
@@ -183,6 +184,7 @@ int
xfs_ilock_nowait(
xfs_inode_t *ip,
uint lock_flags)
+ __capability_unsafe(/* conditional locking */)
{
trace_xfs_ilock_nowait(ip, lock_flags, _RET_IP_);
@@ -243,6 +245,7 @@ void
xfs_iunlock(
xfs_inode_t *ip,
uint lock_flags)
+ __capability_unsafe(/* conditional locking */)
{
xfs_lock_flags_assert(lock_flags);
@@ -272,6 +275,7 @@ void
xfs_ilock_demote(
xfs_inode_t *ip,
uint lock_flags)
+ __capability_unsafe(/* conditional locking */)
{
ASSERT(lock_flags & (XFS_IOLOCK_EXCL|XFS_MMAPLOCK_EXCL|XFS_ILOCK_EXCL));
ASSERT((lock_flags &
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index d9ac9521c203..9c4ec3aa8bf9 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -472,6 +472,7 @@ xfs_log_reserve(
static void
xlog_state_shutdown_callbacks(
struct xlog *log)
+ __must_hold(&log->l_icloglock)
{
struct xlog_in_core *iclog;
LIST_HEAD(cb_list);
^ permalink raw reply related
* Re: [PATCH v4 11/34] lsm: get rid of the lsm_names list and do some cleanup
From: Mimi Zohar @ 2025-09-22 10:52 UTC (permalink / raw)
To: Paul Moore
Cc: linux-security-module, linux-integrity, selinux, 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: <CAHC9VhSOhaB86yEV0+2HWRc3oYgZmLX+Nz3ERbohGRHeroKThA@mail.gmail.com>
On Sun, 2025-09-21 at 15:23 -0400, Paul Moore wrote:
> On Fri, Sep 19, 2025 at 3:16 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Tue, 2025-09-16 at 18:03 -0400, Paul Moore wrote:
> > > The LSM currently has a lot of code to maintain a list of the currently
> > > active LSMs in a human readable string, with the only user being the
> > > "/sys/kernel/security/lsm" code. Let's drop all of that code and
> > > generate the string on first use and then cache it for subsequent use.
> > >
> > > Signed-off-by: Paul Moore <paul@paul-moore.com>
> >
> > FYI, checkpatch.pl complains of unbalanced braces, otherwise
>
> Looks good to me?
>
> % stg export --stdout lsm-lsm_names_cleanup | ./scripts/checkpatch.pl -
> total: 0 errors, 0 warnings, 139 lines checked
>
> Your patch has no obvious style problems and is ready for submission.
Try adding "--strict", which enforces
https://www.kernel.org/doc/html/v4.10/process/coding-style.html#placing-braces-and-spaces
Mimi
^ permalink raw reply
* Re: [PATCH bpf-next v7 4/5] bpftool: Add support for signing BPF programs
From: Quentin Monnet @ 2025-09-22 11:24 UTC (permalink / raw)
To: KP Singh, bpf, linux-security-module
Cc: bboscaccy, paul, kys, ast, daniel, andrii
In-Reply-To: <20250921160120.9711-5-kpsingh@kernel.org>
2025-09-21 18:01 UTC+0200 ~ KP Singh <kpsingh@kernel.org>
> Two modes of operation being added:
>
> Add two modes of operation:
>
> * For prog load, allow signing a program immediately before loading. This
> is essential for command-line testing and administration.
>
> bpftool prog load -S -k <private_key> -i <identity_cert> fentry_test.bpf.o
>
> * For gen skeleton, embed a pre-generated signature into the C skeleton
> file. This supports the use of signed programs in compiled applications.
>
> bpftool gen skeleton -S -k <private_key> -i <identity_cert> fentry_test.bpf.o
>
> Generation of the loader program and its metadata map is implemented in
> libbpf (bpf_obj__gen_loader). bpftool generates a skeleton that loads
> the program and automates the required steps: freezing the map, creating
> an exclusive map, loading, and running. Users can use standard libbpf
> APIs directly or integrate loader program generation into their own
> toolchains.
>
> Signed-off-by: KP Singh <kpsingh@kernel.org>
Acked-by: Quentin Monnet <qmo@kernel.org>
Thanks a lot!
> ---
> .../bpf/bpftool/Documentation/bpftool-gen.rst | 13 +-
> .../bpftool/Documentation/bpftool-prog.rst | 14 +-
> tools/bpf/bpftool/Makefile | 6 +-
> tools/bpf/bpftool/cgroup.c | 4 +
> tools/bpf/bpftool/gen.c | 68 +++++-
> tools/bpf/bpftool/main.c | 26 ++-
> tools/bpf/bpftool/main.h | 11 +
> tools/bpf/bpftool/prog.c | 29 ++-
> tools/bpf/bpftool/sign.c | 212 ++++++++++++++++++
> 9 files changed, 372 insertions(+), 11 deletions(-)
> create mode 100644 tools/bpf/bpftool/sign.c
>
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-gen.rst b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
> index ca860fd97d8d..d0a36f442db7 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-gen.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
> @@ -16,7 +16,7 @@ SYNOPSIS
>
> **bpftool** [*OPTIONS*] **gen** *COMMAND*
>
> -*OPTIONS* := { |COMMON_OPTIONS| | { **-L** | **--use-loader** } }
> +*OPTIONS* := { |COMMON_OPTIONS| | { **-L** | **--use-loader** } | [ { **-S** | **--sign** } {**-k** <private_key.pem>} **-i** <certificate.x509> ] }
>
> *COMMAND* := { **object** | **skeleton** | **help** }
>
> @@ -186,6 +186,17 @@ OPTIONS
> skeleton). A light skeleton contains a loader eBPF program. It does not use
> the majority of the libbpf infrastructure, and does not need libelf.
>
> +-S, --sign
> + For skeletons, generate a signed skeleton. This option must be used with
> + **-k** and **-i**. Using this flag implicitly enables **--use-loader**.
> +
> +-k <private_key.pem>
> + Path to the private key file in PEM format, required for signing.
> +
> +-i <certificate.x509>
> + Path to the X.509 certificate file in PEM or DER format, required for
> + signing.
> +
> EXAMPLES
> ========
> **$ cat example1.bpf.c**
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> index f69fd92df8d8..009633294b09 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> @@ -18,7 +18,7 @@ SYNOPSIS
>
> *OPTIONS* := { |COMMON_OPTIONS| |
> { **-f** | **--bpffs** } | { **-m** | **--mapcompat** } | { **-n** | **--nomount** } |
> -{ **-L** | **--use-loader** } }
> +{ **-L** | **--use-loader** } | [ { **-S** | **--sign** } **-k** <private_key.pem> **-i** <certificate.x509> ] }
Perfect, thank you!
>
> *COMMANDS* :=
> { **show** | **list** | **dump xlated** | **dump jited** | **pin** | **load** |
> @@ -248,6 +248,18 @@ OPTIONS
> creating the maps, and loading the programs (see **bpftool prog tracelog**
> as a way to dump those messages).
>
> +-S, --sign
> + Enable signing of the BPF program before loading. This option must be
> + used with **-k** and **-i**. Using this flag implicitly enables
> + **--use-loader**.
> +
> +-k <private_key.pem>
> + Path to the private key file in PEM format, required when signing.
> +
> +-i <certificate.x509>
> + Path to the X.509 certificate file in PEM or DER format, required when
> + signing.
> +
> EXAMPLES
> ========
> **# bpftool prog show**
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index 9e9a5f006cd2..586d1b2595d1 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -130,8 +130,8 @@ include $(FEATURES_DUMP)
> endif
> endif
>
> -LIBS = $(LIBBPF) -lelf -lz
> -LIBS_BOOTSTRAP = $(LIBBPF_BOOTSTRAP) -lelf -lz
> +LIBS = $(LIBBPF) -lelf -lz -lcrypto
> +LIBS_BOOTSTRAP = $(LIBBPF_BOOTSTRAP) -lelf -lz -lcrypto
>
> ifeq ($(feature-libelf-zstd),1)
> LIBS += -lzstd
> @@ -194,7 +194,7 @@ endif
>
> BPFTOOL_BOOTSTRAP := $(BOOTSTRAP_OUTPUT)bpftool
>
> -BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o gen.o btf.o)
> +BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o gen.o btf.o sign.o)
> $(BOOTSTRAP_OBJS): $(LIBBPF_BOOTSTRAP)
>
> OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
> diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
> index 944ebe21a216..ec356deb27c9 100644
> --- a/tools/bpf/bpftool/cgroup.c
> +++ b/tools/bpf/bpftool/cgroup.c
> @@ -2,6 +2,10 @@
> // Copyright (C) 2017 Facebook
> // Author: Roman Gushchin <guro@fb.com>
>
> +#undef GCC_VERSION
> +#ifndef _GNU_SOURCE
> +#define _GNU_SOURCE
> +#endif
> #define _XOPEN_SOURCE 500
> #include <errno.h>
> #include <fcntl.h>
> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> index 67a60114368f..993c7d9484a4 100644
> --- a/tools/bpf/bpftool/gen.c
> +++ b/tools/bpf/bpftool/gen.c
> @@ -1930,7 +1990,7 @@ static int do_help(int argc, char **argv)
> " %1$s %2$s help\n"
> "\n"
> " " HELP_SPEC_OPTIONS " |\n"
> - " {-L|--use-loader} }\n"
> + " {-L|--use-loader} | [ {-S|--sign } {-k} <private_key.pem> {-i} <certificate.x509> ]}\n"
With regards to our discussion on v4 - Sorry, I had not realised
removing the braces would make the sync test fail. ACK for keeping them
until this is resolved in the test.
As for the bash completion, I agree this should not block this series.
Please make sure to follow-up with it. I think it should be as follows:
------
diff --git i/tools/bpf/bpftool/bash-completion/bpftool w/tools/bpf/bpftool/bash-completion/bpftool
index 527bb47ac462..53bcfeb1a76e 100644
--- i/tools/bpf/bpftool/bash-completion/bpftool
+++ w/tools/bpf/bpftool/bash-completion/bpftool
@@ -262,7 +262,7 @@ _bpftool()
# Deal with options
if [[ ${words[cword]} == -* ]]; then
local c='--version --json --pretty --bpffs --mapcompat --debug \
- --use-loader --base-btf'
+ --use-loader --base-btf --sign -i -k'
COMPREPLY=( $( compgen -W "$c" -- "$cur" ) )
return 0
fi
@@ -283,7 +283,7 @@ _bpftool()
_sysfs_get_netdevs
return 0
;;
- file|pinned|-B|--base-btf)
+ file|pinned|-B|--base-btf|-i|-k)
_filedir
return 0
;;
@@ -296,13 +296,21 @@ _bpftool()
# Remove all options so completions don't have to deal with them.
local i pprev
for (( i=1; i < ${#words[@]}; )); do
- if [[ ${words[i]::1} == - ]] &&
- [[ ${words[i]} != "-B" ]] && [[ ${words[i]} != "--base-btf" ]]; then
- words=( "${words[@]:0:i}" "${words[@]:i+1}" )
- [[ $i -le $cword ]] && cword=$(( cword - 1 ))
- else
- i=$(( ++i ))
- fi
+ case ${words[i]} in
+ # Remove option and its argument
+ -B|--base-btf|-i|-k)
+ words=( "${words[@]:0:i}" "${words[@]:i+2}" )
+ [[ $i -le $(($cword + 1)) ]] && cword=$(( cword - 2 ))
+ ;;
+ # No argument, remove option only
+ -*)
+ words=( "${words[@]:0:i}" "${words[@]:i+1}" )
+ [[ $i -le $cword ]] && cword=$(( cword - 1 ))
+ ;;
+ *)
+ i=$(( ++i ))
+ ;;
+ esac
done
cur=${words[cword]}
prev=${words[cword - 1]}
^ permalink raw reply related
* [PATCH] tpm2-sessions: Remove unnecessary wrapper
From: Jarkko Sakkinen @ 2025-09-22 11:50 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
James Bottomley, Mimi Zohar, open list, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM
From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Open code tpm_buf_append_hmac_session_opt() because it adds unnecessary
disperancy to the call sites (and reduces the amount of code).
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
---
drivers/char/tpm/tpm2-cmd.c | 14 +++++++++++---
include/linux/tpm.h | 23 -----------------------
security/keys/trusted-keys/trusted_tpm2.c | 12 ++++++++++--
3 files changed, 21 insertions(+), 28 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 7d77f6fbc152..89e6169add88 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -257,9 +257,17 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
do {
tpm_buf_reset(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
- tpm_buf_append_hmac_session_opt(chip, &buf, TPM2_SA_ENCRYPT
- | TPM2_SA_CONTINUE_SESSION,
- NULL, 0);
+ if (tpm2_chip_auth(chip)) {
+ tpm_buf_append_hmac_session(chip, &buf,
+ TPM2_SA_ENCRYPT |
+ TPM2_SA_CONTINUE_SESSION,
+ NULL, 0);
+ } else {
+ offset = buf.handles * 4 + TPM_HEADER_SIZE;
+ head = (struct tpm_header *)buf.data;
+ if (tpm_buf_length(&buf) == offset)
+ head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
+ }
tpm_buf_append_u16(&buf, num_bytes);
tpm_buf_fill_hmac_session(chip, &buf);
err = tpm_transmit_cmd(chip, &buf,
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 667d290789ca..aaa407ddef21 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -534,29 +534,6 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
int passphraselen);
void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf,
u8 attributes, u8 *passphrase, int passphraselen);
-static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip,
- struct tpm_buf *buf,
- u8 attributes,
- u8 *passphrase,
- int passphraselen)
-{
- struct tpm_header *head;
- int offset;
-
- if (tpm2_chip_auth(chip)) {
- tpm_buf_append_hmac_session(chip, buf, attributes, passphrase, passphraselen);
- } else {
- offset = buf->handles * 4 + TPM_HEADER_SIZE;
- head = (struct tpm_header *)buf->data;
-
- /*
- * If the only sessions are optional, the command tag must change to
- * TPM2_ST_NO_SESSIONS.
- */
- if (tpm_buf_length(buf) == offset)
- head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
- }
-}
#ifdef CONFIG_TCG_TPM2_HMAC
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index e165b117bbca..c414a7006d78 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -482,8 +482,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
struct trusted_key_options *options,
u32 blob_handle)
{
+ struct tpm_header *head;
struct tpm_buf buf;
u16 data_len;
+ int offset;
u8 *data;
int rc;
@@ -518,8 +520,14 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
tpm2_buf_append_auth(&buf, options->policyhandle,
NULL /* nonce */, 0, 0,
options->blobauth, options->blobauth_len);
- tpm_buf_append_hmac_session_opt(chip, &buf, TPM2_SA_ENCRYPT,
- NULL, 0);
+ if (tpm2_chip_auth(chip)) {
+ tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0);
+ } else {
+ offset = buf.handles * 4 + TPM_HEADER_SIZE;
+ head = (struct tpm_header *)buf.data;
+ if (tpm_buf_length(&buf) == offset)
+ head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
+ }
}
tpm_buf_fill_hmac_session(chip, &buf);
--
2.39.5
^ permalink raw reply related
* Re: [PATCH 31/39] convert selinuxfs
From: Stephen Smalley @ 2025-09-22 12:34 UTC (permalink / raw)
To: Paul Moore
Cc: Al Viro, selinux, linux-fsdevel, torvalds, brauner, jack, kees,
casey, linux-security-module, john.johansen
In-Reply-To: <CAHC9VhSJJ5YLXZbB-SvQket-PJCv81quM6XLrBDc7+erus-vhA@mail.gmail.com>
On Sun, Sep 21, 2025 at 10:45 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Sun, Sep 21, 2025 at 5:41 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> > On Sun, Sep 21, 2025 at 04:44:28PM -0400, Paul Moore wrote:
> > > > + dput(dentry);
> > > > + return dentry; // borrowed
> > > > }
> > >
> > > Prefer C style comments on their own line:
> > >
> > > dput(dentry);
> > > /* borrowed dentry */
> > > return dentry;
> >
> > Umm... IMO that's more of an annotation along the lines of "fallthru"...
>
> Maybe, I still prefer the example provided above. The heart wants
> what the heart wants I guess.
>
> > > > @@ -2079,15 +2088,14 @@ static int sel_fill_super(struct super_block *sb, struct fs_context *fc)
> > > > goto err;
> > > > }
> > > >
> > > > - fsi->policycap_dir = sel_make_dir(sb->s_root, POLICYCAP_DIR_NAME,
> > > > + dentry = sel_make_dir(sb->s_root, POLICYCAP_DIR_NAME,
> > > > &fsi->last_ino);
> > >
> > > I'd probably keep fsi->policycap_dir in this patch simply to limit the
> > > scope of this patch to just the DCACHE_PERSISTENT related changes, but
> > > I'm not going to make a big fuss about that.
> >
> > Not hard to split that way. Will do...
>
> Thanks.
>
> > BTW, an unrelated question: does userland care about selinuxfs /null being
> > called that (and being on selinuxfs, for that matter)? Same for the
> > apparmor's securityfs /apparmor/.null...
>
> That's an interesting question. The kernel really only references it
> in one place after creation, and as you've already seen, that's easily
> changed. It's more important that it can be uniquely labeled such
> that most any process can open it, otherwise we run into problems when
> trying to replace fds when another file that the process can't open.
>
> I'm adding the SELinux list to tap into the folks that play with
> userland more than I do, but off the top of my head I can't think of
> why userspace would need to do anything directly with
> /sys/fs/selinux/null. There are some comments in the userland code
> about not being able to mount selinuxfs with MS_NODEV due to the null
> device, but that's the only obvious thing I see while quickly
> searching through the code tonight.
Is there a reason why these patches weren't sent to selinux list in
the first place?
In any event, yes, Android userspace (in particular the Android init
program) relies on /sys/fs/selinux/null at a point where /dev/null
does not yet exist [1]. Hence, I don't believe we can drop it since it
would break userspace.
[1] https://cs.android.com/search?q=%2Fsys%2Ffs%2Fselinux%2Fnull&sq=&ss=android%2Fplatform%2Fsuperproject%2Fmain
>
> > If nobody cares, I would rather add an internal-only filesystem with
> > root being a character device (1,3) and whatever markings selinux et.al.
> > need for it. With open_devnull(creds) provided for selinux,
> > apparmor and whoever else wants to play with neutering descriptors
> > on exec...
>
> With the ongoing efforts to push towards better support for multiple
> simultaneous LSMs, we would likely need to make sure there each LSM
> that currently has their own null device would continue to have their
> own, otherwise we potentially run into permission conflicts between
> LSMs where one could end up blocking another and then we're back to
> not having a file to use as a replacement. Not sure if that is what
> you had in mind with your proposal, but just wanted to make sure that
> was factored into the idea.
>
> --
> paul-moore.com
>
^ permalink raw reply
* Re: [PATCH v3] memfd,selinux: call security_inode_init_security_anon
From: Stephen Smalley @ 2025-09-22 13:12 UTC (permalink / raw)
To: Paul Moore
Cc: Thiébaud Weksteen, Hugh Dickins, James Morris,
Jeff Vander Stoep, Nick Kralevich, Jeff Xu, Baolin Wang,
Isaac Manjarres, linux-kernel, linux-security-module, selinux,
linux-mm
In-Reply-To: <CAHC9VhSbWJ-8tj5BxSTxznGO8zraKRSE31a+tqdfMHB53ef-MQ@mail.gmail.com>
On Sun, Sep 21, 2025 at 2:31 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Wed, Sep 17, 2025 at 10:04 PM Thiébaud Weksteen <tweek@google.com> wrote:
> >
> > Prior to this change, no security hooks were called at the creation of a
> > memfd file. It means that, for SELinux as an example, it will receive
> > the default type of the filesystem that backs the in-memory inode. In
> > most cases, that would be tmpfs, but if MFD_HUGETLB is passed, it will
> > be hugetlbfs. Both can be considered implementation details of memfd.
> >
> > It also means that it is not possible to differentiate between a file
> > coming from memfd_create and a file coming from a standard tmpfs mount
> > point.
> >
> > Additionally, no permission is validated at creation, which differs from
> > the similar memfd_secret syscall.
> >
> > Call security_inode_init_security_anon during creation. This ensures
> > that the file is setup similarly to other anonymous inodes. On SELinux,
> > it means that the file will receive the security context of its task.
> >
> > The ability to limit fexecve on memfd has been of interest to avoid
> > potential pitfalls where /proc/self/exe or similar would be executed
> > [1][2]. Reuse the "execute_no_trans" and "entrypoint" access vectors,
> > similarly to the file class. These access vectors may not make sense for
> > the existing "anon_inode" class. Therefore, define and assign a new
> > class "memfd_file" to support such access vectors.
> >
> > Guard these changes behind a new policy capability named "memfd_class".
> >
> > [1] https://crbug.com/1305267
> > [2] https://lore.kernel.org/lkml/20221215001205.51969-1-jeffxu@google.com/
> >
> > Signed-off-by: Thiébaud Weksteen <tweek@google.com>
> > ---
> > Changes since v2:
> > - Add WARN_ON when using unexpected class. Return -EACCES instead
> > of -EPERM
> > - Remove extra new line
> > - Rebase on selinux/dev
> >
> > Changes since v1:
> > - Move test of class earlier in selinux_bprm_creds_for_exec
> > - Remove duplicate call to security_transition_sid
> >
> > Changes since RFC:
> > - Remove enum argument, simply compare the anon inode name
> > - Introduce a policy capability for compatility
> > - Add validation of class in selinux_bprm_creds_for_exec
> > include/linux/memfd.h | 2 ++
> > mm/memfd.c | 14 ++++++++++--
> > security/selinux/hooks.c | 26 +++++++++++++++++-----
> > security/selinux/include/classmap.h | 2 ++
> > security/selinux/include/policycap.h | 1 +
> > security/selinux/include/policycap_names.h | 1 +
> > security/selinux/include/security.h | 5 +++++
> > 7 files changed, 44 insertions(+), 7 deletions(-)
>
> Thanks Thiébaud, I'm going to merge this into selinux/dev-staging now
> with the plan to move it over to selinux/dev after the upcoming merge
> window closes.
>
> Hugh, since the changes between this patch and the v2 you ACK'd are
> minimal and limited to the SELinux error handling code (see diff
> below), I'm going to carry over your ACK, but if you have any concerns
> or objections please let us know.
>
> Thanks everyone!
When would you recommend that I re-apply the corresponding userspace
patch to reserve this policy capability number for memfd_class?
After it is moved to selinux/dev? Understand that it isn't truly
reserved until it lands in a kernel.org kernel but would prefer to
reapply it sooner than that since there may be other policy capability
requests queueing up (e.g. bpf token) that should be done relative to
it. Can always revert it again if necessary, at least until another
userspace release is made (not sure on timeline for that).
>
> --
> paul-moore.com
^ permalink raw reply
* Re: [PATCH 31/39] convert selinuxfs
From: Al Viro @ 2025-09-22 13:46 UTC (permalink / raw)
To: Stephen Smalley
Cc: Paul Moore, selinux, linux-fsdevel, torvalds, brauner, jack, kees,
casey, linux-security-module, john.johansen
In-Reply-To: <CAEjxPJ4Ez1oYXa4hEcSLSrO+ikLN0kgrWQc+2n2K7wWoy7a7pQ@mail.gmail.com>
On Mon, Sep 22, 2025 at 08:34:02AM -0400, Stephen Smalley wrote:
> Is there a reason why these patches weren't sent to selinux list in
> the first place?
Will Cc on the next posting of that series.
> In any event, yes, Android userspace (in particular the Android init
> program) relies on /sys/fs/selinux/null at a point where /dev/null
> does not yet exist [1]. Hence, I don't believe we can drop it since it
> would break userspace.
Pity. Oh, well...
^ permalink raw reply
* Re: [RFC PATCH v2] lsm,selinux: introduce LSM_ATTR_UNSHARE and wire it up for SELinux
From: Casey Schaufler @ 2025-09-22 15:45 UTC (permalink / raw)
To: Stephen Smalley, linux-security-module, selinux
Cc: paul, omosnace, john.johansen, serge, Casey Schaufler
In-Reply-To: <20250918135904.9997-2-stephen.smalley.work@gmail.com>
On 9/18/2025 6:59 AM, Stephen Smalley wrote:
> RFC-only, will ultimately split the LSM-only changes to their own
> patch for submission. I have now tested this with the corresponding
> selinux userspace change that you can find at
> https://lore.kernel.org/selinux/20250918135118.9896-2-stephen.smalley.work@gmail.com/
> and also verified that my modified systemd-nspawn still works when
> starting containers with their own SELinux namespace.
>
> This defines a new LSM_ATTR_UNSHARE attribute for the
> lsm_set_self_attr(2) system call and wires it up for SELinux to invoke
> the underlying function for unsharing the SELinux namespace. As with
> the selinuxfs interface, this immediately unshares the SELinux
> namespace of the current process just like an unshare(2) system call
> would do for other namespaces. I have not yet explored the
> alternatives of deferring the unshare to the next unshare(2),
> clone(2), or execve(2) call and would want to first confirm that doing
> so does not introduce any issues in the kernel or make it harder to
> integrate with existing container runtimes.
>
> Differences between this syscall interface and the selinuxfs interface
> that need discussion before moving forward:
>
> 1. The syscall interface does not currently check any Linux capability
> or DAC permissions, whereas the selinuxfs interface can only be set by
> uid-0 or CAP_DAC_OVERRIDE processes. We need to decide what if any
> capability or DAC check should apply to this syscall interface and if
> any, add the checks to either the LSM framework code or to the SELinux
> hook function.
>
> Pros: Checking a capability or DAC permissions prevents misuse of this
> interface by unprivileged processes, particularly on systems with
> policies that do not yet define any of the new SELinux permissions
> introduced for controlling this operation. This is a potential concern
> on Linux distributions that do not tightly coordinate kernel updates
> with policy updates (or where users may choose to deploy upstream
> kernels on their own), but not on Android.
>
> Cons: Checking a capability or DAC permissions requires any process
> that uses this facility to have the corresponding capability or
> permissions, which might otherwise be unnecessary and create
> additional risks. This is less likely if we use a capability already
> required by container runtimes and similar components that might
> leverage this facility for unsharing SELinux namespaces.
>
> 2. The syscall interface checks a new SELinux unshare_selinuxns
> permission in the process2 class between the task SID and itself,
> similar to other checks for setting process attributes. This means
> that:
> allow domain self:process2 *; -or-
> allow domain self:process2 ~anything-other-than-unshare_selinuxns; -or-
> allow domain self:process2 unshare_selinuxns;
> would allow a process to unshare its SELinux namespace.
>
> The selinuxfs interface checks a new unshare permission in the
> security class between the task SID and the security initial SID,
> likewise similar to other checks for setting selinuxfs attributes.
> This means that:
> allow domain security_t:security *; -or-
> allow domain security_t:security ~anything-other-than-unshare; -or-
> allow domain security_t:security unshare;
> would allow a process to unshare its SELinux namespace.
>
> Technically, the selinuxfs interface also currently requires open and
> write access to the selinuxfs node; hence:
> allow domain security_t:file { open write };
> is also required for the selinuxfs interface.
>
> We need to decide what we want the SELinux check(s) to be for the
> syscall and whether it should be more like the former (process
> attributes) or more like the latter (security policy settings). Note
> that the permission name itself is unimportant here and only differs
> because it seemed less evident in the process2 class that we are
> talking about a SELinux namespace otherwise.
>
> Regardless, either form of allow rule can be prohibited in policies
> via neverallow rules on systems that enforce their usage
> (e.g. Android, not necessarily on Linux distributions).
>
> 3. The selinuxfs interface currently offers more functionality than I
> have implemented here for the sycall interface, including:
>
> a) the ability to read the selinuxfs node to see if your namespace has
> been unshared, which should be easily implementable via
> lsm_get_self_attr(2). However, questions remain as to when that
> should return 1 versus 0 (currently returns 1 whenever your namespace
> is NOT the initial SELinux namespace, useful for the testsuite to
> detect it is in a child, but could instead be reset to 0 by a
> subsequent policy load to indicate completion of the setup of the
> namespace, thus hiding from child processes that they are in a child
> namespace once its policy has been loaded).
>
> b) the abilities to get and set the maximum number of SELinux
> namespaces (via a /sys/fs/selinux/maxns node) and to get and set the
> maximum depth for SELinux namespaces (via a /sys/fs/selinux/maxnsdepth
> node). These could be left in selinuxfs or migrated to some other LSM
> management APIs since they are global in scope, not per-process
> attributes.
>
> Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>
This looks like an appropriate use of lsm_set_self_attr() to me.
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> v2 fixes a typo (PROCESS->PROCESS2) and is now tested.
>
> include/uapi/linux/lsm.h | 1 +
> security/selinux/hooks.c | 8 ++++++++
> security/selinux/include/classmap.h | 4 +++-
> 3 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
> index 938593dfd5da..fb1b4a8aa639 100644
> --- a/include/uapi/linux/lsm.h
> +++ b/include/uapi/linux/lsm.h
> @@ -83,6 +83,7 @@ struct lsm_ctx {
> #define LSM_ATTR_KEYCREATE 103
> #define LSM_ATTR_PREV 104
> #define LSM_ATTR_SOCKCREATE 105
> +#define LSM_ATTR_UNSHARE 106
>
> /*
> * LSM_FLAG_XXX definitions identify special handling instructions
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index f48483383d6e..1e34a16b7954 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6816,6 +6816,10 @@ static int selinux_lsm_setattr(u64 attr, void *value, size_t size)
> error = avc_has_perm(state, mysid, mysid, SECCLASS_PROCESS,
> PROCESS__SETCURRENT, NULL);
> break;
> + case LSM_ATTR_UNSHARE:
> + error = avc_has_perm(state, mysid, mysid, SECCLASS_PROCESS2,
> + PROCESS2__UNSHARE_SELINUXNS, NULL);
> + break;
> default:
> error = -EOPNOTSUPP;
> break;
> @@ -6927,6 +6931,10 @@ static int selinux_lsm_setattr(u64 attr, void *value, size_t size)
> }
>
> tsec->sid = sid;
> + } else if (attr == LSM_ATTR_UNSHARE) {
> + error = selinux_state_create(new);
> + if (error)
> + goto abort_change;
> } else {
> error = -EINVAL;
> goto abort_change;
> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index be52ebb6b94a..07fe316308cd 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -60,7 +60,9 @@ const struct security_class_mapping secclass_map[] = {
> "siginh", "setrlimit", "rlimitinh", "dyntransition",
> "setcurrent", "execmem", "execstack", "execheap",
> "setkeycreate", "setsockcreate", "getrlimit", NULL } },
> - { "process2", { "nnp_transition", "nosuid_transition", NULL } },
> + { "process2",
> + { "nnp_transition", "nosuid_transition", "unshare_selinuxns",
> + NULL } },
> { "system",
> { "ipc_info", "syslog_read", "syslog_mod", "syslog_console",
> "module_request", "module_load", "firmware_load",
^ permalink raw reply
* [PATCH 0/4] tpm2-session: correct disperancies
From: Jarkko Sakkinen @ 2025-09-22 16:43 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM, open list
Correct various disperancies in tpm2-session implementation that set knots
on improving the feature.
Jarkko Sakkinen (4):
tpm: Use -EPERM as fallback error code in tpm_ret_to_err
tpm2-sessions: Remove unused parameter from tpm_buf_append_auth
tpm2-sessions: Remove unnecessary wrapper
keys, trusted: Remove redundant helper
drivers/char/tpm/tpm2-cmd.c | 16 +++-
drivers/char/tpm/tpm2-sessions.c | 5 +-
include/linux/tpm.h | 34 ++-------
security/keys/trusted-keys/trusted_tpm2.c | 89 +++++++----------------
4 files changed, 47 insertions(+), 97 deletions(-)
--
2.39.5
^ permalink raw reply
* [PATCH 1/4] tpm: Use -EPERM as fallback error code in tpm_ret_to_err
From: Jarkko Sakkinen @ 2025-09-22 16:43 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
James Bottomley, Mimi Zohar, Stefano Garzarella, open list,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM
In-Reply-To: <20250922164318.3540792-1-jarkko@kernel.org>
From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Using -EFAULT here was not the best idea for tpm_ret_to_err as the fallback
error code as it is no concise with trusted keys.
Change the fallback as -EPERM, process TPM_RC_HASH also in tpm_ret_to_err,
and by these changes make the helper applicable for trusted keys.
Fixes: 539fbab37881 ("tpm: Mask TPM RC in tpm2_start_auth_session()")
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
---
include/linux/tpm.h | 9 +++++---
security/keys/trusted-keys/trusted_tpm2.c | 26 ++++++-----------------
2 files changed, 13 insertions(+), 22 deletions(-)
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index dc0338a783f3..667d290789ca 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -449,13 +449,16 @@ static inline ssize_t tpm_ret_to_err(ssize_t ret)
if (ret < 0)
return ret;
- switch (tpm2_rc_value(ret)) {
- case TPM2_RC_SUCCESS:
+ if (!ret)
return 0;
+
+ switch (tpm2_rc_value(ret)) {
case TPM2_RC_SESSION_MEMORY:
return -ENOMEM;
+ case TPM2_RC_HASH:
+ return -EINVAL;
default:
- return -EFAULT;
+ return -EPERM;
}
}
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 024be262702f..e165b117bbca 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -348,25 +348,19 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
}
blob_len = tpm2_key_encode(payload, options, &buf.data[offset], blob_len);
+ if (blob_len < 0)
+ rc = blob_len;
out:
tpm_buf_destroy(&sized);
tpm_buf_destroy(&buf);
- if (rc > 0) {
- if (tpm2_rc_value(rc) == TPM2_RC_HASH)
- rc = -EINVAL;
- else
- rc = -EPERM;
- }
- if (blob_len < 0)
- rc = blob_len;
- else
+ if (!rc)
payload->blob_len = blob_len;
out_put:
tpm_put_ops(chip);
- return rc;
+ return tpm_ret_to_err(rc);
}
/**
@@ -468,10 +462,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
kfree(blob);
tpm_buf_destroy(&buf);
- if (rc > 0)
- rc = -EPERM;
-
- return rc;
+ return tpm_ret_to_err(rc);
}
/**
@@ -534,8 +525,6 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
tpm_buf_fill_hmac_session(chip, &buf);
rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
rc = tpm_buf_check_hmac_response(chip, &buf, rc);
- if (rc > 0)
- rc = -EPERM;
if (!rc) {
data_len = be16_to_cpup(
@@ -568,7 +557,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
out:
tpm_buf_destroy(&buf);
- return rc;
+ return tpm_ret_to_err(rc);
}
/**
@@ -600,6 +589,5 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
out:
tpm_put_ops(chip);
-
- return rc;
+ return tpm_ret_to_err(rc);
}
--
2.39.5
^ permalink raw reply related
* [PATCH 2/4] tpm2-sessions: Remove unused parameter from tpm_buf_append_auth
From: Jarkko Sakkinen @ 2025-09-22 16:43 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
Mimi Zohar, Roberto Sassu, open list, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM
In-Reply-To: <20250922164318.3540792-1-jarkko@kernel.org>
From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
In earlier bug fix, an extra parameter was by mistake to the function.
Fixes: 27184f8905ba ("tpm: Opt-in in disable PCR integrity protection")
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
---
drivers/char/tpm/tpm2-cmd.c | 2 +-
drivers/char/tpm/tpm2-sessions.c | 5 ++---
include/linux/tpm.h | 25 +------------------------
3 files changed, 4 insertions(+), 28 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 7d77f6fbc152..61a4daaef292 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -191,7 +191,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
} else {
tpm_buf_append_handle(chip, &buf, pcr_idx);
- tpm_buf_append_auth(chip, &buf, 0, NULL, 0);
+ tpm_buf_append_auth(chip, &buf, NULL, 0);
}
tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 6d03c224e6b2..13f019d1312a 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -266,7 +266,7 @@ void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
EXPORT_SYMBOL_GPL(tpm_buf_append_name);
void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf,
- u8 attributes, u8 *passphrase, int passphrase_len)
+ u8 *passphrase, int passphrase_len)
{
/* offset tells us where the sessions area begins */
int offset = buf->handles * 4 + TPM_HEADER_SIZE;
@@ -327,8 +327,7 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
#endif
if (!tpm2_chip_auth(chip)) {
- tpm_buf_append_auth(chip, buf, attributes, passphrase,
- passphrase_len);
+ tpm_buf_append_auth(chip, buf, passphrase, passphrase_len);
return;
}
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 667d290789ca..a8984d273c28 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -533,30 +533,7 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
u8 attributes, u8 *passphrase,
int passphraselen);
void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf,
- u8 attributes, u8 *passphrase, int passphraselen);
-static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip,
- struct tpm_buf *buf,
- u8 attributes,
- u8 *passphrase,
- int passphraselen)
-{
- struct tpm_header *head;
- int offset;
-
- if (tpm2_chip_auth(chip)) {
- tpm_buf_append_hmac_session(chip, buf, attributes, passphrase, passphraselen);
- } else {
- offset = buf->handles * 4 + TPM_HEADER_SIZE;
- head = (struct tpm_header *)buf->data;
-
- /*
- * If the only sessions are optional, the command tag must change to
- * TPM2_ST_NO_SESSIONS.
- */
- if (tpm_buf_length(buf) == offset)
- head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
- }
-}
+ u8 *passphrase, int passphraselen);
#ifdef CONFIG_TCG_TPM2_HMAC
--
2.39.5
^ permalink raw reply related
* [PATCH 3/4] tpm2-sessions: Remove unnecessary wrapper
From: Jarkko Sakkinen @ 2025-09-22 16:43 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
James Bottomley, Mimi Zohar, open list, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM
In-Reply-To: <20250922164318.3540792-1-jarkko@kernel.org>
From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Open code tpm_buf_append_hmac_session_opt() because it adds unnecessary
disperancy to the call sites (and reduces the amount of code).
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
---
drivers/char/tpm/tpm2-cmd.c | 14 +++++++++++---
security/keys/trusted-keys/trusted_tpm2.c | 12 ++++++++++--
2 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 61a4daaef292..0a795adbdc11 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -257,9 +257,17 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
do {
tpm_buf_reset(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
- tpm_buf_append_hmac_session_opt(chip, &buf, TPM2_SA_ENCRYPT
- | TPM2_SA_CONTINUE_SESSION,
- NULL, 0);
+ if (tpm2_chip_auth(chip)) {
+ tpm_buf_append_hmac_session(chip, &buf,
+ TPM2_SA_ENCRYPT |
+ TPM2_SA_CONTINUE_SESSION,
+ NULL, 0);
+ } else {
+ offset = buf.handles * 4 + TPM_HEADER_SIZE;
+ head = (struct tpm_header *)buf.data;
+ if (tpm_buf_length(&buf) == offset)
+ head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
+ }
tpm_buf_append_u16(&buf, num_bytes);
tpm_buf_fill_hmac_session(chip, &buf);
err = tpm_transmit_cmd(chip, &buf,
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index e165b117bbca..c414a7006d78 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -482,8 +482,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
struct trusted_key_options *options,
u32 blob_handle)
{
+ struct tpm_header *head;
struct tpm_buf buf;
u16 data_len;
+ int offset;
u8 *data;
int rc;
@@ -518,8 +520,14 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
tpm2_buf_append_auth(&buf, options->policyhandle,
NULL /* nonce */, 0, 0,
options->blobauth, options->blobauth_len);
- tpm_buf_append_hmac_session_opt(chip, &buf, TPM2_SA_ENCRYPT,
- NULL, 0);
+ if (tpm2_chip_auth(chip)) {
+ tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0);
+ } else {
+ offset = buf.handles * 4 + TPM_HEADER_SIZE;
+ head = (struct tpm_header *)buf.data;
+ if (tpm_buf_length(&buf) == offset)
+ head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
+ }
}
tpm_buf_fill_hmac_session(chip, &buf);
--
2.39.5
^ permalink raw reply related
* [PATCH 4/4] keys, trusted: Remove redundant helper
From: Jarkko Sakkinen @ 2025-09-22 16:43 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, David Howells, Jarkko Sakkinen, Paul Moore,
James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <20250922164318.3540792-1-jarkko@kernel.org>
From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
tpm2_buf_append_auth has only single call site and most of its parameters
are redundant. Open code it to the call site. Remove illegit FIXME comment
as there is no categorized bug and replace it with more sane comment about
implementation (i.e. "non-opionated inline comment").
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
---
security/keys/trusted-keys/trusted_tpm2.c | 51 ++++-------------------
1 file changed, 9 insertions(+), 42 deletions(-)
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index c414a7006d78..8e3b283a59b2 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -198,36 +198,6 @@ int tpm2_key_priv(void *context, size_t hdrlen,
return 0;
}
-/**
- * tpm2_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer.
- *
- * @buf: an allocated tpm_buf instance
- * @session_handle: session handle
- * @nonce: the session nonce, may be NULL if not used
- * @nonce_len: the session nonce length, may be 0 if not used
- * @attributes: the session attributes
- * @hmac: the session HMAC or password, may be NULL if not used
- * @hmac_len: the session HMAC or password length, maybe 0 if not used
- */
-static void tpm2_buf_append_auth(struct tpm_buf *buf, u32 session_handle,
- const u8 *nonce, u16 nonce_len,
- u8 attributes,
- const u8 *hmac, u16 hmac_len)
-{
- tpm_buf_append_u32(buf, 9 + nonce_len + hmac_len);
- tpm_buf_append_u32(buf, session_handle);
- tpm_buf_append_u16(buf, nonce_len);
-
- if (nonce && nonce_len)
- tpm_buf_append(buf, nonce, nonce_len);
-
- tpm_buf_append_u8(buf, attributes);
- tpm_buf_append_u16(buf, hmac_len);
-
- if (hmac && hmac_len)
- tpm_buf_append(buf, hmac, hmac_len);
-}
-
/**
* tpm2_seal_trusted() - seal the payload of a trusted key
*
@@ -507,19 +477,16 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
options->blobauth_len);
} else {
/*
- * FIXME: The policy session was generated outside the
- * kernel so we don't known the nonce and thus can't
- * calculate a HMAC on it. Therefore, the user can
- * only really use TPM2_PolicyPassword and we must
- * send down the plain text password, which could be
- * intercepted. We can still encrypt the returned
- * key, but that's small comfort since the interposer
- * could repeat our actions with the exfiltrated
- * password.
+ * The policy session is generated outside the kernel, and thus
+ * the password will end up being unencrypted on the bus, as
+ * HMAC nonce cannot be calculated for it.
*/
- tpm2_buf_append_auth(&buf, options->policyhandle,
- NULL /* nonce */, 0, 0,
- options->blobauth, options->blobauth_len);
+ tpm_buf_append_u32(&buf, 9 + options->blobauth_len);
+ tpm_buf_append_u32(&buf, options->policyhandle);
+ tpm_buf_append_u16(&buf, 0);
+ tpm_buf_append_u8(&buf, 0);
+ tpm_buf_append_u16(&buf, options->blobauth_len);
+ tpm_buf_append(&buf, options->blobauth, options->blobauth_len);
if (tpm2_chip_auth(chip)) {
tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0);
} else {
--
2.39.5
^ permalink raw reply related
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