* Re: [PATCH v4 1/4] arm64: Handle KCOV __init vs inline mismatches
From: Will Deacon @ 2025-07-29 8:10 UTC (permalink / raw)
To: Kees Cook
Cc: Arnd Bergmann, Ard Biesheuvel, Catalin Marinas, Jonathan Cameron,
Gavin Shan, Russell King (Oracle), James Morse, Oza Pawandeep,
Anshuman Khandual, linux-arm-kernel, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, H. Peter Anvin, Paolo Bonzini,
Mike Rapoport, Vitaly Kuznetsov, Henrique de Moraes Holschuh,
Hans de Goede, Ilpo Järvinen, Rafael J. Wysocki, Len Brown,
Masami Hiramatsu, Michal Wilczynski, Juergen Gross,
Andy Shevchenko, Kirill A. Shutemov, Roger Pau Monne,
David Woodhouse, Usama Arif, Guilherme G. Piccoli, Thomas Huth,
Brian Gerst, Marco Elver, Andrey Konovalov, Andrey Ryabinin,
Hou Wenlong, Andrew Morton, Masahiro Yamada,
Peter Zijlstra (Intel), Luis Chamberlain, Sami Tolvanen,
Christophe Leroy, Nathan Chancellor, Nicolas Schier,
Gustavo A. R. Silva, Andy Lutomirski, Baoquan He, Alexander Graf,
Changyuan Lyu, Paul Moore, James Morris, Serge E. Hallyn,
Nick Desaulniers, Bill Wendling, Justin Stitt, Jan Beulich,
Boqun Feng, Viresh Kumar, Paul E. McKenney, Bibo Mao,
linux-kernel, x86, kvm, ibm-acpi-devel, platform-driver-x86,
linux-acpi, linux-trace-kernel, linux-efi, linux-mm, kasan-dev,
linux-kbuild, linux-hardening, kexec, linux-security-module, llvm
In-Reply-To: <20250724055029.3623499-1-kees@kernel.org>
On Wed, Jul 23, 2025 at 10:50:25PM -0700, Kees Cook wrote:
> GCC appears to have kind of fragile inlining heuristics, in the
> sense that it can change whether or not it inlines something based on
> optimizations. It looks like the kcov instrumentation being added (or in
> this case, removed) from a function changes the optimization results,
> and some functions marked "inline" are _not_ inlined. In that case,
> we end up with __init code calling a function not marked __init, and we
> get the build warnings I'm trying to eliminate in the coming patch that
> adds __no_sanitize_coverage to __init functions:
>
> WARNING: modpost: vmlinux: section mismatch in reference: acpi_get_enable_method+0x1c (section: .text.unlikely) -> acpi_psci_present (section: .init.text)
>
> This problem is somewhat fragile (though using either __always_inline
> or __init will deterministically solve it), but we've tripped over
> this before with GCC and the solution has usually been to just use
> __always_inline and move on.
>
> For arm64 this requires forcing one ACPI function to be inlined with
> __always_inline.
>
> Signed-off-by: Kees Cook <kees@kernel.org>
> ---
> Cc: Will Deacon <will@kernel.org>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Gavin Shan <gshan@redhat.com>
> Cc: "Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>
> Cc: James Morse <james.morse@arm.com>
> Cc: Oza Pawandeep <quic_poza@quicinc.com>
> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: <linux-arm-kernel@lists.infradead.org>
> ---
> arch/arm64/include/asm/acpi.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> index a407f9cd549e..c07a58b96329 100644
> --- a/arch/arm64/include/asm/acpi.h
> +++ b/arch/arm64/include/asm/acpi.h
> @@ -150,7 +150,7 @@ acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor)
> {}
> #endif
>
> -static inline const char *acpi_get_enable_method(int cpu)
> +static __always_inline const char *acpi_get_enable_method(int cpu)
> {
> if (acpi_psci_present())
> return "psci";
Thanks for improving the commit message:
Acked-by: Will Deacon <will@kernel.org>
Will
^ permalink raw reply
* Re: [PATCH v4 2/4] x86: Handle KCOV __init vs inline mismatches
From: Mike Rapoport @ 2025-07-29 8:28 UTC (permalink / raw)
To: Kees Cook
Cc: Arnd Bergmann, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Paolo Bonzini, Ard Biesheuvel,
Vitaly Kuznetsov, Henrique de Moraes Holschuh, Hans de Goede,
Ilpo Järvinen, Rafael J. Wysocki, Len Brown,
Masami Hiramatsu, Michal Wilczynski, Juergen Gross,
Andy Shevchenko, Kirill A. Shutemov, Roger Pau Monne,
David Woodhouse, Usama Arif, Guilherme G. Piccoli, Thomas Huth,
Brian Gerst, kvm, ibm-acpi-devel, platform-driver-x86, linux-acpi,
linux-trace-kernel, linux-efi, linux-mm, Will Deacon,
Catalin Marinas, Jonathan Cameron, Gavin Shan,
Russell King (Oracle), James Morse, Oza Pawandeep,
Anshuman Khandual, Hans de Goede, Kirill A. Shutemov, Marco Elver,
Andrey Konovalov, Andrey Ryabinin, Hou Wenlong, Andrew Morton,
Masahiro Yamada, Peter Zijlstra (Intel), Luis Chamberlain,
Sami Tolvanen, Christophe Leroy, Nathan Chancellor,
Nicolas Schier, Gustavo A. R. Silva, Andy Lutomirski, Baoquan He,
Alexander Graf, Changyuan Lyu, Paul Moore, James Morris,
Serge E. Hallyn, Nick Desaulniers, Bill Wendling, Justin Stitt,
Jan Beulich, Boqun Feng, Viresh Kumar, Paul E. McKenney, Bibo Mao,
linux-kernel, linux-arm-kernel, kasan-dev, linux-kbuild,
linux-hardening, kexec, linux-security-module, llvm
In-Reply-To: <20250724055029.3623499-2-kees@kernel.org>
On Wed, Jul 23, 2025 at 10:50:26PM -0700, Kees Cook wrote:
> GCC appears to have kind of fragile inlining heuristics, in the
> sense that it can change whether or not it inlines something based on
> optimizations. It looks like the kcov instrumentation being added (or in
> this case, removed) from a function changes the optimization results,
> and some functions marked "inline" are _not_ inlined. In that case,
> we end up with __init code calling a function not marked __init, and we
> get the build warnings I'm trying to eliminate in the coming patch that
> adds __no_sanitize_coverage to __init functions:
>
> WARNING: modpost: vmlinux: section mismatch in reference: xbc_exit+0x8 (section: .text.unlikely) -> _xbc_exit (section: .init.text)
> WARNING: modpost: vmlinux: section mismatch in reference: real_mode_size_needed+0x15 (section: .text.unlikely) -> real_mode_blob_end (section: .init.data)
> WARNING: modpost: vmlinux: section mismatch in reference: __set_percpu_decrypted+0x16 (section: .text.unlikely) -> early_set_memory_decrypted (section: .init.text)
> WARNING: modpost: vmlinux: section mismatch in reference: memblock_alloc_from+0x26 (section: .text.unlikely) -> memblock_alloc_try_nid (section: .init.text)
> WARNING: modpost: vmlinux: section mismatch in reference: acpi_arch_set_root_pointer+0xc (section: .text.unlikely) -> x86_init (section: .init.data)
> WARNING: modpost: vmlinux: section mismatch in reference: acpi_arch_get_root_pointer+0x8 (section: .text.unlikely) -> x86_init (section: .init.data)
> WARNING: modpost: vmlinux: section mismatch in reference: efi_config_table_is_usable+0x16 (section: .text.unlikely) -> xen_efi_config_table_is_usable (section: .init.text)
>
> This problem is somewhat fragile (though using either __always_inline
> or __init will deterministically solve it), but we've tripped over
> this before with GCC and the solution has usually been to just use
> __always_inline and move on.
>
> For x86 this means forcing several functions to be inline with
> __always_inline.
>
> Signed-off-by: Kees Cook <kees@kernel.org>
For memblock bit:
Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
--
Sincerely yours,
Mike.
^ permalink raw reply
* [PATCH] lsm: simplify security_inode_copy_up_xattr()
From: Tianjia Zhang @ 2025-07-29 9:09 UTC (permalink / raw)
To: Paul Moore, James Morris, Serge E. Hallyn, linux-security-module,
linux-kernel
Cc: Tianjia Zhang
The implementation of function security_inode_copy_up_xattr can be
simplified to directly call call_int_hook().
Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
---
security/security.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/security/security.c b/security/security.c
index 596d41818577..a5c2e5a8009f 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2774,13 +2774,7 @@ EXPORT_SYMBOL(security_inode_copy_up);
*/
int security_inode_copy_up_xattr(struct dentry *src, const char *name)
{
- int rc;
-
- rc = call_int_hook(inode_copy_up_xattr, src, name);
- if (rc != LSM_RET_DEFAULT(inode_copy_up_xattr))
- return rc;
-
- return LSM_RET_DEFAULT(inode_copy_up_xattr);
+ return call_int_hook(inode_copy_up_xattr, src, name);
}
EXPORT_SYMBOL(security_inode_copy_up_xattr);
--
2.39.5 (Apple Git-154)
^ permalink raw reply related
* Re: [PATCH v4 2/4] x86: Handle KCOV __init vs inline mismatches
From: Arnd Bergmann @ 2025-07-29 9:34 UTC (permalink / raw)
To: Kees Cook
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Paolo Bonzini, Mike Rapoport, Ard Biesheuvel,
Vitaly Kuznetsov, Henrique de Moraes Holschuh, Hans de Goede,
Ilpo Järvinen, Rafael J . Wysocki, Len Brown,
Masami Hiramatsu, Michal Wilczynski, Juergen Gross,
Andy Shevchenko, Kirill A. Shutemov, Roger Pau Monne,
David Woodhouse, Usama Arif, Guilherme G. Piccoli, Thomas Huth,
Brian Gerst, kvm, ibm-acpi-devel, platform-driver-x86, linux-acpi,
linux-trace-kernel, linux-efi, linux-mm, Will Deacon,
Catalin Marinas, Jonathan Cameron, Gavin Shan, Russell King,
James Morse, Oza Pawandeep, Anshuman Khandual, Hans de Goede,
Kirill A. Shutemov, Marco Elver, Andrey Konovalov,
Andrey Ryabinin, Hou Wenlong, Andrew Morton, Masahiro Yamada,
Peter Zijlstra, Luis Chamberlain, Sami Tolvanen, Christophe Leroy,
Nathan Chancellor, Nicolas Schier, Gustavo A. R. Silva,
Andy Lutomirski, Baoquan He, Alexander Graf, Changyuan Lyu,
Paul Moore, James Morris, Serge E. Hallyn, Nick Desaulniers,
Bill Wendling, Justin Stitt, Jan Beulich, Boqun Feng,
Viresh Kumar, Paul E. McKenney, Bibo Mao, linux-kernel,
linux-arm-kernel, kasan-dev, linux-kbuild, linux-hardening, kexec,
linux-security-module, llvm
In-Reply-To: <20250724055029.3623499-2-kees@kernel.org>
On Thu, Jul 24, 2025, at 07:50, Kees Cook wrote:
> GCC appears to have kind of fragile inlining heuristics, in the
> sense that it can change whether or not it inlines something based on
> optimizations. It looks like the kcov instrumentation being added (or in
> this case, removed) from a function changes the optimization results,
> and some functions marked "inline" are _not_ inlined. In that case,
> we end up with __init code calling a function not marked __init, and we
> get the build warnings I'm trying to eliminate in the coming patch that
> adds __no_sanitize_coverage to __init functions:
>
> WARNING: modpost: vmlinux: section mismatch in reference: xbc_exit+0x8
> (section: .text.unlikely) -> _xbc_exit (section: .init.text)
> WARNING: modpost: vmlinux: section mismatch in reference:
> real_mode_size_needed+0x15 (section: .text.unlikely) ->
> real_mode_blob_end (section: .init.data)
> WARNING: modpost: vmlinux: section mismatch in reference:
> __set_percpu_decrypted+0x16 (section: .text.unlikely) ->
> early_set_memory_decrypted (section: .init.text)
> WARNING: modpost: vmlinux: section mismatch in reference:
> memblock_alloc_from+0x26 (section: .text.unlikely) ->
> memblock_alloc_try_nid (section: .init.text)
> WARNING: modpost: vmlinux: section mismatch in reference:
> acpi_arch_set_root_pointer+0xc (section: .text.unlikely) -> x86_init
> (section: .init.data)
> WARNING: modpost: vmlinux: section mismatch in reference:
> acpi_arch_get_root_pointer+0x8 (section: .text.unlikely) -> x86_init
> (section: .init.data)
> WARNING: modpost: vmlinux: section mismatch in reference:
> efi_config_table_is_usable+0x16 (section: .text.unlikely) ->
> xen_efi_config_table_is_usable (section: .init.text)
>
> This problem is somewhat fragile (though using either __always_inline
> or __init will deterministically solve it), but we've tripped over
> this before with GCC and the solution has usually been to just use
> __always_inline and move on.
>
> For x86 this means forcing several functions to be inline with
> __always_inline.
>
> Signed-off-by: Kees Cook <kees@kernel.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
In my randconfig tests, I got these ones as well:
WARNING: modpost: vmlinux: section mismatch in reference: early_page_ext_enabled+0x14 (section: .text.unlikely) -> early_
page_ext (section: .init.data)
x86_64-linux-ld: lm75.c:(.text+0xd25): undefined reference to `i3c_device_do_priv_xfers'
And one more with a private patch of mine.
These are the fixups that make it build for arm/arm64/x86
randconfigs for me, so you could fold them as well in
as well. I have already sent the i3c patch for upstream
but not the page_ext.h patch.
--- a/include/linux/page_ext.h
+++ b/include/linux/page_ext.h
@@ -57,7 +57,7 @@ extern bool early_page_ext;
extern unsigned long page_ext_size;
extern void pgdat_page_ext_init(struct pglist_data *pgdat);
-static inline bool early_page_ext_enabled(void)
+static __always_inline bool early_page_ext_enabled(void)
{
return early_page_ext;
}
@@ -189,7 +189,7 @@ static inline struct page_ext *page_ext_iter_get(const struct page_ext_iter *ite
#else /* !CONFIG_PAGE_EXTENSION */
struct page_ext;
-static inline bool early_page_ext_enabled(void)
+static __always_inline bool early_page_ext_enabled(void)
{
return false;
}
--- a/include/linux/i3c/device.h
+++ b/include/linux/i3c/device.h
@@ -245,7 +245,7 @@ void i3c_driver_unregister(struct i3c_driver *drv);
*
* Return: 0 if both registrations succeeds, a negative error code otherwise.
*/
-static inline int i3c_i2c_driver_register(struct i3c_driver *i3cdrv,
+static __always_inline int i3c_i2c_driver_register(struct i3c_driver *i3cdrv,
struct i2c_driver *i2cdrv)
{
int ret;
@@ -270,7 +270,7 @@ static inline int i3c_i2c_driver_register(struct i3c_driver *i3cdrv,
* Note that when CONFIG_I3C is not enabled, this function only unregisters the
* @i2cdrv.
*/
-static inline void i3c_i2c_driver_unregister(struct i3c_driver *i3cdrv,
+static __always_inline void i3c_i2c_driver_unregister(struct i3c_driver *i3cdrv,
struct i2c_driver *i2cdrv)
{
if (IS_ENABLED(CONFIG_I3C))
As I understand, the underlying problem is less gcc inlining
being fragile, but more that gcc does not inline functions
when they have different __no_sanitize_coverage attributes.
Arnd
^ permalink raw reply
* Re: [PATCH v2 0/3] Allow individual features to be locked down
From: Nicolas Bouchinet @ 2025-07-29 12:16 UTC (permalink / raw)
To: Nikolay Borisov
Cc: linux-security-module, linux-kernel, paul, serge, jmorris,
dan.j.williams
In-Reply-To: <20250728111517.134116-1-nik.borisov@suse.com>
Hi Nikolay,
Thanks for you patch.
Quoting Kees [1], Lockdown is "about creating a bright line between
uid-0 and ring-0".
Having a bitmap enabled Lockdown would mean that Lockdown reasons could
be activated independently. I fear this would lead to a false sense of
security, locking one reason alone often permits Lockdown restrictions
bypass. i.e enforcing kernel module signature verification but not
blocking accesses to `/dev/{k,}mem` or authorizing gkdb which can be
used to disable the module signature enforcement.
If one wants to restrict accesses to `/dev/mem`,
`security_locked_down(LOCKDOWN_DEV_MEM)` should be sufficient.
My understanding of your problem is that this locks too much for your
usecase and you want to restrict reasons of Lockdown independently in
case it has not been enabled in "integrity" mode by default ?
Can you elaborate more on the usecases for COCO ?
[1]: https://lore.kernel.org/all/CAGXu5j+UWQWDacMvvRCke3xUOb7uTkxn=WaHzG4kJTKWh-6tAA@mail.gmail.com/
Best regards,
Nicolas
---
On Mon, Jul 28, 2025 at 02:15:14PM +0300, Nikolay Borisov wrote:
> This simple change allows usecases where someone might want to lock only specific
> feature at a finer granularity than integrity/confidentiality levels allows.
> The first likely user of this is the CoCo subsystem where certain features will be
> disabled.
>
> Changes since v1:
> * Added Patch 3 to incoroporate Serge's hardening suggestion
>
> Nikolay Borisov (3):
> lockdown: Switch implementation to using bitmap
> lockdown/kunit: Introduce kunit tests
> lockdown: Use snprintf in lockdown_read
>
> security/lockdown/Kconfig | 5 +++
> security/lockdown/Makefile | 1 +
> security/lockdown/lockdown.c | 36 +++++++++++++++------
> security/lockdown/lockdown_test.c | 54 +++++++++++++++++++++++++++++++
> 4 files changed, 86 insertions(+), 10 deletions(-)
> create mode 100644 security/lockdown/lockdown_test.c
>
> --
> 2.34.1
>
^ permalink raw reply
* Re: [PATCH v2 0/3] Allow individual features to be locked down
From: Nikolay Borisov @ 2025-07-29 12:25 UTC (permalink / raw)
To: Nicolas Bouchinet
Cc: linux-security-module, linux-kernel, paul, serge, jmorris,
dan.j.williams
In-Reply-To: <kl4rvgnupxnz4zrwlofrawdfy23tj2ylp5s3wovnsjxkr6tbrt@x5s3avqo2e7t>
On 29.07.25 г. 15:16 ч., Nicolas Bouchinet wrote:
> Hi Nikolay,
>
> Thanks for you patch.
>
> Quoting Kees [1], Lockdown is "about creating a bright line between
> uid-0 and ring-0".
>
> Having a bitmap enabled Lockdown would mean that Lockdown reasons could
> be activated independently. I fear this would lead to a false sense of
> security, locking one reason alone often permits Lockdown restrictions
> bypass. i.e enforcing kernel module signature verification but not
> blocking accesses to `/dev/{k,}mem` or authorizing gkdb which can be
> used to disable the module signature enforcement.
>
> If one wants to restrict accesses to `/dev/mem`,
> `security_locked_down(LOCKDOWN_DEV_MEM)` should be sufficient.
>
> My understanding of your problem is that this locks too much for your
> usecase and you want to restrict reasons of Lockdown independently in
> case it has not been enabled in "integrity" mode by default ?
>
> Can you elaborate more on the usecases for COCO ?
Initially this patchset was supposed to allow us selectively disable
/dev/iomem access in a CoCo context [0]. As evident from Dan's initial
response that point pretty much became moot as the issue was fixed in a
different way. However, later [1] he came back and said that actually
this patch could be useful in a similar context. So This v2 is
essentially following up on that.
[0]
https://lore.kernel.org/all/67f69600ed221_71fe2946f@dwillia2-xfh.jf.intel.com.notmuch/
[1]
https://lore.kernel.org/all/68226ad551afd_29032945b@dwillia2-xfh.jf.intel.com.notmuch/
<snip>
^ permalink raw reply
* Re: [PATCH] lsm: simplify security_inode_copy_up_xattr()
From: Casey Schaufler @ 2025-07-29 14:43 UTC (permalink / raw)
To: Tianjia Zhang, Paul Moore, James Morris, Serge E. Hallyn,
linux-security-module, linux-kernel, Casey Schaufler
In-Reply-To: <20250729090933.94942-1-tianjia.zhang@linux.alibaba.com>
On 7/29/2025 2:09 AM, Tianjia Zhang wrote:
> The implementation of function security_inode_copy_up_xattr can be
> simplified to directly call call_int_hook().
>
> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> ---
> security/security.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/security/security.c b/security/security.c
> index 596d41818577..a5c2e5a8009f 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2774,13 +2774,7 @@ EXPORT_SYMBOL(security_inode_copy_up);
> */
> int security_inode_copy_up_xattr(struct dentry *src, const char *name)
> {
> - int rc;
> -
> - rc = call_int_hook(inode_copy_up_xattr, src, name);
> - if (rc != LSM_RET_DEFAULT(inode_copy_up_xattr))
> - return rc;
> -
> - return LSM_RET_DEFAULT(inode_copy_up_xattr);
> + return call_int_hook(inode_copy_up_xattr, src, name);
Both the existing code and the proposed change are incorrect.
If two LSMs supply the hook, and the first does not recognize
the attribute, the second, which might recognize the attribute,
will not be called. As SELinux and EVM both supply this hook
there may be a real problem here.
> }
> EXPORT_SYMBOL(security_inode_copy_up_xattr);
>
^ permalink raw reply
* Re: [PATCH] lsm: simplify security_inode_copy_up_xattr()
From: Paul Moore @ 2025-07-29 15:09 UTC (permalink / raw)
To: Casey Schaufler
Cc: Tianjia Zhang, James Morris, Serge E. Hallyn,
linux-security-module, linux-kernel
In-Reply-To: <e81ba8e7-8938-4b76-ae7b-bfee6021aeac@schaufler-ca.com>
On Tue, Jul 29, 2025 at 10:43 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 7/29/2025 2:09 AM, Tianjia Zhang wrote:
> > The implementation of function security_inode_copy_up_xattr can be
> > simplified to directly call call_int_hook().
> >
> > Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> > ---
> > security/security.c | 8 +-------
> > 1 file changed, 1 insertion(+), 7 deletions(-)
> >
> > diff --git a/security/security.c b/security/security.c
> > index 596d41818577..a5c2e5a8009f 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -2774,13 +2774,7 @@ EXPORT_SYMBOL(security_inode_copy_up);
> > */
> > int security_inode_copy_up_xattr(struct dentry *src, const char *name)
> > {
> > - int rc;
> > -
> > - rc = call_int_hook(inode_copy_up_xattr, src, name);
> > - if (rc != LSM_RET_DEFAULT(inode_copy_up_xattr))
> > - return rc;
> > -
> > - return LSM_RET_DEFAULT(inode_copy_up_xattr);
> > + return call_int_hook(inode_copy_up_xattr, src, name);
>
> Both the existing code and the proposed change are incorrect.
> If two LSMs supply the hook, and the first does not recognize
> the attribute, the second, which might recognize the attribute,
> will not be called. As SELinux and EVM both supply this hook
> there may be a real problem here.
It appears that Smack also supplies a inode_copy_up_xattr() callback
via smack_inode_copy_up_xattr().
Someone should double check this logic, but looking at it very
quickly, it would appear that LSM framework should run the individual
LSM callbacks in order so long as they return -EOPNOTSUPP, if they do
not return -EOPNOTSUPP, the return value should be returned to the
caller without executing any further callbacks. As a default return
value, or if all of the LSM callbacks succeed with -EOPNOTSUPP, the
hook should return -EOPNOTSUPP.
Tianjia Zhang, would you be able to develop and test a patch for this?
--
paul-moore.com
^ permalink raw reply
* [GIT PULL] capabilities update for v6.17-rc1
From: Serge E. Hallyn @ 2025-07-29 19:23 UTC (permalink / raw)
To: torvalds, linux-security-module, Linux Kernel Mailing List,
Ariel Otilibili, Andrew G. Morgan, Paul Moore, Eric W. Biederman,
Max Kellermann, Jann Horn, Kees Cook
The following changes since commit 19272b37aa4f83ca52bdf9c16d5d81bdd1354494:
Linux 6.16-rc1 (2025-06-08 13:44:43 -0700)
are available in the Git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/sergeh/linux.git tags/caps-pr-20250729
for you to fetch changes up to cdd73b1666079a73d061396f361df55d59fe96e6:
uapi: fix broken link in linux/capability.h (2025-07-04 19:21:53 -0500)
----------------------------------------------------------------
Capabilities update for 6.17
This branch contains two patches:
cdd73b1666079a73d061396f361df55d59fe96e6 uapi: fix broken link in linux/capability.h
This updates documentation in capability.h.
337490f0007f910968f828e46501db3091b1a4f8 exec: Correct the permission check for unsafe exec
This is not a trivial patch, but fixes a real problem where during
exec, different effective and real credentials were assumed to mean
changed credentials, making it impossible in the no-new-privs case
to keep different uid and euid.
These are available at:
git://git.kernel.org/pub/scm/linux/kernel/git/sergeh/linux.git #caps-pr-20250729
on top of commit 19272b37aa4f83ca52bdf9c16d5d81bdd1354494 (tag: v6.16-rc1)
----------------------------------------------------------------
Ariel Otilibili (1):
uapi: fix broken link in linux/capability.h
Eric W. Biederman (1):
exec: Correct the permission check for unsafe exec
include/uapi/linux/capability.h | 5 +++--
security/commoncap.c | 20 ++++++++------------
2 files changed, 11 insertions(+), 14 deletions(-)
^ permalink raw reply
* Re: [PATCH v2 02/13] bpf: Implement exclusive map creation
From: Fan Wu @ 2025-07-29 22:59 UTC (permalink / raw)
To: KP Singh
Cc: bpf, linux-security-module, bboscaccy, paul, kys, ast, daniel,
andrii
In-Reply-To: <20250721211958.1881379-3-kpsingh@kernel.org>
On Mon, Jul 21, 2025 at 2:35 PM KP Singh <kpsingh@kernel.org> wrote:
>
> Exclusive maps allow maps to only be accessed by program with a
> program with a matching hash which is specified in the excl_prog_hash
> attr.
>
> For the signing use-case, this allows the trusted loader program
> to load the map and verify the integrity
>
> Signed-off-by: KP Singh <kpsingh@kernel.org>
> ---
> include/linux/bpf.h | 1 +
> include/uapi/linux/bpf.h | 2 ++
> kernel/bpf/syscall.c | 32 ++++++++++++++++++++++++++++----
> kernel/bpf/verifier.c | 6 ++++++
> tools/include/uapi/linux/bpf.h | 2 ++
> 5 files changed, 39 insertions(+), 4 deletions(-)
>
...
> -static int map_create(union bpf_attr *attr, bool kernel)
> +static int map_create(union bpf_attr *attr, bpfptr_t uattr)
> {
> const struct bpf_map_ops *ops;
> struct bpf_token *token = NULL;
> @@ -1527,7 +1528,30 @@ static int map_create(union bpf_attr *attr, bool kernel)
> attr->btf_vmlinux_value_type_id;
> }
>
> - err = security_bpf_map_create(map, attr, token, kernel);
> + if (attr->excl_prog_hash) {
> + bpfptr_t uprog_hash = make_bpfptr(attr->excl_prog_hash, uattr.is_kernel);
> +
> + map->excl_prog_sha = kzalloc(SHA256_DIGEST_SIZE, GFP_KERNEL);
> + if (!map->excl_prog_sha) {
> + err = -ENOMEM;
> + goto free_map;
> + }
> +
> + if (attr->excl_prog_hash_size != SHA256_DIGEST_SIZE) {
> + err = -EINVAL;
> + goto free_map;
> + }
Nit: Maybe check the size first to avoid unncessary kzalloc?
-Fan
> +
> + if (copy_from_bpfptr(map->excl_prog_sha, uprog_hash,
> + SHA256_DIGEST_SIZE)) {
> + err = -EFAULT;
> + goto free_map;
> + }
> + } else if (attr->excl_prog_hash_size) {
> + return -EINVAL;
> + }
> +
> + err = security_bpf_map_create(map, attr, token, uattr.is_kernel);
> if (err)
> goto free_map_sec;
>
^ permalink raw reply
* Re: [PATCH v2 2/3] lockdown/kunit: Introduce kunit tests
From: Philip Li @ 2025-07-29 23:28 UTC (permalink / raw)
To: Nikolay Borisov
Cc: kernel test robot, linux-security-module, llvm, oe-kbuild-all,
linux-kernel, paul, serge, jmorris, dan.j.williams
In-Reply-To: <c2754ac3-28d0-4c6c-bd2d-0dcc9ca66b77@suse.com>
On Tue, Jul 29, 2025 at 10:46:48AM +0300, Nikolay Borisov wrote:
>
>
> On 29.07.25 г. 1:04 ч., kernel test robot wrote:
> > Hi Nikolay,
> >
> > kernel test robot noticed the following build warnings:
> >
> > [auto build test WARNING on linus/master]
> > [also build test WARNING on v6.16 next-20250728]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> >
> > url: https://github.com/intel-lab-lkp/linux/commits/Nikolay-Borisov/lockdown-Switch-implementation-to-using-bitmap/20250728-191807
> > base: linus/master
> > patch link: https://lore.kernel.org/r/20250728111517.134116-3-nik.borisov%40suse.com
> > patch subject: [PATCH v2 2/3] lockdown/kunit: Introduce kunit tests
> > config: arm-randconfig-004-20250729 (https://download.01.org/0day-ci/archive/20250729/202507290540.9IANrMED-lkp@intel.com/config)
> > compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 1b4db78d2eaa070b3f364a2d2b2b826a5439b892)
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250729/202507290540.9IANrMED-lkp@intel.com/reproduce)
> >
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/202507290540.9IANrMED-lkp@intel.com/
> >
> > All warnings (new ones prefixed by >>):
> >
> > > > security/lockdown/lockdown.c:31:5: warning: no previous prototype for function 'lock_kernel_down' [-Wmissing-prototypes]
> > 31 | int lock_kernel_down(const char *where, enum lockdown_reason level)
> > | ^
> > security/lockdown/lockdown.c:31:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
> > 31 | int lock_kernel_down(const char *where, enum lockdown_reason level)
> > | ^
> > | static
> > 1 warning generated.
>
>
> That's a false positive, since the function is exported only for KUNIT case,
> what's the correct way to make testbot ignore it ?
thanks for the info, i will configure the bot to ignore this firstly, and
later to check whether there's any pattern for more general ignore. Sorry
for the noise.
> >
> >
> > vim +/lock_kernel_down +31 security/lockdown/lockdown.c
> >
> > 20
> > 21 static const enum lockdown_reason lockdown_levels[] = {LOCKDOWN_NONE,
> > 22 LOCKDOWN_INTEGRITY_MAX,
> > 23 LOCKDOWN_CONFIDENTIALITY_MAX};
> > 24
> > 25 /*
> > 26 * Put the kernel into lock-down mode.
> > 27 */
> > 28 #if !IS_ENABLED(CONFIG_KUNIT)
> > 29 static
> > 30 #endif
> > > 31 int lock_kernel_down(const char *where, enum lockdown_reason level)
> > 32 {
> > 33
> > 34 if (level > LOCKDOWN_CONFIDENTIALITY_MAX)
> > 35 return -EINVAL;
> > 36
> > 37 if (level == LOCKDOWN_INTEGRITY_MAX || level == LOCKDOWN_CONFIDENTIALITY_MAX)
> > 38 bitmap_set(kernel_locked_down, 1, level);
> > 39 else
> > 40 bitmap_set(kernel_locked_down, level, 1);
> > 41
> > 42 pr_notice("Kernel is locked down from %s; see man kernel_lockdown.7\n",
> > 43 where);
> > 44 return 0;
> > 45 }
> > 46
> >
>
^ permalink raw reply
* Re: [PATCH] lsm: simplify security_inode_copy_up_xattr()
From: tianjia.zhang @ 2025-07-30 9:25 UTC (permalink / raw)
To: Paul Moore, Casey Schaufler
Cc: James Morris, Serge E. Hallyn, linux-security-module,
linux-kernel
In-Reply-To: <CAHC9VhQAVvvXUoFu7xnh0uBhmvgYinP=AhiC4y17JJ02M9s5Nw@mail.gmail.com>
On 7/29/25 11:09 PM, Paul Moore wrote:
> On Tue, Jul 29, 2025 at 10:43 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 7/29/2025 2:09 AM, Tianjia Zhang wrote:
>>> The implementation of function security_inode_copy_up_xattr can be
>>> simplified to directly call call_int_hook().
>>>
>>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
>>> ---
>>> security/security.c | 8 +-------
>>> 1 file changed, 1 insertion(+), 7 deletions(-)
>>>
>>> diff --git a/security/security.c b/security/security.c
>>> index 596d41818577..a5c2e5a8009f 100644
>>> --- a/security/security.c
>>> +++ b/security/security.c
>>> @@ -2774,13 +2774,7 @@ EXPORT_SYMBOL(security_inode_copy_up);
>>> */
>>> int security_inode_copy_up_xattr(struct dentry *src, const char *name)
>>> {
>>> - int rc;
>>> -
>>> - rc = call_int_hook(inode_copy_up_xattr, src, name);
>>> - if (rc != LSM_RET_DEFAULT(inode_copy_up_xattr))
>>> - return rc;
>>> -
>>> - return LSM_RET_DEFAULT(inode_copy_up_xattr);
>>> + return call_int_hook(inode_copy_up_xattr, src, name);
>>
>> Both the existing code and the proposed change are incorrect.
>> If two LSMs supply the hook, and the first does not recognize
>> the attribute, the second, which might recognize the attribute,
>> will not be called. As SELinux and EVM both supply this hook
>> there may be a real problem here.
>
> It appears that Smack also supplies a inode_copy_up_xattr() callback
> via smack_inode_copy_up_xattr().
>
> Someone should double check this logic, but looking at it very
> quickly, it would appear that LSM framework should run the individual
> LSM callbacks in order so long as they return -EOPNOTSUPP, if they do
> not return -EOPNOTSUPP, the return value should be returned to the
> caller without executing any further callbacks. As a default return
> value, or if all of the LSM callbacks succeed with -EOPNOTSUPP, the
> hook should return -EOPNOTSUPP.
>
> Tianjia Zhang, would you be able to develop and test a patch for this?
>
Yes, I will submit a new patch to try to fix this issue. Thanks for your
suggestion.
Cheers,
Tianjia
^ permalink raw reply
* Re: [PATCH -next] apparmor: Remove the unused variable rules
From: John Johansen @ 2025-07-30 9:55 UTC (permalink / raw)
To: Jiapeng Chong
Cc: paul, jmorris, serge, apparmor, linux-security-module,
linux-kernel, Abaci Robot
In-Reply-To: <20250725095252.2087274-1-jiapeng.chong@linux.alibaba.com>
On 7/25/25 02:52, Jiapeng Chong wrote:
> Variable rules is not effectively used, so delete it.
>
> security/apparmor/lsm.c:182:23: warning: variable ‘rules’ set but not used.
>
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=22942
> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
Acked-by: John Johansen <john.johansen@canonical.com>
I have pulled this in
> ---
> security/apparmor/lsm.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 5b1b5ac831e8..182a0e55802e 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -179,10 +179,8 @@ static int apparmor_capget(const struct task_struct *target, kernel_cap_t *effec
> struct label_it i;
>
> label_for_each_confined(i, label, profile) {
> - struct aa_ruleset *rules;
> kernel_cap_t allowed;
>
> - rules = profile->label.rules[0];
> allowed = aa_profile_capget(profile);
> *effective = cap_intersect(*effective, allowed);
> *permitted = cap_intersect(*permitted, allowed);
^ permalink raw reply
* Re: [syzbot] [apparmor?] linux-next test error: WARNING in apparmor_unix_stream_connect
From: John Johansen @ 2025-07-30 10:17 UTC (permalink / raw)
To: Alexander Potapenko
Cc: apparmor, jmorris, john, linux-kernel, linux-next,
linux-security-module, paul, serge, sfr, syzkaller-bugs
In-Reply-To: <CAG_fn=WSae7yjaHh=_iUc7eFALHX1vLQFMw8ryfas4-ijgFTiQ@mail.gmail.com>
On 7/28/25 01:16, Alexander Potapenko wrote:
> On Mon, Jul 21, 2025 at 11:35 AM syzbot
> <syzbot+cd38ee04bcb3866b0c6d@syzkaller.appspotmail.com> wrote:
>>
>> Hello,
>>
>> syzbot found the following issue on:
>
> John, do you have an idea what's going on?
> This is pretty likely to be related to your "apparmor: make sure unix
> socket labeling is correctly updated." patch.
yeah it is being caused by that patch. Specifically it introduces using
security_sk_alloc() to make sure the sk is given a default label which
makes this check now incorrect the fix is just to drop the check. I have
added the fix with the syzbot ref
^ permalink raw reply
* Re: -Wformat-invalid-specifier after 88fec3526e84 in -next
From: John Johansen @ 2025-07-30 10:20 UTC (permalink / raw)
To: Nathan Chancellor; +Cc: apparmor, linux-security-module, llvm
In-Reply-To: <20250721231041.GA1015606@ax162>
On 7/21/25 16:10, Nathan Chancellor wrote:
> Hi John,
>
> After commit 88fec3526e84 ("apparmor: make sure unix socket labeling is
> correctly updated.") in -next, I am seeing some warnings from clang when
> building arm64 allmodconfig with LTO enabled. This can be more simply
> reproduced on top of defconfig:
>
> $ make -skj"$(nproc)" ARCH=arm64 LLVM=1 mrproper defconfig
>
> $ scripts/config \
> -d LTO_NONE \
> -e LTO_CLANG_THIN \
> -e SECURITY_APPARMOR \
> -e SECURITY_APPARMOR_DEBUG
>
> $ make -skj"$(nproc)" ARCH=arm64 LLVM=1 olddefconfig security/apparmor/lsm.o
> security/apparmor/lsm.c:1206:2: warning: invalid conversion specifier '0' [-Wformat-invalid-specifier]
> 1206 | AA_BUG(rcu_access_pointer(new_ctx->label));
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> security/apparmor/include/lib.h:56:3: note: expanded from macro 'AA_BUG'
> 56 | AA_BUG_FMT((X), "" args); \
> | ^~~~~~~~~~~~~~~~~~~~~~~~
> security/apparmor/include/lib.h:61:34: note: expanded from macro 'AA_BUG_FMT'
> 61 | WARN((X), "AppArmor WARN %s: (" #X "): " fmt, __func__, ##args)
> | ^~
> <scratch space>:2:1144: note: expanded from here
> 2 | "(({ typeof(*(new_ctx->label)) *__UNIQUE_ID_rcu1155 = (typeof(*(new_ctx->label)) *)({ do { __attribute__((__noreturn__)) extern void __compiletime_assert_1156(void) __attribute__((__error__(\"Unsupported access size for {READ,WRITE}_ONCE().\"))); ..."
> ...
> include/asm-generic/bug.h:134:29: note: expanded from macro 'WARN'
> 134 | __WARN_printf(TAINT_WARN, format); \
> | ^~~~~~
> include/asm-generic/bug.h:106:17: note: expanded from macro '__WARN_printf'
> 106 | __warn_printk(arg); \
> | ^~~
>
> Ultimately, rcu_access_pointer() expands to __READ_ONCE(), which arm64
> specifically defines for CONFIG_LTO using some inline asm expressions,
> see commit e35123d83ee3 ("arm64: lto: Strengthen READ_ONCE() to acquire
> when CONFIG_LTO=y"). Within those asm literals are % characters for the
> asm templates, which are ultimately interpreted as format specifiers
> when they get expanded by the preprocessors, hence the warning.
>
> There is nothing technically wrong here, although if this were to ever
> trigger, it would probably look quite ugly in the kernel log because of
> how long the string literal expansion of __READ_ONCE would be. It is
> possible to shut this warning up in a similar manner to the existing GCC
> pragma if necessary but I was unsure if that would be preferred off bat,
> hence just the report at first.
>
Thanks for the info Nathan,
in this case
88fec3526e84 apparmor: make sure unix socket labeling is correctly updated.
actually made this check invalid, so it as been removed, fixing this issue.
> Cheers,
> Nathan
>
> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> index 2e7c2c282f3a..9dfbc6dc8859 100644
> --- a/include/linux/compiler-clang.h
> +++ b/include/linux/compiler-clang.h
> @@ -114,10 +114,10 @@
> #define __diag_str(s) __diag_str1(s)
> #define __diag(s) _Pragma(__diag_str(clang diagnostic s))
>
> -#define __diag_clang_13(s) __diag(s)
> +#define __diag_clang_all(s) __diag(s)
>
> #define __diag_ignore_all(option, comment) \
> - __diag_clang(13, ignore, option)
> + __diag_clang(all, ignore, option)
>
> /*
> * clang has horrible behavior with "g" or "rm" constraints for asm
> diff --git a/security/apparmor/include/lib.h b/security/apparmor/include/lib.h
> index 444197075fd6..7f2c649dc7dd 100644
> --- a/security/apparmor/include/lib.h
> +++ b/security/apparmor/include/lib.h
> @@ -53,7 +53,11 @@ do { \
> #define AA_BUG(X, args...) \
> do { \
> _Pragma("GCC diagnostic ignored \"-Wformat-zero-length\""); \
> + __diag_push(); \
> + __diag_ignore(clang, all, "-Wformat-invalid-specifier", \
> + "May be called with asm that has %"); \
> AA_BUG_FMT((X), "" args); \
> + __diag_pop(); \
> _Pragma("GCC diagnostic warning \"-Wformat-zero-length\""); \
> } while (0)
> #ifdef CONFIG_SECURITY_APPARMOR_DEBUG_ASSERTS
^ permalink raw reply
* Re: [PATCH] lsm: simplify security_inode_copy_up_xattr()
From: Paul Moore @ 2025-07-30 15:35 UTC (permalink / raw)
To: tianjia.zhang
Cc: Casey Schaufler, James Morris, Serge E. Hallyn,
linux-security-module, linux-kernel
In-Reply-To: <c946ad53-15c8-497d-863b-a237e6c4466c@linux.alibaba.com>
On Wed, Jul 30, 2025 at 5:26 AM tianjia.zhang
<tianjia.zhang@linux.alibaba.com> wrote:
> On 7/29/25 11:09 PM, Paul Moore wrote:
> > On Tue, Jul 29, 2025 at 10:43 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> On 7/29/2025 2:09 AM, Tianjia Zhang wrote:
> >>> The implementation of function security_inode_copy_up_xattr can be
> >>> simplified to directly call call_int_hook().
> >>>
> >>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> >>> ---
> >>> security/security.c | 8 +-------
> >>> 1 file changed, 1 insertion(+), 7 deletions(-)
> >>>
> >>> diff --git a/security/security.c b/security/security.c
> >>> index 596d41818577..a5c2e5a8009f 100644
> >>> --- a/security/security.c
> >>> +++ b/security/security.c
> >>> @@ -2774,13 +2774,7 @@ EXPORT_SYMBOL(security_inode_copy_up);
> >>> */
> >>> int security_inode_copy_up_xattr(struct dentry *src, const char *name)
> >>> {
> >>> - int rc;
> >>> -
> >>> - rc = call_int_hook(inode_copy_up_xattr, src, name);
> >>> - if (rc != LSM_RET_DEFAULT(inode_copy_up_xattr))
> >>> - return rc;
> >>> -
> >>> - return LSM_RET_DEFAULT(inode_copy_up_xattr);
> >>> + return call_int_hook(inode_copy_up_xattr, src, name);
> >>
> >> Both the existing code and the proposed change are incorrect.
> >> If two LSMs supply the hook, and the first does not recognize
> >> the attribute, the second, which might recognize the attribute,
> >> will not be called. As SELinux and EVM both supply this hook
> >> there may be a real problem here.
> >
> > It appears that Smack also supplies a inode_copy_up_xattr() callback
> > via smack_inode_copy_up_xattr().
> >
> > Someone should double check this logic, but looking at it very
> > quickly, it would appear that LSM framework should run the individual
> > LSM callbacks in order so long as they return -EOPNOTSUPP, if they do
> > not return -EOPNOTSUPP, the return value should be returned to the
> > caller without executing any further callbacks. As a default return
> > value, or if all of the LSM callbacks succeed with -EOPNOTSUPP, the
> > hook should return -EOPNOTSUPP.
> >
> > Tianjia Zhang, would you be able to develop and test a patch for this?
> >
>
> Yes, I will submit a new patch to try to fix this issue. Thanks for your
> suggestion.
Great, thank you.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] lsm: yama: Check for PTRACE_MODE_READ_FSCREDS access
From: kernel test robot @ 2025-07-31 7:17 UTC (permalink / raw)
To: nicolas.bouchinet
Cc: oe-lkp, lkp, linux-security-module, ltp, Kees Cook, Paul Moore,
James Morris, Serge E. Hallyn, linux-kernel, Olivier Bal-Petre,
Nicolas Bouchinet, oliver.sang
In-Reply-To: <20250718-yama_fix-v1-1-a51455359e67@ssi.gouv.fr>
Hello,
kernel test robot noticed "ltp.migrate_pages02.fail" on:
commit: 0d6496041d407998103595db3dc42240f124a7f1 ("[PATCH] lsm: yama: Check for PTRACE_MODE_READ_FSCREDS access")
url: https://github.com/intel-lab-lkp/linux/commits/nicolas-bouchinet-oss-cyber-gouv-fr/lsm-yama-Check-for-PTRACE_MODE_READ_FSCREDS-access/20250718-164849
patch link: https://lore.kernel.org/all/20250718-yama_fix-v1-1-a51455359e67@ssi.gouv.fr/
patch subject: [PATCH] lsm: yama: Check for PTRACE_MODE_READ_FSCREDS access
in testcase: ltp
version: ltp-x86_64-0e4be9201-1_20250726
with following parameters:
test: numa/migrate_pages02
config: x86_64-rhel-9.4-ltp
compiler: gcc-12
test machine: 224 threads 2 sockets Intel(R) Xeon(R) Platinum 8480+ (Sapphire Rapids) with 256G memory
(please refer to attached dmesg/kmsg for entire log/backtrace)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202507310843.52fb528f-lkp@intel.com
Running tests.......
<<<test_start>>>
tag=migrate_pages02 stime=1753707486
cmdline="migrate_pages02"
contacts=""
analysis=exit
<<<test_output>>>
tst_tmpdir.c:316: TINFO: Using /tmp/ltp-5PDNJlpHwH/LTP_mig4NxxK0 as tmpdir (tmpfs filesystem)
tst_test.c:2004: TINFO: LTP version: 20250530-108-g0e4be9201
tst_test.c:2007: TINFO: Tested kernel: 6.16.0-rc2-00013-g0d6496041d40 #1 SMP PREEMPT_DYNAMIC Mon Jul 28 20:46:21 CST 2025 x86_64
tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
tst_kconfig.c:676: TINFO: CONFIG_KASAN kernel option detected which might slow the execution
tst_test.c:1825: TINFO: Overall timeout per run is 0h 02m 00s
migrate_pages02.c:313: TINFO: Using nodes: 0 1
migrate_pages02.c:143: TINFO: current_process, cap_sys_nice: 1
migrate_pages02.c:147: TINFO: private anonymous: 0x7f1006150000
migrate_pages02.c:75: TINFO: pid(6801) migrate pid 0 to node -> 1
migrate_pages02.c:96: TINFO: migrate_pages could not migrate all pages, not migrated: 452
migrate_pages02.c:125: TPASS: pid(6801) addr 0x7f1006150000 is on expected node: 1
migrate_pages02.c:75: TINFO: pid(6801) migrate pid 0 to node -> 0
migrate_pages02.c:125: TPASS: pid(6801) addr 0x7f1006150000 is on expected node: 0
migrate_pages02.c:159: TINFO: shared anonymous: 0x7f1006150000
migrate_pages02.c:75: TINFO: pid(6801) migrate pid 0 to node -> 1
migrate_pages02.c:96: TINFO: migrate_pages could not migrate all pages, not migrated: 452
migrate_pages02.c:125: TPASS: pid(6801) addr 0x7f1006150000 is on expected node: 1
migrate_pages02.c:167: TINFO: child shared anonymous, cap_sys_nice: 1
migrate_pages02.c:75: TINFO: pid(6803) migrate pid 0 to node -> 0
migrate_pages02.c:125: TPASS: pid(6803) addr 0x7f100614f000 is on expected node: 0
migrate_pages02.c:125: TPASS: pid(6801) addr 0x7f1006150000 is on expected node: 0
migrate_pages02.c:143: TINFO: current_process, cap_sys_nice: 0
migrate_pages02.c:147: TINFO: private anonymous: 0x7f1006150000
migrate_pages02.c:75: TINFO: pid(6801) migrate pid 0 to node -> 1
migrate_pages02.c:96: TINFO: migrate_pages could not migrate all pages, not migrated: 452
migrate_pages02.c:125: TPASS: pid(6801) addr 0x7f1006150000 is on expected node: 1
migrate_pages02.c:75: TINFO: pid(6801) migrate pid 0 to node -> 0
migrate_pages02.c:125: TPASS: pid(6801) addr 0x7f1006150000 is on expected node: 0
migrate_pages02.c:159: TINFO: shared anonymous: 0x7f1006150000
migrate_pages02.c:75: TINFO: pid(6801) migrate pid 0 to node -> 1
migrate_pages02.c:96: TINFO: migrate_pages could not migrate all pages, not migrated: 452
migrate_pages02.c:125: TPASS: pid(6801) addr 0x7f1006150000 is on expected node: 1
migrate_pages02.c:167: TINFO: child shared anonymous, cap_sys_nice: 0
migrate_pages02.c:75: TINFO: pid(6804) migrate pid 0 to node -> 0
migrate_pages02.c:125: TPASS: pid(6804) addr 0x7f100614f000 is on expected node: 0
migrate_pages02.c:125: TPASS: pid(6801) addr 0x7f1006150000 is on expected node: 1
migrate_pages02.c:200: TINFO: other_process, cap_sys_nice: 1
migrate_pages02.c:75: TINFO: pid(6805) migrate pid 0 to node -> 0
migrate_pages02.c:125: TPASS: pid(6805) addr 0x7f1006150000 is on expected node: 0
migrate_pages02.c:75: TINFO: pid(6806) migrate pid 6805 to node -> 1
migrate_pages02.c:96: TINFO: migrate_pages could not migrate all pages, not migrated: 362
migrate_pages02.c:125: TPASS: pid(6805) addr 0x7f1006150000 is on expected node: 1
migrate_pages02.c:200: TINFO: other_process, cap_sys_nice: 0
migrate_pages02.c:75: TINFO: pid(6807) migrate pid 0 to node -> 0
migrate_pages02.c:125: TPASS: pid(6807) addr 0x7f1006150000 is on expected node: 0
migrate_pages02.c:75: TINFO: pid(6808) migrate pid 6807 to node -> 1
migrate_pages02.c:92: TFAIL: migrate_pages failed ret: -1, : EPERM (1)
migrate_pages02.c:55: TINFO: mem_stats pid: 6807, node: 1
Name: migrate_pages02
Umask: 0000
State: S (sleeping)
Tgid: 6807
Ngid: 0
Pid: 6807
PPid: 6801
TracerPid: 0
Uid: 65534 65534 65534 65534
Gid: 0 0 0 0
FDSize: 64
Groups:
NStgid: 6807
NSpid: 6807
NSpgid: 6801
NSsid: 2420
Kthread: 0
VmPeak: 2868 kB
VmSize: 2868 kB
VmLck: 0 kB
VmPin: 0 kB
VmHWM: 0 kB
VmRSS: 0 kB
RssAnon: 0 kB
RssFile: 0 kB
RssShmem: 0 kB
VmData: 348 kB
VmStk: 136 kB
VmExe: 132 kB
VmLib: 1548 kB
VmPTE: 48 kB
VmSwap: 0 kB
HugetlbPages: 0 kB
CoreDumping: 0
THP_enabled: 1
untag_mask: 0xffffffffffffffff
Threads: 1
SigQ: 0/444866
SigPnd: 0000000000000000
ShdPnd: 0000000000000000
SigBlk: 0000000000000000
SigIgn: 0000000000000004
SigCgt: 0000000000000000
CapInh: 0000000000000000
CapPrm: 0000000000000000
CapEff: 0000000000000000
CapBnd: 000001ffffffffff
CapAmb: 0000000000000000
NoNewPrivs: 0
Seccomp: 0
Seccomp_filters: 0
Speculation_Store_Bypass: thread vulnerable
SpeculationIndirectBranch: conditional enabled
Cpus_allowed: ffffffff,ffffffff,ffffffff,ffffffff,ffffffff,ffffffff,ffffffff
Cpus_allowed_list: 0-223
Mems_allowed: 00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000003
Mems_allowed_list: 0-1
voluntary_ctxt_switches: 5
nonvoluntary_ctxt_switches: 0
cat: /proc/6807/numa_maps: Permission denied
migrate_pages02.c:65: TINFO: Node id: 1, size: 117471789056, free: 106811031552
migrate_pages02.c:129: TFAIL: pid(6807) addr 0x7f1006150000 not on expected node: 0 , expected 1
migrate_pages02.c:55: TINFO: mem_stats pid: 6807, node: 1
Name: migrate_pages02
Umask: 0000
State: S (sleeping)
Tgid: 6807
Ngid: 0
Pid: 6807
PPid: 6801
TracerPid: 0
Uid: 65534 65534 65534 65534
Gid: 0 0 0 0
FDSize: 64
Groups:
NStgid: 6807
NSpid: 6807
NSpgid: 6801
NSsid: 2420
Kthread: 0
VmPeak: 2904 kB
VmSize: 2868 kB
VmLck: 0 kB
VmPin: 0 kB
VmHWM: 0 kB
VmRSS: 0 kB
RssAnon: 0 kB
RssFile: 0 kB
RssShmem: 0 kB
VmData: 348 kB
VmStk: 136 kB
VmExe: 132 kB
VmLib: 1548 kB
VmPTE: 48 kB
VmSwap: 0 kB
HugetlbPages: 0 kB
CoreDumping: 0
THP_enabled: 1
untag_mask: 0xffffffffffffffff
Threads: 1
SigQ: 0/444866
SigPnd: 0000000000000000
ShdPnd: 0000000000000000
SigBlk: 0000000000010000
SigIgn: 0000000000000006
SigCgt: 0000000000000000
CapInh: 0000000000000000
CapPrm: 0000000000000000
CapEff: 0000000000000000
CapBnd: 000001ffffffffff
CapAmb: 0000000000000000
NoNewPrivs: 0
Seccomp: 0
Seccomp_filters: 0
Speculation_Store_Bypass: thread vulnerable
SpeculationIndirectBranch: conditional enabled
Cpus_allowed: ffffffff,ffffffff,ffffffff,ffffffff,ffffffff,ffffffff,ffffffff
Cpus_allowed_list: 0-223
Mems_allowed: 00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000003
Mems_allowed_list: 0-1
voluntary_ctxt_switches: 7
nonvoluntary_ctxt_switches: 0
cat: /proc/6807/numa_maps: Permission denied
migrate_pages02.c:65: TINFO: Node id: 1, size: 117471789056, free: 106811031552
Summary:
passed 13
failed 2
broken 0
skipped 0
warnings 0
incrementing stop
<<<execution_status>>>
initiation_status="ok"
duration=0 termination_type=exited termination_id=1 corefile=no
cutime=0 cstime=11
<<<test_end>>>
INFO: ltp-pan reported some tests FAIL
LTP Version: 20250530-108-g0e4be9201
###############################################################
Done executing testcases.
LTP Version: 20250530-108-g0e4be9201
###############################################################
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20250731/202507310843.52fb528f-lkp@intel.com
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH] lsm: simplify security_inode_copy_up_xattr()
From: tianjia.zhang @ 2025-07-31 11:59 UTC (permalink / raw)
To: Paul Moore, Casey Schaufler
Cc: James Morris, Serge E. Hallyn, linux-security-module,
linux-kernel
In-Reply-To: <CAHC9VhQAVvvXUoFu7xnh0uBhmvgYinP=AhiC4y17JJ02M9s5Nw@mail.gmail.com>
On 7/29/25 11:09 PM, Paul Moore wrote:
> On Tue, Jul 29, 2025 at 10:43 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 7/29/2025 2:09 AM, Tianjia Zhang wrote:
>>> The implementation of function security_inode_copy_up_xattr can be
>>> simplified to directly call call_int_hook().
>>>
>>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
>>> ---
>>> security/security.c | 8 +-------
>>> 1 file changed, 1 insertion(+), 7 deletions(-)
>>>
>>> diff --git a/security/security.c b/security/security.c
>>> index 596d41818577..a5c2e5a8009f 100644
>>> --- a/security/security.c
>>> +++ b/security/security.c
>>> @@ -2774,13 +2774,7 @@ EXPORT_SYMBOL(security_inode_copy_up);
>>> */
>>> int security_inode_copy_up_xattr(struct dentry *src, const char *name)
>>> {
>>> - int rc;
>>> -
>>> - rc = call_int_hook(inode_copy_up_xattr, src, name);
>>> - if (rc != LSM_RET_DEFAULT(inode_copy_up_xattr))
>>> - return rc;
>>> -
>>> - return LSM_RET_DEFAULT(inode_copy_up_xattr);
>>> + return call_int_hook(inode_copy_up_xattr, src, name);
>>
>> Both the existing code and the proposed change are incorrect.
>> If two LSMs supply the hook, and the first does not recognize
>> the attribute, the second, which might recognize the attribute,
>> will not be called. As SELinux and EVM both supply this hook
>> there may be a real problem here.
>
> It appears that Smack also supplies a inode_copy_up_xattr() callback
> via smack_inode_copy_up_xattr().
>
> Someone should double check this logic, but looking at it very
> quickly, it would appear that LSM framework should run the individual
> LSM callbacks in order so long as they return -EOPNOTSUPP, if they do
> not return -EOPNOTSUPP, the return value should be returned to the
> caller without executing any further callbacks. As a default return
> value, or if all of the LSM callbacks succeed with -EOPNOTSUPP, the
> hook should return -EOPNOTSUPP.
>
> Tianjia Zhang, would you be able to develop and test a patch for this?
>
I carefully checked the logic of security_inode_copy_up_xattr(). I think
there is no problem with the current code. The default return value of
inode_copy_up_xattr LSM is -EOPNOTSUPP. Therefore, when -EOPNOTSUPP is
returned in the LSM callback, the next callback function will be called
in a loop. When an LSM module recognizes the attribute name that needs
to be ignored, it will return -ECANCELED to indicate
security_inode_copy_up_xattr() to jump out of the loop and ignore the
copy of this attribute in overlayfs.
Currently, the SELinux, EVM, and Smack that supply inode_copy_up_xattr
callback all return -ECANCELED after recognizing the extended attribute
name they are concerned about, to indicate overlayfs to discard the
copy_up operation of this attribute. I think this is in line with
expectations.
Tianjia,
Cheers
^ permalink raw reply
* Re: [PATCH] lsm: simplify security_inode_copy_up_xattr()
From: Casey Schaufler @ 2025-07-31 15:00 UTC (permalink / raw)
To: tianjia.zhang, Paul Moore
Cc: James Morris, Serge E. Hallyn, linux-security-module,
linux-kernel, Casey Schaufler
In-Reply-To: <2aa4da27-28fc-46e0-8d1a-d9e63b03d502@linux.alibaba.com>
On 7/31/2025 4:59 AM, tianjia.zhang wrote:
>
>
> On 7/29/25 11:09 PM, Paul Moore wrote:
>> On Tue, Jul 29, 2025 at 10:43 AM Casey Schaufler
>> <casey@schaufler-ca.com> wrote:
>>> On 7/29/2025 2:09 AM, Tianjia Zhang wrote:
>>>> The implementation of function security_inode_copy_up_xattr can be
>>>> simplified to directly call call_int_hook().
>>>>
>>>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
>>>> ---
>>>> security/security.c | 8 +-------
>>>> 1 file changed, 1 insertion(+), 7 deletions(-)
>>>>
>>>> diff --git a/security/security.c b/security/security.c
>>>> index 596d41818577..a5c2e5a8009f 100644
>>>> --- a/security/security.c
>>>> +++ b/security/security.c
>>>> @@ -2774,13 +2774,7 @@ EXPORT_SYMBOL(security_inode_copy_up);
>>>> */
>>>> int security_inode_copy_up_xattr(struct dentry *src, const char
>>>> *name)
>>>> {
>>>> - int rc;
>>>> -
>>>> - rc = call_int_hook(inode_copy_up_xattr, src, name);
>>>> - if (rc != LSM_RET_DEFAULT(inode_copy_up_xattr))
>>>> - return rc;
>>>> -
>>>> - return LSM_RET_DEFAULT(inode_copy_up_xattr);
>>>> + return call_int_hook(inode_copy_up_xattr, src, name);
>>>
>>> Both the existing code and the proposed change are incorrect.
>>> If two LSMs supply the hook, and the first does not recognize
>>> the attribute, the second, which might recognize the attribute,
>>> will not be called. As SELinux and EVM both supply this hook
>>> there may be a real problem here.
>>
>> It appears that Smack also supplies a inode_copy_up_xattr() callback
>> via smack_inode_copy_up_xattr().
>>
>> Someone should double check this logic, but looking at it very
>> quickly, it would appear that LSM framework should run the individual
>> LSM callbacks in order so long as they return -EOPNOTSUPP, if they do
>> not return -EOPNOTSUPP, the return value should be returned to the
>> caller without executing any further callbacks. As a default return
>> value, or if all of the LSM callbacks succeed with -EOPNOTSUPP, the
>> hook should return -EOPNOTSUPP.
>>
>> Tianjia Zhang, would you be able to develop and test a patch for this?
>>
>
> I carefully checked the logic of security_inode_copy_up_xattr(). I think
> there is no problem with the current code. The default return value of
> inode_copy_up_xattr LSM is -EOPNOTSUPP. Therefore, when -EOPNOTSUPP is
> returned in the LSM callback, the next callback function will be called
> in a loop. When an LSM module recognizes the attribute name that needs
> to be ignored, it will return -ECANCELED to indicate
> security_inode_copy_up_xattr() to jump out of the loop and ignore the
> copy of this attribute in overlayfs.
>
> Currently, the SELinux, EVM, and Smack that supply inode_copy_up_xattr
> callback all return -ECANCELED after recognizing the extended attribute
> name they are concerned about, to indicate overlayfs to discard the
> copy_up operation of this attribute. I think this is in line with
> expectations.
I looked at the code more carefully and I think you're right.
My objection was based on the behavior of a much earlier version
of call_int_hook(). With that, I think your proposed change is
reasonable.
>
> Tianjia,
> Cheers
^ permalink raw reply
* Re: [PATCH v2 08/13] bpf: Implement signature verification for BPF programs
From: Dan Carpenter @ 2025-07-31 15:57 UTC (permalink / raw)
To: oe-kbuild, KP Singh, bpf, linux-security-module
Cc: lkp, oe-kbuild-all, bboscaccy, paul, kys, ast, daniel, andrii,
KP Singh
In-Reply-To: <20250721211958.1881379-9-kpsingh@kernel.org>
Hi KP,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/KP-Singh/bpf-Update-the-bpf_prog_calc_tag-to-use-SHA256/20250722-052316
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link: https://lore.kernel.org/r/20250721211958.1881379-9-kpsingh%40kernel.org
patch subject: [PATCH v2 08/13] bpf: Implement signature verification for BPF programs
config: m68k-randconfig-r073-20250723 (https://download.01.org/0day-ci/archive/20250723/202507231202.8rYZJ8D1-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 14.3.0
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202507231202.8rYZJ8D1-lkp@intel.com/
smatch warnings:
kernel/bpf/syscall.c:2797 bpf_prog_verify_signature() warn: 'sig' is an error pointer or valid
vim +/sig +2797 kernel/bpf/syscall.c
c83b0ba795b625 KP Singh 2025-07-21 2782 static noinline int bpf_prog_verify_signature(struct bpf_prog *prog,
c83b0ba795b625 KP Singh 2025-07-21 2783 union bpf_attr *attr,
c83b0ba795b625 KP Singh 2025-07-21 2784 bool is_kernel)
c83b0ba795b625 KP Singh 2025-07-21 2785 {
c83b0ba795b625 KP Singh 2025-07-21 2786 bpfptr_t usig = make_bpfptr(attr->signature, is_kernel);
c83b0ba795b625 KP Singh 2025-07-21 2787 struct bpf_dynptr_kern sig_ptr, insns_ptr;
c83b0ba795b625 KP Singh 2025-07-21 2788 struct bpf_key *key = NULL;
c83b0ba795b625 KP Singh 2025-07-21 2789 void *sig;
c83b0ba795b625 KP Singh 2025-07-21 2790 int err = 0;
c83b0ba795b625 KP Singh 2025-07-21 2791
c83b0ba795b625 KP Singh 2025-07-21 2792 key = bpf_lookup_user_key(attr->keyring_id, 0);
c83b0ba795b625 KP Singh 2025-07-21 2793 if (!key)
c83b0ba795b625 KP Singh 2025-07-21 2794 return -ENOKEY;
c83b0ba795b625 KP Singh 2025-07-21 2795
c83b0ba795b625 KP Singh 2025-07-21 2796 sig = kvmemdup_bpfptr(usig, attr->signature_size);
c83b0ba795b625 KP Singh 2025-07-21 @2797 if (!sig) {
This should be an if (!IS_ERR(sig)) { check.
c83b0ba795b625 KP Singh 2025-07-21 2798 bpf_key_put(key);
c83b0ba795b625 KP Singh 2025-07-21 2799 return -ENOMEM;
c83b0ba795b625 KP Singh 2025-07-21 2800 }
c83b0ba795b625 KP Singh 2025-07-21 2801
c83b0ba795b625 KP Singh 2025-07-21 2802 bpf_dynptr_init(&sig_ptr, sig, BPF_DYNPTR_TYPE_LOCAL, 0,
c83b0ba795b625 KP Singh 2025-07-21 2803 attr->signature_size);
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [GIT PULL] IPE update for 6.17
From: pr-tracker-bot @ 2025-07-31 17:55 UTC (permalink / raw)
To: Fan Wu
Cc: torvalds, linux-security-module, Linux Kernel Mailing List,
Eric Biggers
In-Reply-To: <CAKtyLkFWt6+5iKWy8BKJH2L51OpfPsEFkpjjn-EQGfHaSRnqAA@mail.gmail.com>
The pull request you sent on Mon, 28 Jul 2025 19:15:44 -0700:
> git://git.kernel.org/pub/scm/linux/kernel/git/wufan/ipe.git tags/ipe-pr-20250728
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/b4efd62564e96d1edb99eb00dd0ff620dbd1afab
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [GIT PULL] capabilities update for v6.17-rc1
From: pr-tracker-bot @ 2025-07-31 19:07 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: torvalds, linux-security-module, Linux Kernel Mailing List,
Ariel Otilibili, Andrew G. Morgan, Paul Moore, Eric W. Biederman,
Max Kellermann, Jann Horn, Kees Cook
In-Reply-To: <aIkfm1AGBoINgSRF@mail.hallyn.com>
The pull request you sent on Tue, 29 Jul 2025 14:23:07 -0500:
> https://git.kernel.org/pub/scm/linux/kernel/git/sergeh/linux.git tags/caps-pr-20250729
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/12ed593ee88170145fff25c7b3325b227731c2a1
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [PATCH] lsm: simplify security_inode_copy_up_xattr()
From: Paul Moore @ 2025-07-31 21:17 UTC (permalink / raw)
To: tianjia.zhang
Cc: Casey Schaufler, James Morris, Serge E. Hallyn,
linux-security-module, linux-kernel
In-Reply-To: <2aa4da27-28fc-46e0-8d1a-d9e63b03d502@linux.alibaba.com>
On Thu, Jul 31, 2025 at 7:59 AM tianjia.zhang
<tianjia.zhang@linux.alibaba.com> wrote:
> On 7/29/25 11:09 PM, Paul Moore wrote:
> > On Tue, Jul 29, 2025 at 10:43 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> On 7/29/2025 2:09 AM, Tianjia Zhang wrote:
> >>> The implementation of function security_inode_copy_up_xattr can be
> >>> simplified to directly call call_int_hook().
> >>>
> >>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> >>> ---
> >>> security/security.c | 8 +-------
> >>> 1 file changed, 1 insertion(+), 7 deletions(-)
> >>>
> >>> diff --git a/security/security.c b/security/security.c
> >>> index 596d41818577..a5c2e5a8009f 100644
> >>> --- a/security/security.c
> >>> +++ b/security/security.c
> >>> @@ -2774,13 +2774,7 @@ EXPORT_SYMBOL(security_inode_copy_up);
> >>> */
> >>> int security_inode_copy_up_xattr(struct dentry *src, const char *name)
> >>> {
> >>> - int rc;
> >>> -
> >>> - rc = call_int_hook(inode_copy_up_xattr, src, name);
> >>> - if (rc != LSM_RET_DEFAULT(inode_copy_up_xattr))
> >>> - return rc;
> >>> -
> >>> - return LSM_RET_DEFAULT(inode_copy_up_xattr);
> >>> + return call_int_hook(inode_copy_up_xattr, src, name);
> >>
> >> Both the existing code and the proposed change are incorrect.
> >> If two LSMs supply the hook, and the first does not recognize
> >> the attribute, the second, which might recognize the attribute,
> >> will not be called. As SELinux and EVM both supply this hook
> >> there may be a real problem here.
> >
> > It appears that Smack also supplies a inode_copy_up_xattr() callback
> > via smack_inode_copy_up_xattr().
> >
> > Someone should double check this logic, but looking at it very
> > quickly, it would appear that LSM framework should run the individual
> > LSM callbacks in order so long as they return -EOPNOTSUPP, if they do
> > not return -EOPNOTSUPP, the return value should be returned to the
> > caller without executing any further callbacks. As a default return
> > value, or if all of the LSM callbacks succeed with -EOPNOTSUPP, the
> > hook should return -EOPNOTSUPP.
> >
> > Tianjia Zhang, would you be able to develop and test a patch for this?
> >
>
> I carefully checked the logic of security_inode_copy_up_xattr(). I think
> there is no problem with the current code. The default return value of
> inode_copy_up_xattr LSM is -EOPNOTSUPP. Therefore, when -EOPNOTSUPP is
> returned in the LSM callback, the next callback function will be called
> in a loop. When an LSM module recognizes the attribute name that needs
> to be ignored, it will return -ECANCELED to indicate
> security_inode_copy_up_xattr() to jump out of the loop and ignore the
> copy of this attribute in overlayfs.
>
> Currently, the SELinux, EVM, and Smack that supply inode_copy_up_xattr
> callback all return -ECANCELED after recognizing the extended attribute
> name they are concerned about, to indicate overlayfs to discard the
> copy_up operation of this attribute. I think this is in line with
> expectations.
Perfect, thanks!
--
paul-moore.com
^ permalink raw reply
* [PATCH AUTOSEL 6.6 22/23] securityfs: don't pin dentries twice, once is enough...
From: Sasha Levin @ 2025-08-03 21:20 UTC (permalink / raw)
To: patches, stable
Cc: Al Viro, Sasha Levin, paul, jmorris, serge, linux-security-module
In-Reply-To: <20250803212031.3547641-1-sashal@kernel.org>
From: Al Viro <viro@zeniv.linux.org.uk>
[ Upstream commit 27cd1bf1240d482e4f02ca4f9812e748f3106e4f ]
incidentally, securityfs_recursive_remove() is broken without that -
it leaks dentries, since simple_recursive_removal() does not expect
anything of that sort. It could be worked around by dput() in
remove_one() callback, but it's easier to just drop that double-get
stuff.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have a clear understanding of the issue. Let me analyze this:
**YES**
This commit should be backported to stable kernel trees. Here's the
extensive analysis:
## Analysis of the Double Dentry Reference Issue
The commit fixes a clear bug in securityfs where dentries are being
pinned twice:
1. **The Bug**: In `securityfs_create_dentry()` at line 162, there's a
`dget(dentry)` call immediately after `d_instantiate(dentry, inode)`.
This creates an extra reference count on the dentry that is never
properly balanced.
2. **Impact on `securityfs_remove()`**: The traditional
`securityfs_remove()` function does call `dput(dentry)` at line 309,
which would balance the extra `dget()`. However, this only happens
for the simple remove case.
3. **Critical Impact on `securityfs_recursive_remove()`**: The newly
added `securityfs_recursive_remove()` function (added in commit
7138679ff2a2) uses `simple_recursive_removal()` which expects normal
dentry reference counting. The `simple_recursive_removal()` function
in fs/libfs.c:
- Takes its own reference with `dget(dentry)` at the beginning
- Properly releases references with `dput()` calls throughout its
execution
- Does NOT expect the dentries to have extra references
4. **The Leak**: When `securityfs_recursive_remove()` is called, the
extra reference from the `dget()` in `securityfs_create_dentry()` is
never released because:
- `simple_recursive_removal()` only releases the references it takes
- The `remove_one()` callback only calls `simple_release_fs()` but
doesn't do any `dput()`
- This results in dentries being leaked with a refcount that never
reaches zero
5. **Pattern Consistency**: Looking at similar filesystem
implementations like debugfs (fs/debugfs/inode.c), they do NOT add an
extra `dget()` after `d_instantiate()`. The pattern is simply:
```c
d_instantiate(dentry, inode);
return end_creating(dentry);
```
No extra reference is taken.
6. **Backport Criteria Met**:
- **Fixes a real bug**: Memory leak of dentries when using
`securityfs_recursive_remove()`
- **Small and contained**: Single-line removal in two locations
- **No side effects**: Removing the extra `dget()` brings the code in
line with standard VFS patterns
- **Low regression risk**: The fix makes securityfs consistent with
other similar filesystems
- **Important for stability**: Memory leaks in kernel filesystem code
can accumulate over time
The commit message clearly identifies this as a bug fix
("securityfs_recursive_remove() is broken without that - it leaks
dentries"), and the fix is minimal and correct. This makes it an ideal
candidate for stable backporting.
security/inode.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/security/inode.c b/security/inode.c
index 3aa75fffa8c9..a90b043695d9 100644
--- a/security/inode.c
+++ b/security/inode.c
@@ -159,7 +159,6 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
inode->i_fop = fops;
}
d_instantiate(dentry, inode);
- dget(dentry);
inode_unlock(dir);
return dentry;
@@ -306,7 +305,6 @@ void securityfs_remove(struct dentry *dentry)
simple_rmdir(dir, dentry);
else
simple_unlink(dir, dentry);
- dput(dentry);
}
inode_unlock(dir);
simple_release_fs(&mount, &mount_count);
--
2.39.5
^ permalink raw reply related
* [PATCH AUTOSEL 6.1 16/16] securityfs: don't pin dentries twice, once is enough...
From: Sasha Levin @ 2025-08-03 21:21 UTC (permalink / raw)
To: patches, stable
Cc: Al Viro, Sasha Levin, paul, jmorris, serge, linux-security-module
In-Reply-To: <20250803212127.3548367-1-sashal@kernel.org>
From: Al Viro <viro@zeniv.linux.org.uk>
[ Upstream commit 27cd1bf1240d482e4f02ca4f9812e748f3106e4f ]
incidentally, securityfs_recursive_remove() is broken without that -
it leaks dentries, since simple_recursive_removal() does not expect
anything of that sort. It could be worked around by dput() in
remove_one() callback, but it's easier to just drop that double-get
stuff.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have a clear understanding of the issue. Let me analyze this:
**YES**
This commit should be backported to stable kernel trees. Here's the
extensive analysis:
## Analysis of the Double Dentry Reference Issue
The commit fixes a clear bug in securityfs where dentries are being
pinned twice:
1. **The Bug**: In `securityfs_create_dentry()` at line 162, there's a
`dget(dentry)` call immediately after `d_instantiate(dentry, inode)`.
This creates an extra reference count on the dentry that is never
properly balanced.
2. **Impact on `securityfs_remove()`**: The traditional
`securityfs_remove()` function does call `dput(dentry)` at line 309,
which would balance the extra `dget()`. However, this only happens
for the simple remove case.
3. **Critical Impact on `securityfs_recursive_remove()`**: The newly
added `securityfs_recursive_remove()` function (added in commit
7138679ff2a2) uses `simple_recursive_removal()` which expects normal
dentry reference counting. The `simple_recursive_removal()` function
in fs/libfs.c:
- Takes its own reference with `dget(dentry)` at the beginning
- Properly releases references with `dput()` calls throughout its
execution
- Does NOT expect the dentries to have extra references
4. **The Leak**: When `securityfs_recursive_remove()` is called, the
extra reference from the `dget()` in `securityfs_create_dentry()` is
never released because:
- `simple_recursive_removal()` only releases the references it takes
- The `remove_one()` callback only calls `simple_release_fs()` but
doesn't do any `dput()`
- This results in dentries being leaked with a refcount that never
reaches zero
5. **Pattern Consistency**: Looking at similar filesystem
implementations like debugfs (fs/debugfs/inode.c), they do NOT add an
extra `dget()` after `d_instantiate()`. The pattern is simply:
```c
d_instantiate(dentry, inode);
return end_creating(dentry);
```
No extra reference is taken.
6. **Backport Criteria Met**:
- **Fixes a real bug**: Memory leak of dentries when using
`securityfs_recursive_remove()`
- **Small and contained**: Single-line removal in two locations
- **No side effects**: Removing the extra `dget()` brings the code in
line with standard VFS patterns
- **Low regression risk**: The fix makes securityfs consistent with
other similar filesystems
- **Important for stability**: Memory leaks in kernel filesystem code
can accumulate over time
The commit message clearly identifies this as a bug fix
("securityfs_recursive_remove() is broken without that - it leaks
dentries"), and the fix is minimal and correct. This makes it an ideal
candidate for stable backporting.
security/inode.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/security/inode.c b/security/inode.c
index 6c326939750d..e6e07787eec9 100644
--- a/security/inode.c
+++ b/security/inode.c
@@ -159,7 +159,6 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
inode->i_fop = fops;
}
d_instantiate(dentry, inode);
- dget(dentry);
inode_unlock(dir);
return dentry;
@@ -306,7 +305,6 @@ void securityfs_remove(struct dentry *dentry)
simple_rmdir(dir, dentry);
else
simple_unlink(dir, dentry);
- dput(dentry);
}
inode_unlock(dir);
simple_release_fs(&mount, &mount_count);
--
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