* [PATCH RFC V10 2/7] mm/filemap: Extend __filemap_get_folio() to support NUMA memory policies
From: Shivank Garg @ 2025-08-11 9:06 UTC (permalink / raw)
To: seanjc, david, vbabka, willy, akpm, shuah, pbonzini, brauner,
viro
Cc: ackerleytng, paul, jmorris, serge, pvorel, bfoster, tabba,
vannapurve, chao.gao, bharata, nikunj, michael.day, shdhiman,
yan.y.zhao, Neeraj.Upadhyay, thomas.lendacky, michael.roth, aik,
jgg, kalyazin, peterx, shivankg, jack, rppt, hch, cgzones,
ira.weiny, rientjes, roypat, ziy, matthew.brost, joshua.hahnjy,
rakie.kim, byungchul, gourry, kent.overstreet, ying.huang,
apopple, chao.p.peng, amit, ddutile, dan.j.williams, ashish.kalra,
gshan, jgowans, pankaj.gupta, papaluri, yuzhao, suzuki.poulose,
quic_eberman, aneeshkumar.kizhakeveetil, linux-fsdevel, linux-mm,
linux-kernel, linux-security-module, kvm, linux-kselftest,
linux-coco
In-Reply-To: <20250811090605.16057-2-shivankg@amd.com>
From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
Extend __filemap_get_folio() to support NUMA memory policies by
renaming the implementation to __filemap_get_folio_mpol() and adding
a mempolicy parameter. The original function becomes a static inline
wrapper that passes NULL for the mempolicy.
This infrastructure will enable future support for NUMA-aware page cache
allocations in guest_memfd memory backend KVM guests.
Reviewed-by: Pankaj Gupta <pankaj.gupta@amd.com>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
include/linux/pagemap.h | 10 ++++++++--
mm/filemap.c | 11 ++++++-----
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index ce617a35dc35..94d65ced0a1d 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -747,11 +747,17 @@ static inline fgf_t fgf_set_order(size_t size)
}
void *filemap_get_entry(struct address_space *mapping, pgoff_t index);
-struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
- fgf_t fgp_flags, gfp_t gfp);
+struct folio *__filemap_get_folio_mpol(struct address_space *mapping,
+ pgoff_t index, fgf_t fgf_flags, gfp_t gfp, struct mempolicy *policy);
struct page *pagecache_get_page(struct address_space *mapping, pgoff_t index,
fgf_t fgp_flags, gfp_t gfp);
+static inline struct folio *__filemap_get_folio(struct address_space *mapping,
+ pgoff_t index, fgf_t fgf_flags, gfp_t gfp)
+{
+ return __filemap_get_folio_mpol(mapping, index, fgf_flags, gfp, NULL);
+}
+
/**
* write_begin_get_folio - Get folio for write_begin with flags.
* @iocb: The kiocb passed from write_begin (may be NULL).
diff --git a/mm/filemap.c b/mm/filemap.c
index 495f7f5c3d2e..03f223be575c 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1897,11 +1897,12 @@ void *filemap_get_entry(struct address_space *mapping, pgoff_t index)
}
/**
- * __filemap_get_folio - Find and get a reference to a folio.
+ * __filemap_get_folio_mpol - Find and get a reference to a folio.
* @mapping: The address_space to search.
* @index: The page index.
* @fgp_flags: %FGP flags modify how the folio is returned.
* @gfp: Memory allocation flags to use if %FGP_CREAT is specified.
+ * @policy: NUMA memory allocation policy to follow.
*
* Looks up the page cache entry at @mapping & @index.
*
@@ -1912,8 +1913,8 @@ void *filemap_get_entry(struct address_space *mapping, pgoff_t index)
*
* Return: The found folio or an ERR_PTR() otherwise.
*/
-struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
- fgf_t fgp_flags, gfp_t gfp)
+struct folio *__filemap_get_folio_mpol(struct address_space *mapping,
+ pgoff_t index, fgf_t fgp_flags, gfp_t gfp, struct mempolicy *policy)
{
struct folio *folio;
@@ -1983,7 +1984,7 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
err = -ENOMEM;
if (order > min_order)
alloc_gfp |= __GFP_NORETRY | __GFP_NOWARN;
- folio = filemap_alloc_folio(alloc_gfp, order, NULL);
+ folio = filemap_alloc_folio(alloc_gfp, order, policy);
if (!folio)
continue;
@@ -2030,7 +2031,7 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
folio_clear_dropbehind(folio);
return folio;
}
-EXPORT_SYMBOL(__filemap_get_folio);
+EXPORT_SYMBOL(__filemap_get_folio_mpol);
static inline struct folio *find_get_entry(struct xa_state *xas, pgoff_t max,
xa_mark_t mark)
--
2.43.0
^ permalink raw reply related
* [PATCH RFC V10 1/7] mm/filemap: Add NUMA mempolicy support to filemap_alloc_folio()
From: Shivank Garg @ 2025-08-11 9:06 UTC (permalink / raw)
To: seanjc, david, vbabka, willy, akpm, shuah, pbonzini, brauner,
viro
Cc: ackerleytng, paul, jmorris, serge, pvorel, bfoster, tabba,
vannapurve, chao.gao, bharata, nikunj, michael.day, shdhiman,
yan.y.zhao, Neeraj.Upadhyay, thomas.lendacky, michael.roth, aik,
jgg, kalyazin, peterx, shivankg, jack, rppt, hch, cgzones,
ira.weiny, rientjes, roypat, ziy, matthew.brost, joshua.hahnjy,
rakie.kim, byungchul, gourry, kent.overstreet, ying.huang,
apopple, chao.p.peng, amit, ddutile, dan.j.williams, ashish.kalra,
gshan, jgowans, pankaj.gupta, papaluri, yuzhao, suzuki.poulose,
quic_eberman, aneeshkumar.kizhakeveetil, linux-fsdevel, linux-mm,
linux-kernel, linux-security-module, kvm, linux-kselftest,
linux-coco
In-Reply-To: <20250811090605.16057-2-shivankg@amd.com>
From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
Add a mempolicy parameter to filemap_alloc_folio() to enable NUMA-aware
page cache allocations. This will be used by upcoming changes to
support NUMA policies in guest-memfd, where guest_memory need to be
allocated NUMA policy specified by VMM.
All existing users pass NULL maintaining current behavior.
Reviewed-by: Pankaj Gupta <pankaj.gupta@amd.com>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
fs/bcachefs/fs-io-buffered.c | 2 +-
fs/btrfs/compression.c | 4 ++--
fs/btrfs/verity.c | 2 +-
fs/erofs/zdata.c | 2 +-
fs/f2fs/compress.c | 2 +-
include/linux/pagemap.h | 8 +++++---
mm/filemap.c | 14 +++++++++-----
mm/readahead.c | 2 +-
8 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/fs/bcachefs/fs-io-buffered.c b/fs/bcachefs/fs-io-buffered.c
index 1c54b9b5bd69..3af2eabb7ed3 100644
--- a/fs/bcachefs/fs-io-buffered.c
+++ b/fs/bcachefs/fs-io-buffered.c
@@ -124,7 +124,7 @@ static int readpage_bio_extend(struct btree_trans *trans,
if (folio && !xa_is_value(folio))
break;
- folio = filemap_alloc_folio(readahead_gfp_mask(iter->mapping), order);
+ folio = filemap_alloc_folio(readahead_gfp_mask(iter->mapping), order, NULL);
if (!folio)
break;
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index d09d622016ef..139f9609e737 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -474,8 +474,8 @@ static noinline int add_ra_bio_pages(struct inode *inode,
continue;
}
- folio = filemap_alloc_folio(mapping_gfp_constraint(mapping,
- ~__GFP_FS), 0);
+ folio = filemap_alloc_folio(mapping_gfp_constraint(mapping, ~__GFP_FS),
+ 0, NULL);
if (!folio)
break;
diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c
index b7a96a005487..c43a789ba6d2 100644
--- a/fs/btrfs/verity.c
+++ b/fs/btrfs/verity.c
@@ -742,7 +742,7 @@ static struct page *btrfs_read_merkle_tree_page(struct inode *inode,
}
folio = filemap_alloc_folio(mapping_gfp_constraint(inode->i_mapping, ~__GFP_FS),
- 0);
+ 0, NULL);
if (!folio)
return ERR_PTR(-ENOMEM);
diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
index 792f20888a8f..09e2ed2ae0d1 100644
--- a/fs/erofs/zdata.c
+++ b/fs/erofs/zdata.c
@@ -562,7 +562,7 @@ static void z_erofs_bind_cache(struct z_erofs_frontend *fe)
* Allocate a managed folio for cached I/O, or it may be
* then filled with a file-backed folio for in-place I/O
*/
- newfolio = filemap_alloc_folio(gfp, 0);
+ newfolio = filemap_alloc_folio(gfp, 0, NULL);
if (!newfolio)
continue;
newfolio->private = Z_EROFS_PREALLOCATED_FOLIO;
diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 5c1f47e45dab..56a51c9ba4f1 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1942,7 +1942,7 @@ static void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi,
return;
}
- cfolio = filemap_alloc_folio(__GFP_NOWARN | __GFP_IO, 0);
+ cfolio = filemap_alloc_folio(__GFP_NOWARN | __GFP_IO, 0, NULL);
if (!cfolio)
return;
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 12a12dae727d..ce617a35dc35 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -646,9 +646,11 @@ static inline void *detach_page_private(struct page *page)
}
#ifdef CONFIG_NUMA
-struct folio *filemap_alloc_folio_noprof(gfp_t gfp, unsigned int order);
+struct folio *filemap_alloc_folio_noprof(gfp_t gfp, unsigned int order,
+ struct mempolicy *policy);
#else
-static inline struct folio *filemap_alloc_folio_noprof(gfp_t gfp, unsigned int order)
+static inline struct folio *filemap_alloc_folio_noprof(gfp_t gfp, unsigned int order,
+ struct mempolicy *policy)
{
return folio_alloc_noprof(gfp, order);
}
@@ -659,7 +661,7 @@ static inline struct folio *filemap_alloc_folio_noprof(gfp_t gfp, unsigned int o
static inline struct page *__page_cache_alloc(gfp_t gfp)
{
- return &filemap_alloc_folio(gfp, 0)->page;
+ return &filemap_alloc_folio(gfp, 0, NULL)->page;
}
static inline gfp_t readahead_gfp_mask(struct address_space *x)
diff --git a/mm/filemap.c b/mm/filemap.c
index 751838ef05e5..495f7f5c3d2e 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -989,11 +989,16 @@ int filemap_add_folio(struct address_space *mapping, struct folio *folio,
EXPORT_SYMBOL_GPL(filemap_add_folio);
#ifdef CONFIG_NUMA
-struct folio *filemap_alloc_folio_noprof(gfp_t gfp, unsigned int order)
+struct folio *filemap_alloc_folio_noprof(gfp_t gfp, unsigned int order,
+ struct mempolicy *policy)
{
int n;
struct folio *folio;
+ if (policy)
+ return folio_alloc_mpol_noprof(gfp, order, policy,
+ NO_INTERLEAVE_INDEX, numa_node_id());
+
if (cpuset_do_page_mem_spread()) {
unsigned int cpuset_mems_cookie;
do {
@@ -1978,7 +1983,7 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
err = -ENOMEM;
if (order > min_order)
alloc_gfp |= __GFP_NORETRY | __GFP_NOWARN;
- folio = filemap_alloc_folio(alloc_gfp, order);
+ folio = filemap_alloc_folio(alloc_gfp, order, NULL);
if (!folio)
continue;
@@ -2517,7 +2522,7 @@ static int filemap_create_folio(struct kiocb *iocb, struct folio_batch *fbatch)
if (iocb->ki_flags & (IOCB_NOWAIT | IOCB_WAITQ))
return -EAGAIN;
- folio = filemap_alloc_folio(mapping_gfp_mask(mapping), min_order);
+ folio = filemap_alloc_folio(mapping_gfp_mask(mapping), min_order, NULL);
if (!folio)
return -ENOMEM;
if (iocb->ki_flags & IOCB_DONTCACHE)
@@ -3916,8 +3921,7 @@ static struct folio *do_read_cache_folio(struct address_space *mapping,
repeat:
folio = filemap_get_folio(mapping, index);
if (IS_ERR(folio)) {
- folio = filemap_alloc_folio(gfp,
- mapping_min_folio_order(mapping));
+ folio = filemap_alloc_folio(gfp, mapping_min_folio_order(mapping), NULL);
if (!folio)
return ERR_PTR(-ENOMEM);
index = mapping_align_index(mapping, index);
diff --git a/mm/readahead.c b/mm/readahead.c
index 406756d34309..a4dfa837dfbd 100644
--- a/mm/readahead.c
+++ b/mm/readahead.c
@@ -183,7 +183,7 @@ static struct folio *ractl_alloc_folio(struct readahead_control *ractl,
{
struct folio *folio;
- folio = filemap_alloc_folio(gfp_mask, order);
+ folio = filemap_alloc_folio(gfp_mask, order, NULL);
if (folio && ractl->dropbehind)
__folio_set_dropbehind(folio);
--
2.43.0
^ permalink raw reply related
* [PATCH RFC V10 0/7] Add NUMA mempolicy support for KVM guest-memfd
From: Shivank Garg @ 2025-08-11 9:05 UTC (permalink / raw)
To: seanjc, david, vbabka, willy, akpm, shuah, pbonzini, brauner,
viro
Cc: ackerleytng, paul, jmorris, serge, pvorel, bfoster, tabba,
vannapurve, chao.gao, bharata, nikunj, michael.day, shdhiman,
yan.y.zhao, Neeraj.Upadhyay, thomas.lendacky, michael.roth, aik,
jgg, kalyazin, peterx, shivankg, jack, rppt, hch, cgzones,
ira.weiny, rientjes, roypat, ziy, matthew.brost, joshua.hahnjy,
rakie.kim, byungchul, gourry, kent.overstreet, ying.huang,
apopple, chao.p.peng, amit, ddutile, dan.j.williams, ashish.kalra,
gshan, jgowans, pankaj.gupta, papaluri, yuzhao, suzuki.poulose,
quic_eberman, aneeshkumar.kizhakeveetil, linux-fsdevel, linux-mm,
linux-kernel, linux-security-module, kvm, linux-kselftest,
linux-coco
This series introduces NUMA-aware memory placement support for KVM guests
with guest_memfd memory backends. It builds upon Fuad Tabba's work (V17)
that enabled host-mapping for guest_memfd memory [1].
== Background ==
KVM's guest-memfd memory backend currently lacks support for NUMA policy
enforcement, causing guest memory allocations to be distributed across host
nodes according to kernel's default behavior, irrespective of any policy
specified by the VMM. This limitation arises because conventional userspace
NUMA control mechanisms like mbind(2) don't work since the memory isn't
directly mapped to userspace when allocations occur.
Fuad's work [1] provides the necessary mmap capability, and this series
leverages it to enable mbind(2).
== Implementation ==
This series implements proper NUMA policy support for guest-memfd by:
1. Adding mempolicy-aware allocation APIs to the filemap layer.
2. Introducing custom inodes (via a dedicated slab-allocated inode cache,
kvm_gmem_inode_info) to store NUMA policy and metadata for guest memory.
3. Implementing get/set_policy vm_ops in guest_memfd to support NUMA
policy.
With these changes, VMMs can now control guest memory placement by mapping
guest_memfd file descriptor and using mbind(2) to specify:
- Policy modes: default, bind, interleave, or preferred
- Host NUMA nodes: List of target nodes for memory allocation
These Policies affect only future allocations and do not migrate existing
memory. This matches mbind(2)'s default behavior which affects only new
allocations unless overridden with MPOL_MF_MOVE/MPOL_MF_MOVE_ALL flags (Not
supported for guest_memfd as it is unmovable by design).
== Upstream Plan ==
Phased approach as per David's guest_memfd extension overview [2] and
community calls [3]:
Phase 1 (this series):
1. Focuses on shared guest_memfd support (non-CoCo VMs).
2. Builds on Fuad's host-mapping work.
Phase2 (future work):
1. NUMA support for private guest_memfd (CoCo VMs).
2. Depends on SNP in-place conversion support [4].
This series provides a clean integration path for NUMA-aware memory
management for guest_memfd and lays the groundwork for future confidential
computing NUMA capabilities.
Please review and provide feedback!
Thanks,
Shivank
== Changelog ==
- v1,v2: Extended the KVM_CREATE_GUEST_MEMFD IOCTL to pass mempolicy.
- v3: Introduced fbind() syscall for VMM memory-placement configuration.
- v4-v6: Current approach using shared_policy support and vm_ops (based on
suggestions from David [5] and guest_memfd bi-weekly upstream
call discussion [6]).
- v7: Use inodes to store NUMA policy instead of file [7].
- v8: Rebase on top of Fuad's V12: Host mmaping for guest_memfd memory.
- v9: Rebase on top of Fuad's V13 and incorporate review comments
- V10: Rebase on top of Fuad's V17. Use latest guest_memfd inode patch
from Ackerley (with David's review comments). Use newer kmem_cache_create()
API variant with arg parameter (Vlastimil)
[1] https://lore.kernel.org/all/20250729225455.670324-1-seanjc@google.com
[2] https://lore.kernel.org/all/c1c9591d-218a-495c-957b-ba356c8f8e09@redhat.com
[3] https://docs.google.com/document/d/1M6766BzdY1Lhk7LiR5IqVR8B8mG3cr-cxTxOrAosPOk/edit?tab=t.0#heading=h.svcbod20b5ur
[4] https://lore.kernel.org/all/20250613005400.3694904-1-michael.roth@amd.com
[5] https://lore.kernel.org/all/6fbef654-36e2-4be5-906e-2a648a845278@redhat.com
[6] https://lore.kernel.org/all/2b77e055-98ac-43a1-a7ad-9f9065d7f38f@amd.com
[7] https://lore.kernel.org/all/diqzbjumm167.fsf@ackerleytng-ctop.c.googlers.com
Ackerley Tng (1):
KVM: guest_memfd: Use guest mem inodes instead of anonymous inodes
Matthew Wilcox (Oracle) (2):
mm/filemap: Add NUMA mempolicy support to filemap_alloc_folio()
mm/filemap: Extend __filemap_get_folio() to support NUMA memory
policies
Shivank Garg (4):
mm/mempolicy: Export memory policy symbols
KVM: guest_memfd: Add slab-allocated inode cache
KVM: guest_memfd: Enforce NUMA mempolicy using shared policy
KVM: guest_memfd: selftests: Add tests for mmap and NUMA policy
support
fs/bcachefs/fs-io-buffered.c | 2 +-
fs/btrfs/compression.c | 4 +-
fs/btrfs/verity.c | 2 +-
fs/erofs/zdata.c | 2 +-
fs/f2fs/compress.c | 2 +-
include/linux/pagemap.h | 18 +-
include/uapi/linux/magic.h | 1 +
mm/filemap.c | 23 +-
mm/mempolicy.c | 6 +
mm/readahead.c | 2 +-
tools/testing/selftests/kvm/Makefile.kvm | 1 +
.../testing/selftests/kvm/guest_memfd_test.c | 121 ++++++++
virt/kvm/guest_memfd.c | 260 ++++++++++++++++--
virt/kvm/kvm_main.c | 7 +-
virt/kvm/kvm_mm.h | 9 +-
15 files changed, 410 insertions(+), 50 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH V9 1/7] KVM: guest_memfd: Use guest mem inodes instead of anonymous inodes
From: Garg, Shivank @ 2025-08-11 8:02 UTC (permalink / raw)
To: Ackerley Tng, David Hildenbrand, seanjc, vbabka, willy, akpm,
shuah, pbonzini, brauner, viro
Cc: paul, jmorris, serge, pvorel, bfoster, tabba, vannapurve,
chao.gao, bharata, nikunj, michael.day, shdhiman, yan.y.zhao,
Neeraj.Upadhyay, thomas.lendacky, michael.roth, aik, jgg,
kalyazin, peterx, jack, rppt, hch, cgzones, ira.weiny, rientjes,
roypat, ziy, matthew.brost, joshua.hahnjy, rakie.kim, byungchul,
gourry, kent.overstreet, ying.huang, apopple, chao.p.peng, amit,
ddutile, dan.j.williams, ashish.kalra, gshan, jgowans,
pankaj.gupta, papaluri, yuzhao, suzuki.poulose, quic_eberman,
aneeshkumar.kizhakeveetil, linux-fsdevel, linux-mm, linux-kernel,
linux-security-module, kvm, linux-kselftest, linux-coco
In-Reply-To: <diqz4iui4y00.fsf@ackerleytng-ctop.c.googlers.com>
On 8/8/2025 3:04 AM, Ackerley Tng wrote:
> David Hildenbrand <david@redhat.com> writes:
>
>> On 13.07.25 19:43, Shivank Garg wrote:
>>> From: Ackerley Tng <ackerleytng@google.com>
>>>
>>> + ctx->ops = &kvm_gmem_super_operations;
>>
>> Curious, why is that required? (secretmem doesn't have it, so I wonder)
>>
>
> Good point! pseudo_fs_fill_super() fills in a struct super_operations
> which already does simple_statfs, so guest_memfd doesn't need this.
>
Right, simple_statfs isn't strictly needed in this patch, but the
super_operations is required for the subsequent patches in
the series which add custom alloc_inode, destroy_inode, and free_inode
callback.
>>> + if (!try_module_get(kvm_gmem_fops.owner))
>>> + goto err;
>>
>> Curious, shouldn't there be a module_put() somewhere after this function
>> returned a file?
>>
>
> This was interesting indeed, but IIUC this is correct.
>
> I think this flow was basically copied from __anon_inode_getfile(),
> which does this try_module_get().
>
> The corresponding module_put() is in __fput(), which calls fops_put()
> and calls module_put() on the owner.
>
>>> +
>>>
>>
>> Nothing else jumped at me.
>>
>
> Thanks for the review!
>
> Since we're going to submit this patch through Shivank's mempolicy
> support series, I'll follow up soon by sending a replacement patch in
> reply to this series so Shivank could build on top of that?
>
yes, I'll post the V10 soon.
Thanks,
Shivank
^ permalink raw reply
* Re: [RFC PATCH 2/2] memfd: call security_inode_init_security_anon
From: Thiébaud Weksteen @ 2025-08-11 5:34 UTC (permalink / raw)
To: Stephen Smalley
Cc: Paul Moore, James Morris, Hugh Dickins, Jeff Vander Stoep,
Nick Kralevich, Jeff Xu, linux-kernel, linux-security-module,
selinux
In-Reply-To: <CAEjxPJ7JFHs1w71qeOFYSAmReZsghAz3+XVJrTKH=v=P822yxQ@mail.gmail.com>
On Fri, Aug 8, 2025 at 10:41 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Fri, Aug 8, 2025 at 7:57 AM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
Thanks for the review Stephen.
> >
> > On Thu, Aug 7, 2025 at 3:57 AM 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.
> >
> > To provide backward compatibility, I would anticipate that you will
> > need to define a new SELinux policy capability and make this change
> > conditional on it being enabled, see:
> > https://github.com/SELinuxProject/selinux-kernel/wiki/Getting-Started#adding-a-new-selinux-policy-capability
> > for instructions and links to examples.
> >
I agree. Thanks, I'll add this to the next patchset.
> > Otherwise, see below.
> >
> > >
> > > [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>
> > > ---
> > > mm/memfd.c | 16 ++++++++++++++--
> > > security/selinux/hooks.c | 15 +++++++++++----
> > > security/selinux/include/classmap.h | 2 ++
> > > 3 files changed, 27 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/mm/memfd.c b/mm/memfd.c
> > > index bbe679895ef6..13bff0e91816 100644
> > > --- a/mm/memfd.c
> > > +++ b/mm/memfd.c
> > > @@ -433,6 +433,9 @@ static struct file *alloc_file(const char *name, unsigned int flags)
> > > {
> > > unsigned int *file_seals;
> > > struct file *file;
> > > + struct inode *inode;
> > > + int err = 0;
> > > + const char *anon_name = "[memfd]";
> > >
> > > if (flags & MFD_HUGETLB) {
> > > file = hugetlb_file_setup(name, 0, VM_NORESERVE,
> > > @@ -444,12 +447,21 @@ static struct file *alloc_file(const char *name, unsigned int flags)
> > > }
> > > if (IS_ERR(file))
> > > return file;
> > > +
> > > + inode = file_inode(file);
> > > + err = security_inode_init_security_anon(inode,
> > > + LSM_ANON_INODE_MEMFD,
> > > + &QSTR(anon_name), NULL);
> >
> > Since the anon_name already indicates that this is a memfd, so can't
> > you already distinguish these via name-based type_transition rules?
> > Why do we need the enum argument?
>
> On second thought, I see that you are distinguishing not just the
> security context/type but also the security class, but the question
> remains: can't you compare the name to make this determination?
>
I definitely can. My initial thought was that a string comparison
would not be adequate here (performance-wise), but I guess calls to
this method are infrequent enough that a strncmp would work here?
On the option of just relying on name-based type transitions, this has
an impact on the size of the policy: for Android, the transition from
ashmem to memfd means that virtually all domains will rely on such a
file. It means that we would need to define a type transition and a
new type for all our domains. (This is an argument on top of the need
for execute_no_trans, as I described in the commit message).
> >
> > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > > index 8d36d5ebb6e5..49742930e706 100644
> > > --- a/security/selinux/hooks.c
> > > +++ b/security/selinux/hooks.c
> > > @@ -2367,8 +2367,8 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm)
> > > ad.u.file = bprm->file;
> > >
> > > if (new_tsec->sid == old_tsec->sid) {
> > > - rc = avc_has_perm(old_tsec->sid, isec->sid,
> > > - SECCLASS_FILE, FILE__EXECUTE_NO_TRANS, &ad);
> > > + rc = avc_has_perm(old_tsec->sid, isec->sid, isec->sclass,
> > > + FILE__EXECUTE_NO_TRANS, &ad);
> >
> > Here and below I am a little concerned that we could end up reaching
> > this code on an inode with an isec->sclass that does not define the
> > execute_no_trans and entrypoint permissions. We should do something to
> > make that never happens, or check for it and always deny in that case.
I agree. I can add a condition to make sure only these 2 classes (file
or memfd_file) are used here.
> >
> > > if (rc)
> > > return rc;
> > > } else {
> > > @@ -2378,8 +2378,8 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm)
> > > if (rc)
> > > return rc;
> > >
> > > - rc = avc_has_perm(new_tsec->sid, isec->sid,
> > > - SECCLASS_FILE, FILE__ENTRYPOINT, &ad);
> > > + rc = avc_has_perm(new_tsec->sid, isec->sid, isec->sclass,
> > > + FILE__ENTRYPOINT, &ad);
> > > if (rc)
> > > return rc;
> > >
> > > @@ -2997,6 +2997,13 @@ static int selinux_inode_init_security_anon(struct inode *inode,
> > >
> > > isec->sclass = context_isec->sclass;
> > > isec->sid = context_isec->sid;
> > > + } else if (type == LSM_ANON_INODE_MEMFD) {
> > > + isec->sclass = SECCLASS_MEMFD_FILE;
> > > + rc = security_transition_sid(
> > > + sid, sid,
> > > + isec->sclass, name, &isec->sid);
> >
> > Again, name-based type_transitions ought to be able to distinguish
> > memfd based on the name argument IIUC.
^ permalink raw reply
* Re: [PATCH v3 00/13] stackleak: Support Clang stack depth tracking
From: patchwork-bot+linux-riscv @ 2025-08-10 21:12 UTC (permalink / raw)
To: Kees Cook
Cc: linux-riscv, arnd, mingo, gustavoars, hch, andreyknvl,
ryabinin.a.a, ardb, masahiroy, nathan, nicolas.schier,
nick.desaulniers+lkml, morbo, justinstitt, linux-kernel, x86,
kasan-dev, linux-doc, linux-arm-kernel, kvmarm, linux-s390,
linux-efi, linux-hardening, linux-kbuild, linux-security-module,
linux-kselftest, sparclinux, llvm
In-Reply-To: <20250717231756.make.423-kees@kernel.org>
Hello:
This series was applied to riscv/linux.git (fixes)
by Kees Cook <kees@kernel.org>:
On Thu, 17 Jul 2025 16:25:05 -0700 you wrote:
> v3:
> - split up and drop __init vs inline patches that went via arch trees
> - apply feedback about preferring __init to __always_inline
> - incorporate Ritesh Harjani's patch for __init cleanups in powerpc
> - wider build testing on older compilers
> v2: https://lore.kernel.org/lkml/20250523043251.it.550-kees@kernel.org/
> v1: https://lore.kernel.org/lkml/20250507180852.work.231-kees@kernel.org/
>
> [...]
Here is the summary with links:
- [v3,01/13] stackleak: Rename STACKLEAK to KSTACK_ERASE
(no matching commit)
- [v3,02/13] stackleak: Rename stackleak_track_stack to __sanitizer_cov_stack_depth
(no matching commit)
- [v3,03/13] stackleak: Split KSTACK_ERASE_CFLAGS from GCC_PLUGINS_CFLAGS
(no matching commit)
- [v3,04/13] x86: Handle KCOV __init vs inline mismatches
(no matching commit)
- [v3,05/13] arm: Handle KCOV __init vs inline mismatches
(no matching commit)
- [v3,06/13] arm64: Handle KCOV __init vs inline mismatches
https://git.kernel.org/riscv/c/65c430906eff
- [v3,07/13] s390: Handle KCOV __init vs inline mismatches
https://git.kernel.org/riscv/c/c64d6be1a6f8
- [v3,08/13] powerpc/mm/book3s64: Move kfence and debug_pagealloc related calls to __init section
https://git.kernel.org/riscv/c/645d1b666498
- [v3,09/13] mips: Handle KCOV __init vs inline mismatch
https://git.kernel.org/riscv/c/d01daf9d95c9
- [v3,10/13] init.h: Disable sanitizer coverage for __init and __head
https://git.kernel.org/riscv/c/381a38ea53d2
- [v3,11/13] kstack_erase: Support Clang stack depth tracking
(no matching commit)
- [v3,12/13] configs/hardening: Enable CONFIG_KSTACK_ERASE
https://git.kernel.org/riscv/c/4c56d9f7e75e
- [v3,13/13] configs/hardening: Enable CONFIG_INIT_ON_FREE_DEFAULT_ON
https://git.kernel.org/riscv/c/437641a72d0a
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH] x86/bpf: use bpf_capable() instead of capable(CAP_SYS_ADMIN)
From: Serge E. Hallyn @ 2025-08-09 1:06 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Serge E. Hallyn, daniel.sneddon, Pawan Gupta, Dave Hansen,
alexandre.chartre, Ondrej Mosnacek, Alexei Starovoitov,
Daniel Borkmann, bpf, selinux, LSM List
In-Reply-To: <CAADnVQKY0z1RAJdAmRGbLWZxrJPG6Kawe6_qQHjoVM7Xz8CfuA@mail.gmail.com>
On Fri, Aug 08, 2025 at 05:46:28PM -0700, Alexei Starovoitov wrote:
> On Fri, Aug 8, 2025 at 4:59 PM Serge E. Hallyn <serge@hallyn.com> wrote:
> >
> > On Wed, Aug 06, 2025 at 08:18:55PM -0500, Serge E. Hallyn wrote:
> > > On Wed, Aug 06, 2025 at 04:31:05PM +0200, Ondrej Mosnacek wrote:
> > > > Don't check against the overloaded CAP_SYS_ADMINin do_jit(), but instead
> > > > use bpf_capable(), which checks against the more granular CAP_BPF first.
> > > > Going straight to CAP_SYS_ADMIN may cause unnecessary audit log spam
> > > > under SELinux, as privileged domains using BPF would usually only be
> > > > allowed CAP_BPF and not CAP_SYS_ADMIN.
> > > >
> > > > Link: https://bugzilla.redhat.com/show_bug.cgi?id=2369326
> > > > Fixes: d4e89d212d40 ("x86/bpf: Call branch history clearing sequence on exit")
> > > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> > >
> > > So this seems correct, *provided* that we consider it within the purview of
> > > CAP_BPF to be able to avoid clearing the branch history buffer.
>
> true, but...
>
> > >
> > > I suspect that's the case, but it might warrant discussion.
> > >
> > > Reviewed-by: Serge Hallyn <serge@hallyn.com>
> >
> > (BTW, I'm assuming this will get pulled into a BPF tree or something, and
> > doesn't need to go into the capabilities tree. Let me know if that's wrong)
>
> Right.
> scripts/get_maintainer.pl arch/x86/net/bpf_jit_comp.c
> is your friend.
>
> Pls cc author-s of the commit in question in the future.
> Adding them now.
>
> > > > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> > > > index 15672cb926fc1..2a825e5745ca1 100644
> > > > --- a/arch/x86/net/bpf_jit_comp.c
> > > > +++ b/arch/x86/net/bpf_jit_comp.c
> > > > @@ -2591,8 +2591,7 @@ emit_jmp:
> > > > seen_exit = true;
> > > > /* Update cleanup_addr */
> > > > ctx->cleanup_addr = proglen;
> > > > - if (bpf_prog_was_classic(bpf_prog) &&
> > > > - !capable(CAP_SYS_ADMIN)) {
> > > > + if (bpf_prog_was_classic(bpf_prog) && !bpf_capable()) {
>
> This looks wrong for several reasons.
>
> 1.
> bpf_capable() and CAP_BPF in general applies to eBPF only.
> There is no precedent so far to do anything differently
> for cBPF when CAP_BPF is present.
Oh. I don't see that explicitly laid out in capability.h or in the
commit message for a17b53c4a. I suspect if I were more familiar
with eBPF it would be obvious based on the detailed list of things
protected. Perhaps it should've been called CAP_EBPF...
> 2.
> commit log states that
> "privileged domains using BPF would usually only be allowed CAP_BPF
> and not CAP_SYS_ADMIN"
> which is true for eBPF only, since cBPF is always allowed for
> all unpriv users.
> Start chrome browser and you get cBPF loaded.
>
> 3.
> glancing over bugzilla it seems that the issue is
> excessive audit spam and not related to CAP_BPF and privileges.
> If so then the fix is to use
> ns_capable_noaudit(&init_user_ns, CAP_SYS_ADMIN)
Right, thank you, that seems correct. Callers with CAP_BPF don't
need to be able to avoid the barrier.
Ondrej, can you send a new patch for that?
> 4.
> I don't understand how the patch is supposed to fix the issue.
> iio-sensor-proxy is probably unpriv. Why would it use CAP_BPF?
> It's using cBPF, so there is no reason for it to have CAP_BPF.
> So capable(CAP_BPF) will fail just like capable(CAP_SYS_ADMIN),
> but since CAP_BPF check was done first, the audit won't
> be printed, because it's some undocumented internal selinux behavior ?
> None of it is in the commit log :(
>
> 5.
> And finally all that looks like a selinux bug.
> Just because something in the kernel is asking capable(CAP_SYS_ADMIN)
> there is no need to spam users with the wrong message:
> "SELinux is preventing iio-sensor-prox from using the 'sys_admin' capabilities."
> iio-sensor-prox is not trying to use 'sys_admin' capabilities.
> cBPF prog will be loaded anyway, with or without BHB clearing.
^ permalink raw reply
* Re: [PATCH] x86/bpf: use bpf_capable() instead of capable(CAP_SYS_ADMIN)
From: Alexei Starovoitov @ 2025-08-09 0:46 UTC (permalink / raw)
To: Serge E. Hallyn, daniel.sneddon, Pawan Gupta, Dave Hansen,
alexandre.chartre
Cc: Ondrej Mosnacek, Alexei Starovoitov, Daniel Borkmann, bpf,
selinux, LSM List
In-Reply-To: <aJaPQZqDIcT17aAU@mail.hallyn.com>
On Fri, Aug 8, 2025 at 4:59 PM Serge E. Hallyn <serge@hallyn.com> wrote:
>
> On Wed, Aug 06, 2025 at 08:18:55PM -0500, Serge E. Hallyn wrote:
> > On Wed, Aug 06, 2025 at 04:31:05PM +0200, Ondrej Mosnacek wrote:
> > > Don't check against the overloaded CAP_SYS_ADMINin do_jit(), but instead
> > > use bpf_capable(), which checks against the more granular CAP_BPF first.
> > > Going straight to CAP_SYS_ADMIN may cause unnecessary audit log spam
> > > under SELinux, as privileged domains using BPF would usually only be
> > > allowed CAP_BPF and not CAP_SYS_ADMIN.
> > >
> > > Link: https://bugzilla.redhat.com/show_bug.cgi?id=2369326
> > > Fixes: d4e89d212d40 ("x86/bpf: Call branch history clearing sequence on exit")
> > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> >
> > So this seems correct, *provided* that we consider it within the purview of
> > CAP_BPF to be able to avoid clearing the branch history buffer.
true, but...
> >
> > I suspect that's the case, but it might warrant discussion.
> >
> > Reviewed-by: Serge Hallyn <serge@hallyn.com>
>
> (BTW, I'm assuming this will get pulled into a BPF tree or something, and
> doesn't need to go into the capabilities tree. Let me know if that's wrong)
Right.
scripts/get_maintainer.pl arch/x86/net/bpf_jit_comp.c
is your friend.
Pls cc author-s of the commit in question in the future.
Adding them now.
> > > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> > > index 15672cb926fc1..2a825e5745ca1 100644
> > > --- a/arch/x86/net/bpf_jit_comp.c
> > > +++ b/arch/x86/net/bpf_jit_comp.c
> > > @@ -2591,8 +2591,7 @@ emit_jmp:
> > > seen_exit = true;
> > > /* Update cleanup_addr */
> > > ctx->cleanup_addr = proglen;
> > > - if (bpf_prog_was_classic(bpf_prog) &&
> > > - !capable(CAP_SYS_ADMIN)) {
> > > + if (bpf_prog_was_classic(bpf_prog) && !bpf_capable()) {
This looks wrong for several reasons.
1.
bpf_capable() and CAP_BPF in general applies to eBPF only.
There is no precedent so far to do anything differently
for cBPF when CAP_BPF is present.
2.
commit log states that
"privileged domains using BPF would usually only be allowed CAP_BPF
and not CAP_SYS_ADMIN"
which is true for eBPF only, since cBPF is always allowed for
all unpriv users.
Start chrome browser and you get cBPF loaded.
3.
glancing over bugzilla it seems that the issue is
excessive audit spam and not related to CAP_BPF and privileges.
If so then the fix is to use
ns_capable_noaudit(&init_user_ns, CAP_SYS_ADMIN)
4.
I don't understand how the patch is supposed to fix the issue.
iio-sensor-proxy is probably unpriv. Why would it use CAP_BPF?
It's using cBPF, so there is no reason for it to have CAP_BPF.
So capable(CAP_BPF) will fail just like capable(CAP_SYS_ADMIN),
but since CAP_BPF check was done first, the audit won't
be printed, because it's some undocumented internal selinux behavior ?
None of it is in the commit log :(
5.
And finally all that looks like a selinux bug.
Just because something in the kernel is asking capable(CAP_SYS_ADMIN)
there is no need to spam users with the wrong message:
"SELinux is preventing iio-sensor-prox from using the 'sys_admin' capabilities."
iio-sensor-prox is not trying to use 'sys_admin' capabilities.
cBPF prog will be loaded anyway, with or without BHB clearing.
^ permalink raw reply
* Re: [PATCH] x86/bpf: use bpf_capable() instead of capable(CAP_SYS_ADMIN)
From: Serge E. Hallyn @ 2025-08-08 23:58 UTC (permalink / raw)
To: Ondrej Mosnacek
Cc: Alexei Starovoitov, Daniel Borkmann, bpf, selinux,
linux-security-module
In-Reply-To: <aJP+/1VGbe1EcgKz@mail.hallyn.com>
On Wed, Aug 06, 2025 at 08:18:55PM -0500, Serge E. Hallyn wrote:
> On Wed, Aug 06, 2025 at 04:31:05PM +0200, Ondrej Mosnacek wrote:
> > Don't check against the overloaded CAP_SYS_ADMINin do_jit(), but instead
> > use bpf_capable(), which checks against the more granular CAP_BPF first.
> > Going straight to CAP_SYS_ADMIN may cause unnecessary audit log spam
> > under SELinux, as privileged domains using BPF would usually only be
> > allowed CAP_BPF and not CAP_SYS_ADMIN.
> >
> > Link: https://bugzilla.redhat.com/show_bug.cgi?id=2369326
> > Fixes: d4e89d212d40 ("x86/bpf: Call branch history clearing sequence on exit")
> > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
>
> So this seems correct, *provided* that we consider it within the purview of
> CAP_BPF to be able to avoid clearing the branch history buffer.
>
> I suspect that's the case, but it might warrant discussion.
>
> Reviewed-by: Serge Hallyn <serge@hallyn.com>
(BTW, I'm assuming this will get pulled into a BPF tree or something, and
doesn't need to go into the capabilities tree. Let me know if that's wrong)
> > ---
> > arch/x86/net/bpf_jit_comp.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> > index 15672cb926fc1..2a825e5745ca1 100644
> > --- a/arch/x86/net/bpf_jit_comp.c
> > +++ b/arch/x86/net/bpf_jit_comp.c
> > @@ -2591,8 +2591,7 @@ emit_jmp:
> > seen_exit = true;
> > /* Update cleanup_addr */
> > ctx->cleanup_addr = proglen;
> > - if (bpf_prog_was_classic(bpf_prog) &&
> > - !capable(CAP_SYS_ADMIN)) {
> > + if (bpf_prog_was_classic(bpf_prog) && !bpf_capable()) {
> > u8 *ip = image + addrs[i - 1];
> >
> > if (emit_spectre_bhb_barrier(&prog, ip, bpf_prog))
> > --
> > 2.50.1
> >
^ permalink raw reply
* Re: [RFC PATCH 2/2] memfd: call security_inode_init_security_anon
From: Stephen Smalley @ 2025-08-08 12:41 UTC (permalink / raw)
To: Thiébaud Weksteen
Cc: Paul Moore, James Morris, Hugh Dickins, Jeff Vander Stoep,
Nick Kralevich, Jeff Xu, linux-kernel, linux-security-module,
selinux
In-Reply-To: <CAEjxPJ5nC7s=+Os4+9XjkzhGTyaNVrCyJgx+rz5n3baRFWVrzA@mail.gmail.com>
On Fri, Aug 8, 2025 at 7:57 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Thu, Aug 7, 2025 at 3:57 AM 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.
>
> To provide backward compatibility, I would anticipate that you will
> need to define a new SELinux policy capability and make this change
> conditional on it being enabled, see:
> https://github.com/SELinuxProject/selinux-kernel/wiki/Getting-Started#adding-a-new-selinux-policy-capability
> for instructions and links to examples.
>
> Otherwise, see below.
>
> >
> > [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>
> > ---
> > mm/memfd.c | 16 ++++++++++++++--
> > security/selinux/hooks.c | 15 +++++++++++----
> > security/selinux/include/classmap.h | 2 ++
> > 3 files changed, 27 insertions(+), 6 deletions(-)
> >
> > diff --git a/mm/memfd.c b/mm/memfd.c
> > index bbe679895ef6..13bff0e91816 100644
> > --- a/mm/memfd.c
> > +++ b/mm/memfd.c
> > @@ -433,6 +433,9 @@ static struct file *alloc_file(const char *name, unsigned int flags)
> > {
> > unsigned int *file_seals;
> > struct file *file;
> > + struct inode *inode;
> > + int err = 0;
> > + const char *anon_name = "[memfd]";
> >
> > if (flags & MFD_HUGETLB) {
> > file = hugetlb_file_setup(name, 0, VM_NORESERVE,
> > @@ -444,12 +447,21 @@ static struct file *alloc_file(const char *name, unsigned int flags)
> > }
> > if (IS_ERR(file))
> > return file;
> > +
> > + inode = file_inode(file);
> > + err = security_inode_init_security_anon(inode,
> > + LSM_ANON_INODE_MEMFD,
> > + &QSTR(anon_name), NULL);
>
> Since the anon_name already indicates that this is a memfd, so can't
> you already distinguish these via name-based type_transition rules?
> Why do we need the enum argument?
On second thought, I see that you are distinguishing not just the
security context/type but also the security class, but the question
remains: can't you compare the name to make this determination?
>
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 8d36d5ebb6e5..49742930e706 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -2367,8 +2367,8 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm)
> > ad.u.file = bprm->file;
> >
> > if (new_tsec->sid == old_tsec->sid) {
> > - rc = avc_has_perm(old_tsec->sid, isec->sid,
> > - SECCLASS_FILE, FILE__EXECUTE_NO_TRANS, &ad);
> > + rc = avc_has_perm(old_tsec->sid, isec->sid, isec->sclass,
> > + FILE__EXECUTE_NO_TRANS, &ad);
>
> Here and below I am a little concerned that we could end up reaching
> this code on an inode with an isec->sclass that does not define the
> execute_no_trans and entrypoint permissions. We should do something to
> make that never happens, or check for it and always deny in that case.
>
> > if (rc)
> > return rc;
> > } else {
> > @@ -2378,8 +2378,8 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm)
> > if (rc)
> > return rc;
> >
> > - rc = avc_has_perm(new_tsec->sid, isec->sid,
> > - SECCLASS_FILE, FILE__ENTRYPOINT, &ad);
> > + rc = avc_has_perm(new_tsec->sid, isec->sid, isec->sclass,
> > + FILE__ENTRYPOINT, &ad);
> > if (rc)
> > return rc;
> >
> > @@ -2997,6 +2997,13 @@ static int selinux_inode_init_security_anon(struct inode *inode,
> >
> > isec->sclass = context_isec->sclass;
> > isec->sid = context_isec->sid;
> > + } else if (type == LSM_ANON_INODE_MEMFD) {
> > + isec->sclass = SECCLASS_MEMFD_FILE;
> > + rc = security_transition_sid(
> > + sid, sid,
> > + isec->sclass, name, &isec->sid);
>
> Again, name-based type_transitions ought to be able to distinguish
> memfd based on the name argument IIUC.
^ permalink raw reply
* Re: [RFC PATCH 1/2] lsm: add type to security_inode_init_security_anon
From: Stephen Smalley @ 2025-08-08 12:00 UTC (permalink / raw)
To: Thiébaud Weksteen
Cc: Paul Moore, James Morris, Hugh Dickins, Jeff Vander Stoep,
Nick Kralevich, Jeff Xu, linux-kernel, linux-security-module,
selinux
In-Reply-To: <20250807075647.755848-1-tweek@google.com>
On Thu, Aug 7, 2025 at 3:57 AM Thiébaud Weksteen <tweek@google.com> wrote:
>
> Introduce a new enum (lsm_anon_inode_id) to identify the type of
> anonymous inode being created. This enum is passed down to the
> security_inode_init_security_anon LSM hook.
The intent was to distinguish the type of anonymous inode based on
name-based type_transition rules in the policy using the provided name
string. If that doesn't suffice, this might be justified, but the next
patch doesn't appear to justify it.
>
> Signed-off-by: Thiébaud Weksteen <tweek@google.com>
> ---
> fs/anon_inodes.c | 5 +++--
> include/linux/lsm_hook_defs.h | 3 ++-
> include/linux/security.h | 8 ++++++++
> security/security.c | 4 +++-
> security/selinux/hooks.c | 1 +
> 5 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> index 1d847a939f29..9a2f09808f86 100644
> --- a/fs/anon_inodes.c
> +++ b/fs/anon_inodes.c
> @@ -21,6 +21,7 @@
> #include <linux/magic.h>
> #include <linux/anon_inodes.h>
> #include <linux/pseudo_fs.h>
> +#include <linux/security.h>
>
> #include <linux/uaccess.h>
>
> @@ -121,8 +122,8 @@ struct inode *anon_inode_make_secure_inode(struct super_block *sb, const char *n
> return inode;
> inode->i_flags &= ~S_PRIVATE;
> inode->i_op = &anon_inode_operations;
> - error = security_inode_init_security_anon(inode, &QSTR(name),
> - context_inode);
> + error = security_inode_init_security_anon(inode, LSM_ANON_INODE_GENERIC,
> + &QSTR(name), context_inode);
> if (error) {
> iput(inode);
> return ERR_PTR(error);
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index fd11fffdd3c3..1634f41f7a3c 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -119,7 +119,8 @@ LSM_HOOK(int, -EOPNOTSUPP, inode_init_security, struct inode *inode,
> struct inode *dir, const struct qstr *qstr, struct xattr *xattrs,
> int *xattr_count)
> LSM_HOOK(int, 0, inode_init_security_anon, struct inode *inode,
> - const struct qstr *name, const struct inode *context_inode)
> + enum lsm_anon_inode_id type, const struct qstr *name,
> + const struct inode *context_inode)
> LSM_HOOK(int, 0, inode_create, struct inode *dir, struct dentry *dentry,
> umode_t mode)
> LSM_HOOK(void, LSM_RET_VOID, inode_post_create_tmpfile, struct mnt_idmap *idmap,
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 521bcb5b9717..98a97b8a1093 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -263,6 +263,12 @@ struct request_sock;
> #define LSM_UNSAFE_PTRACE 2
> #define LSM_UNSAFE_NO_NEW_PRIVS 4
>
> +/* anon_inode types */
> +enum lsm_anon_inode_id {
> + LSM_ANON_INODE_GENERIC,
> + LSM_ANON_INODE_MEMFD
> +};
> +
> #ifdef CONFIG_MMU
> extern int mmap_min_addr_handler(const struct ctl_table *table, int write,
> void *buffer, size_t *lenp, loff_t *ppos);
> @@ -402,6 +408,7 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
> const struct qstr *qstr,
> initxattrs initxattrs, void *fs_data);
> int security_inode_init_security_anon(struct inode *inode,
> + enum lsm_anon_inode_id type,
> const struct qstr *name,
> const struct inode *context_inode);
> int security_inode_create(struct inode *dir, struct dentry *dentry, umode_t mode);
> @@ -889,6 +896,7 @@ static inline int security_inode_init_security(struct inode *inode,
> }
>
> static inline int security_inode_init_security_anon(struct inode *inode,
> + enum lsm_anon_inode_id type,
> const struct qstr *name,
> const struct inode *context_inode)
> {
> diff --git a/security/security.c b/security/security.c
> index ad163f06bf7a..09aa858819a2 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1861,6 +1861,7 @@ EXPORT_SYMBOL(security_inode_init_security);
> /**
> * security_inode_init_security_anon() - Initialize an anonymous inode
> * @inode: the inode
> + * @type: the type of anonymous inode
> * @name: the anonymous inode class
> * @context_inode: an optional related inode
> *
> @@ -1871,10 +1872,11 @@ EXPORT_SYMBOL(security_inode_init_security);
> * creation of this inode, or another -errno upon other errors.
> */
> int security_inode_init_security_anon(struct inode *inode,
> + enum lsm_anon_inode_id type,
> const struct qstr *name,
> const struct inode *context_inode)
> {
> - return call_int_hook(inode_init_security_anon, inode, name,
> + return call_int_hook(inode_init_security_anon, inode, type, name,
> context_inode);
> }
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index c95a5874bf7d..8d36d5ebb6e5 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2967,6 +2967,7 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
> }
>
> static int selinux_inode_init_security_anon(struct inode *inode,
> + enum lsm_anon_inode_id type,
> const struct qstr *name,
> const struct inode *context_inode)
> {
> --
> 2.50.1.703.g449372360f-goog
>
^ permalink raw reply
* Re: [RFC PATCH 2/2] memfd: call security_inode_init_security_anon
From: Stephen Smalley @ 2025-08-08 11:57 UTC (permalink / raw)
To: Thiébaud Weksteen
Cc: Paul Moore, James Morris, Hugh Dickins, Jeff Vander Stoep,
Nick Kralevich, Jeff Xu, linux-kernel, linux-security-module,
selinux
In-Reply-To: <20250807075745.756415-1-tweek@google.com>
On Thu, Aug 7, 2025 at 3:57 AM 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.
To provide backward compatibility, I would anticipate that you will
need to define a new SELinux policy capability and make this change
conditional on it being enabled, see:
https://github.com/SELinuxProject/selinux-kernel/wiki/Getting-Started#adding-a-new-selinux-policy-capability
for instructions and links to examples.
Otherwise, see below.
>
> [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>
> ---
> mm/memfd.c | 16 ++++++++++++++--
> security/selinux/hooks.c | 15 +++++++++++----
> security/selinux/include/classmap.h | 2 ++
> 3 files changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/mm/memfd.c b/mm/memfd.c
> index bbe679895ef6..13bff0e91816 100644
> --- a/mm/memfd.c
> +++ b/mm/memfd.c
> @@ -433,6 +433,9 @@ static struct file *alloc_file(const char *name, unsigned int flags)
> {
> unsigned int *file_seals;
> struct file *file;
> + struct inode *inode;
> + int err = 0;
> + const char *anon_name = "[memfd]";
>
> if (flags & MFD_HUGETLB) {
> file = hugetlb_file_setup(name, 0, VM_NORESERVE,
> @@ -444,12 +447,21 @@ static struct file *alloc_file(const char *name, unsigned int flags)
> }
> if (IS_ERR(file))
> return file;
> +
> + inode = file_inode(file);
> + err = security_inode_init_security_anon(inode,
> + LSM_ANON_INODE_MEMFD,
> + &QSTR(anon_name), NULL);
Since the anon_name already indicates that this is a memfd, so can't
you already distinguish these via name-based type_transition rules?
Why do we need the enum argument?
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 8d36d5ebb6e5..49742930e706 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2367,8 +2367,8 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm)
> ad.u.file = bprm->file;
>
> if (new_tsec->sid == old_tsec->sid) {
> - rc = avc_has_perm(old_tsec->sid, isec->sid,
> - SECCLASS_FILE, FILE__EXECUTE_NO_TRANS, &ad);
> + rc = avc_has_perm(old_tsec->sid, isec->sid, isec->sclass,
> + FILE__EXECUTE_NO_TRANS, &ad);
Here and below I am a little concerned that we could end up reaching
this code on an inode with an isec->sclass that does not define the
execute_no_trans and entrypoint permissions. We should do something to
make that never happens, or check for it and always deny in that case.
> if (rc)
> return rc;
> } else {
> @@ -2378,8 +2378,8 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm)
> if (rc)
> return rc;
>
> - rc = avc_has_perm(new_tsec->sid, isec->sid,
> - SECCLASS_FILE, FILE__ENTRYPOINT, &ad);
> + rc = avc_has_perm(new_tsec->sid, isec->sid, isec->sclass,
> + FILE__ENTRYPOINT, &ad);
> if (rc)
> return rc;
>
> @@ -2997,6 +2997,13 @@ static int selinux_inode_init_security_anon(struct inode *inode,
>
> isec->sclass = context_isec->sclass;
> isec->sid = context_isec->sid;
> + } else if (type == LSM_ANON_INODE_MEMFD) {
> + isec->sclass = SECCLASS_MEMFD_FILE;
> + rc = security_transition_sid(
> + sid, sid,
> + isec->sclass, name, &isec->sid);
Again, name-based type_transitions ought to be able to distinguish
memfd based on the name argument IIUC.
^ permalink raw reply
* Re: [RFC PATCH 0/6] fs/9p: Reuse inode based on path (in addition to qid)
From: Christian Schoenebeck @ 2025-08-08 10:52 UTC (permalink / raw)
To: Tingmao Wang, Dominique Martinet
Cc: Eric Van Hensbergen, Latchesar Ionkov, v9fs,
Mickaël Salaün, Günther Noack,
linux-security-module, Jan Kara, Amir Goldstein,
Matthew Bobrowski, linux-fsdevel
In-Reply-To: <aJXRAzCqTrY4aVEP@codewreck.org>
On Friday, August 8, 2025 12:27:15 PM CEST Dominique Martinet wrote:
> Sorry for the delay...
>
> Tingmao Wang wrote on Sun, Apr 06, 2025 at 09:43:01PM +0100:
> > Unrelated to the above problem, it also seems like even with the revert in
> > [2], because in cached mode inode are still reused based on qid (and type,
> > version (aka mtime), etc), the setup mentioned in [2] still causes
> > problems in th latest kernel with cache=loose:
>
> cache=loose is "you're on your own", I think it's fine to keep as is,
> especially given qemu can handle it with multidevs=remap if required
As of QEMU 10.0, multidevs=remap (i.e. remapping inodes from host to guest) is
now the default behaviour, since inode collisions were constantly causing
issues and confusion among 9p users.
And yeah, cache=loose means 9p client is blind for whatever changes on 9p
server side.
/Christian
^ permalink raw reply
* Re: [RFC PATCH 0/6] fs/9p: Reuse inode based on path (in addition to qid)
From: Dominique Martinet @ 2025-08-08 10:27 UTC (permalink / raw)
To: Tingmao Wang
Cc: Eric Van Hensbergen, Latchesar Ionkov, Christian Schoenebeck,
v9fs, Mickaël Salaün, Günther Noack,
linux-security-module, Jan Kara, Amir Goldstein,
Matthew Bobrowski, linux-fsdevel
In-Reply-To: <cover.1743971855.git.m@maowtm.org>
Sorry for the delay...
Tingmao Wang wrote on Sun, Apr 06, 2025 at 09:43:01PM +0100:
> Unrelated to the above problem, it also seems like even with the revert in
> [2], because in cached mode inode are still reused based on qid (and type,
> version (aka mtime), etc), the setup mentioned in [2] still causes
> problems in th latest kernel with cache=loose:
cache=loose is "you're on your own", I think it's fine to keep as is,
especially given qemu can handle it with multidevs=remap if required
> With the above in mind, I have a proposal for 9pfs to:
> 1. Reuse inodes even in uncached mode
> 2. However, reuse them based on qid.path AND the actual pathname, by doing
> the appropriate testing in v9fs_test(_new)?_inode(_dotl)?
I think that's fine for cache=none, but it breaks hardlinks on
cache=loose so I think this ought to only be done without cache
(I haven't really played with the cache flag bits, not check pathname if
any of loose, writeback or metadata are set?)
> The main problem here is how to store the pathname in a sensible way and
> tie it to the inode. For now I opted with an array of names acquired with
> take_dentry_name_snapshot, which reuses the same memory as the dcache to
> store the actual strings, but doesn't tie the lifetime of the dentry with
> the inode (I thought about holding a reference to the dentry in the
> v9fs_inode, but it seemed like a wrong approach and would cause dentries
> to not be evicted/released).
That's pretty hard to get right and I wish we had more robust testing
there... But I guess that's appropriate enough.
I know Atos has done an implementation that keeps the full path
somewhere to re-open fids in case of server reconnections, but that code
has never been submitted upstream that I can see so I can't check how
they used to store the path :/ Ohwell.
> Storing one pathname per inode also means we don't reuse the same inode
> for hardlinks -- maybe this can be fixed as well in a future version, if
> this approach sounds good?
Ah, you pointed it out yourself. I don't see how we could fix that, as
we have no way other than the qid to identify hard links; so this really
ought to depend on cache level if you want to support landlock/*notify
in cache=none.
Frankly the *notify use-case is difficult to support properly, as files
can change from under us (e.g. modifying the file directly on the host
in qemu case, or just multiple mounts of the same directory), so it
can't be relied on in the general case anyway -- 9p doesn't have
anything like NFSv4 leases to get notified when other clients write a
file we "own", so whatever we do will always be limited...
But I guess it can make sense for limited monitoring e.g. rebuilding
something on change and things like that?
--
Dominique Martinet | Asmadeus
^ permalink raw reply
* Re: [RFC PATCH 1/6] fs/9p: Add ability to identify inode by path for .L
From: Mickaël Salaün @ 2025-08-08 8:32 UTC (permalink / raw)
To: Tingmao Wang
Cc: Al Viro, Eric Van Hensbergen, Dominique Martinet,
Latchesar Ionkov, Christian Schoenebeck, v9fs, Günther Noack,
linux-security-module, Jan Kara, Amir Goldstein,
Matthew Bobrowski, linux-fsdevel
In-Reply-To: <b32e2088-92c0-43e0-8c90-cb20d4567973@maowtm.org>
On Fri, Jul 11, 2025 at 08:11:44PM +0100, Tingmao Wang wrote:
> Hi Al, thanks for the review :) I haven't had the chance to properly
> think about this until today, so apologies for the delay.
>
> On 7/5/25 01:19, Al Viro wrote:
> > On Sun, Apr 06, 2025 at 09:43:02PM +0100, Tingmao Wang wrote:
> >
> >> +struct v9fs_ino_path *make_ino_path(struct dentry *dentry)
> >> +{
> >> + struct v9fs_ino_path *path;
> >> + size_t path_components = 0;
> >> + struct dentry *curr = dentry;
> >> + ssize_t i;
> >> +
> >> + lockdep_assert_held_read(&v9fs_dentry2v9ses(dentry)->rename_sem);
> >> +
> >> + rcu_read_lock();
> >> +
> >> + /* Don't include the root dentry */
> >> + while (curr->d_parent != curr) {
> >> + path_components++;
> >> + curr = curr->d_parent;
> >> + }
> >> + if (WARN_ON(path_components > SSIZE_MAX)) {
>
> (Looking at this again I think this check is a bit bogus. I don't know
> how would it be possible at all for us to have > SSIZE_MAX deep
> directories especially since each level requires a dentry allocation, but
> even if this check is actually useful, it should be in the while loop,
> before each path_components++)
WARN_ON_ONCE() would be better, especially in a while loop. I avoid
using WARN_ON(), especially when that can be triggered by users.
>
> >> + rcu_read_unlock();
> >> + return NULL;
> >> + }
> >> +
> >> + path = kmalloc(struct_size(path, names, path_components),
> >> + GFP_KERNEL);
> >
> > Blocking allocation under rcu_read_lock().
>
> I think my first instinct of how to fix this, if the original code is
> correct barring this allocation issue, would be to take rcu read lock
> twice (first walk to calculate how much to allocate, then second walk to
> actually take the snapshots). We should be safe to rcu_read_unlock() in
> the middle as long as caller has a reference to the target dentry (this
> needs to be true even if we just do one rcu_read_lock() anyway), and we
> can start a parent walk again. The v9fs rename_sem should ensure we see
> the same path again.
>
> Alternatively, we can use dget_parent to do the walk, and not lock RCU at
> all. We still need to walk twice tho, to know how much to allocate. But
> for now I will keep the current approach.
>
> New version:
>
> /*
> * Must hold rename_sem due to traversing parents. Caller must hold
> * reference to dentry.
> */
> struct v9fs_ino_path *make_ino_path(struct dentry *dentry)
> {
> struct v9fs_ino_path *path;
> size_t path_components = 0;
> struct dentry *curr = dentry;
> ssize_t i;
>
> lockdep_assert_held_read(&v9fs_dentry2v9ses(dentry)->rename_sem);
> might_sleep(); /* Allocation below might block */
>
> rcu_read_lock();
>
> /* Don't include the root dentry */
> while (curr->d_parent != curr) {
> if (WARN_ON(path_components >= SSIZE_MAX)) {
> rcu_read_unlock();
> return NULL;
> }
> path_components++;
> curr = curr->d_parent;
> }
>
> /*
> * Allocation can block so don't do it in RCU (and because the
> * allocation might be large, since name_snapshot leaves space for
> * inline str, not worth trying GFP_ATOMIC)
> */
> rcu_read_unlock();
>
> path = kmalloc(struct_size(path, names, path_components), GFP_KERNEL);
> if (!path) {
> rcu_read_unlock();
This unlock is wrong.
> return NULL;
> }
>
> path->nr_components = path_components;
> curr = dentry;
>
> rcu_read_lock();
> for (i = path_components - 1; i >= 0; i--) {
> take_dentry_name_snapshot(&path->names[i], curr);
> curr = curr->d_parent;
> }
> WARN_ON(curr != curr->d_parent);
> rcu_read_unlock();
> return path;
> }
>
> How does this look?
Looks good to me overall. Please sent a new patch series.
>
> On 7/5/25 01:25, Al Viro wrote:
> > On Sun, Apr 06, 2025 at 09:43:02PM +0100, Tingmao Wang wrote:
> >> +bool ino_path_compare(struct v9fs_ino_path *ino_path,
> >> + struct dentry *dentry)
> >> +{
> >> + struct dentry *curr = dentry;
> >> + struct qstr *curr_name;
> >> + struct name_snapshot *compare;
> >> + ssize_t i;
> >> +
> >> + lockdep_assert_held_read(&v9fs_dentry2v9ses(dentry)->rename_sem);
> >> +
> >> + rcu_read_lock();
> >> + for (i = ino_path->nr_components - 1; i >= 0; i--) {
> >> + if (curr->d_parent == curr) {
> >> + /* We're supposed to have more components to walk */
> >> + rcu_read_unlock();
> >> + return false;
> >> + }
> >> + curr_name = &curr->d_name;
> >> + compare = &ino_path->names[i];
> >> + /*
> >> + * We can't use hash_len because it is salted with the parent
> >> + * dentry pointer. We could make this faster by pre-computing our
> >> + * own hashlen for compare and ino_path outside, probably.
> >> + */
> >> + if (curr_name->len != compare->name.len) {
> >> + rcu_read_unlock();
> >> + return false;
> >> + }
> >> + if (strncmp(curr_name->name, compare->name.name,
> >> + curr_name->len) != 0) {
> >
> > ... without any kind of protection for curr_name. Incidentally,
> > what about rename()? Not a cross-directory one, just one that
> > changes the name of a subdirectory within the same parent?
>
> As far as I can tell, in v9fs_vfs_rename, v9ses->rename_sem is taken for
> both same-parent and different parent renames, so I think we're safe here
> (and hopefully for any v9fs dentries, nobody should be causing d_name to
> change except for ourselves when we call d_move in v9fs_vfs_rename? If
> yes then because we also take v9ses->rename_sem, in theory we should be
> fine here...?)
A lockdep_assert_held() or similar and a comment would make this clear.
>
> (Let me know if I missed anything. I'm assuming only the filesystem
> "owning" a dentry should d_move/d_exchange the dentry.)
>
> However, I see that there is a d_same_name function in dcache.c which is
> slightly more careful (but still requires the caller to check the dentry
> seqcount, which we do not need to because of the reasoning above), and in
> hindsight I think that is probably the more proper way to do this
> comparison (and will also handle case-insensitivity, although I've not
> explored if this is applicable to 9pfs).
>
> New version:
>
> /*
> * Must hold rename_sem due to traversing parents
> */
> bool ino_path_compare(struct v9fs_ino_path *ino_path, struct dentry *dentry)
> {
> struct dentry *curr = dentry;
> struct name_snapshot *compare;
> ssize_t i;
>
> lockdep_assert_held_read(&v9fs_dentry2v9ses(dentry)->rename_sem);
>
> rcu_read_lock();
> for (i = ino_path->nr_components - 1; i >= 0; i--) {
> if (curr->d_parent == curr) {
> /* We're supposed to have more components to walk */
> rcu_read_unlock();
> return false;
> }
> compare = &ino_path->names[i];
> if (!d_same_name(curr, curr->d_parent, &compare->name)) {
> rcu_read_unlock();
> return false;
> }
> curr = curr->d_parent;
> }
> rcu_read_unlock();
> if (curr != curr->d_parent) {
> /* dentry is deeper than ino_path */
> return false;
> }
> return true;
> }
I like this new version.
>
> If you think this is not enough, can you suggest what would be needed?
> I'm thinking maybe we can check dentry seqcount to be safe, but from
> earlier discussion in "bpf path iterator" my impression is that that is
> VFS internal data - can we use it here (if needed)?
>
> (I assume, from looking at the code, just having a reference does not
> prevent d_name from changing)
>
>
^ permalink raw reply
* Re: [PATCH V9 1/7] KVM: guest_memfd: Use guest mem inodes instead of anonymous inodes
From: Ackerley Tng @ 2025-08-07 22:14 UTC (permalink / raw)
To: David Hildenbrand, Shivank Garg, seanjc, vbabka, willy, akpm,
shuah, pbonzini, brauner, viro
Cc: paul, jmorris, serge, pvorel, bfoster, tabba, vannapurve,
chao.gao, bharata, nikunj, michael.day, shdhiman, yan.y.zhao,
Neeraj.Upadhyay, thomas.lendacky, michael.roth, aik, jgg,
kalyazin, peterx, jack, rppt, hch, cgzones, ira.weiny, rientjes,
roypat, ziy, matthew.brost, joshua.hahnjy, rakie.kim, byungchul,
gourry, kent.overstreet, ying.huang, apopple, chao.p.peng, amit,
ddutile, dan.j.williams, ashish.kalra, gshan, jgowans,
pankaj.gupta, papaluri, yuzhao, suzuki.poulose, quic_eberman,
aneeshkumar.kizhakeveetil, linux-fsdevel, linux-mm, linux-kernel,
linux-security-module, kvm, linux-kselftest, linux-coco
In-Reply-To: <diqz4iui4y00.fsf@ackerleytng-ctop.c.googlers.com>
Ackerley Tng <ackerleytng@google.com> writes:
> David Hildenbrand <david@redhat.com> writes:
>
[snip]
>>
>> Nothing else jumped at me.
>>
>
> Thanks for the review!
>
> Since we're going to submit this patch through Shivank's mempolicy
> support series, I'll follow up soon by sending a replacement patch in
> reply to this series so Shivank could build on top of that?
>
>> --
>> Cheers,
>>
>> David / dhildenb
I hope sending a patch within a reply this way works!
---
From 11845fed725ff68c3bad07cd9c717ae968465bf4 Mon Sep 17 00:00:00 2001
Message-ID: <11845fed725ff68c3bad07cd9c717ae968465bf4.1754603750.git.ackerleytng@google.com>
From: Ackerley Tng <ackerleytng@google.com>
Date: Sun, 13 Jul 2025 17:43:35 +0000
Subject: [PATCH 1/1] KVM: guest_memfd: Use guest mem inodes instead of
anonymous inodes
guest_memfd's inode represents memory the guest_memfd is
providing. guest_memfd's file represents a struct kvm's view of that
memory.
Using a custom inode allows customization of the inode teardown
process via callbacks. For example, ->evict_inode() allows
customization of the truncation process on file close, and
->destroy_inode() and ->free_inode() allow customization of the inode
freeing process.
Customizing the truncation process allows flexibility in management of
guest_memfd memory and customization of the inode freeing process
allows proper cleanup of memory metadata stored on the inode.
Memory metadata is more appropriately stored on the inode (as opposed
to the file), since the metadata is for the memory and is not unique
to a specific binding and struct kvm.
Co-developed-by: Fuad Tabba <tabba@google.com>
Change-Id: I64925f069637323023fbff91fc8521f92b8561bd
Signed-off-by: Fuad Tabba <tabba@google.com>
Signed-off-by: Shivank Garg <shivankg@amd.com>
Signed-off-by: Ackerley Tng <ackerleytng@google.com>
---
include/uapi/linux/magic.h | 1 +
virt/kvm/guest_memfd.c | 128 ++++++++++++++++++++++++++++++-------
virt/kvm/kvm_main.c | 7 +-
virt/kvm/kvm_mm.h | 9 +--
4 files changed, 118 insertions(+), 27 deletions(-)
diff --git a/include/uapi/linux/magic.h b/include/uapi/linux/magic.h
index bb575f3ab45e5..638ca21b7a909 100644
--- a/include/uapi/linux/magic.h
+++ b/include/uapi/linux/magic.h
@@ -103,5 +103,6 @@
#define DEVMEM_MAGIC 0x454d444d /* "DMEM" */
#define SECRETMEM_MAGIC 0x5345434d /* "SECM" */
#define PID_FS_MAGIC 0x50494446 /* "PIDF" */
+#define GUEST_MEMFD_MAGIC 0x474d454d /* "GMEM" */
#endif /* __LINUX_MAGIC_H__ */
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index 08a6bc7d25b60..0e93323fc8392 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -1,12 +1,16 @@
// SPDX-License-Identifier: GPL-2.0
+#include <linux/anon_inodes.h>
#include <linux/backing-dev.h>
#include <linux/falloc.h>
+#include <linux/fs.h>
#include <linux/kvm_host.h>
+#include <linux/pseudo_fs.h>
#include <linux/pagemap.h>
-#include <linux/anon_inodes.h>
#include "kvm_mm.h"
+static struct vfsmount *kvm_gmem_mnt;
+
struct kvm_gmem {
struct kvm *kvm;
struct xarray bindings;
@@ -385,9 +389,45 @@ static struct file_operations kvm_gmem_fops = {
.fallocate = kvm_gmem_fallocate,
};
-void kvm_gmem_init(struct module *module)
+static int kvm_gmem_init_fs_context(struct fs_context *fc)
+{
+ if (!init_pseudo(fc, GUEST_MEMFD_MAGIC))
+ return -ENOMEM;
+
+ fc->s_iflags |= SB_I_NOEXEC;
+ fc->s_iflags |= SB_I_NODEV;
+
+ return 0;
+}
+
+static struct file_system_type kvm_gmem_fs = {
+ .name = "guest_memfd",
+ .init_fs_context = kvm_gmem_init_fs_context,
+ .kill_sb = kill_anon_super,
+};
+
+static int kvm_gmem_init_mount(void)
+{
+ kvm_gmem_mnt = kern_mount(&kvm_gmem_fs);
+
+ if (IS_ERR(kvm_gmem_mnt))
+ return PTR_ERR(kvm_gmem_mnt);
+
+ kvm_gmem_mnt->mnt_flags |= MNT_NOEXEC;
+ return 0;
+}
+
+int kvm_gmem_init(struct module *module)
{
kvm_gmem_fops.owner = module;
+
+ return kvm_gmem_init_mount();
+}
+
+void kvm_gmem_exit(void)
+{
+ kern_unmount(kvm_gmem_mnt);
+ kvm_gmem_mnt = NULL;
}
static int kvm_gmem_migrate_folio(struct address_space *mapping,
@@ -463,11 +503,71 @@ bool __weak kvm_arch_supports_gmem_mmap(struct kvm *kvm)
return true;
}
+static struct inode *kvm_gmem_inode_make_secure_inode(const char *name,
+ loff_t size, u64 flags)
+{
+ struct inode *inode;
+
+ inode = anon_inode_make_secure_inode(kvm_gmem_mnt->mnt_sb, name, NULL);
+ if (IS_ERR(inode))
+ return inode;
+
+ inode->i_private = (void *)(unsigned long)flags;
+ inode->i_op = &kvm_gmem_iops;
+ inode->i_mapping->a_ops = &kvm_gmem_aops;
+ inode->i_mode |= S_IFREG;
+ inode->i_size = size;
+ mapping_set_gfp_mask(inode->i_mapping, GFP_HIGHUSER);
+ mapping_set_inaccessible(inode->i_mapping);
+ /* Unmovable mappings are supposed to be marked unevictable as well. */
+ WARN_ON_ONCE(!mapping_unevictable(inode->i_mapping));
+
+ return inode;
+}
+
+static struct file *kvm_gmem_inode_create_getfile(void *priv, loff_t size,
+ u64 flags)
+{
+ static const char *name = "[kvm-gmem]";
+ struct inode *inode;
+ struct file *file;
+ int err;
+
+ err = -ENOENT;
+ if (!try_module_get(kvm_gmem_fops.owner))
+ goto err;
+
+ inode = kvm_gmem_inode_make_secure_inode(name, size, flags);
+ if (IS_ERR(inode)) {
+ err = PTR_ERR(inode);
+ goto err_put_module;
+ }
+
+ file = alloc_file_pseudo(inode, kvm_gmem_mnt, name, O_RDWR,
+ &kvm_gmem_fops);
+ if (IS_ERR(file)) {
+ err = PTR_ERR(file);
+ goto err_put_inode;
+ }
+
+ file->f_flags |= O_LARGEFILE;
+ file->private_data = priv;
+
+out:
+ return file;
+
+err_put_inode:
+ iput(inode);
+err_put_module:
+ module_put(kvm_gmem_fops.owner);
+err:
+ file = ERR_PTR(err);
+ goto out;
+}
+
static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags)
{
- const char *anon_name = "[kvm-gmem]";
struct kvm_gmem *gmem;
- struct inode *inode;
struct file *file;
int fd, err;
@@ -481,32 +581,16 @@ static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags)
goto err_fd;
}
- file = anon_inode_create_getfile(anon_name, &kvm_gmem_fops, gmem,
- O_RDWR, NULL);
+ file = kvm_gmem_inode_create_getfile(gmem, size, flags);
if (IS_ERR(file)) {
err = PTR_ERR(file);
goto err_gmem;
}
- file->f_flags |= O_LARGEFILE;
-
- inode = file->f_inode;
- WARN_ON(file->f_mapping != inode->i_mapping);
-
- inode->i_private = (void *)(unsigned long)flags;
- inode->i_op = &kvm_gmem_iops;
- inode->i_mapping->a_ops = &kvm_gmem_aops;
- inode->i_mode |= S_IFREG;
- inode->i_size = size;
- mapping_set_gfp_mask(inode->i_mapping, GFP_HIGHUSER);
- mapping_set_inaccessible(inode->i_mapping);
- /* Unmovable mappings are supposed to be marked unevictable as well. */
- WARN_ON_ONCE(!mapping_unevictable(inode->i_mapping));
-
kvm_get_kvm(kvm);
gmem->kvm = kvm;
xa_init(&gmem->bindings);
- list_add(&gmem->entry, &inode->i_mapping->i_private_list);
+ list_add(&gmem->entry, &file_inode(file)->i_mapping->i_private_list);
fd_install(fd, file);
return fd;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 18f29ef935437..301d48d6e00d0 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -6489,7 +6489,9 @@ int kvm_init(unsigned vcpu_size, unsigned vcpu_align, struct module *module)
if (WARN_ON_ONCE(r))
goto err_vfio;
- kvm_gmem_init(module);
+ r = kvm_gmem_init(module);
+ if (r)
+ goto err_gmem;
r = kvm_init_virtualization();
if (r)
@@ -6510,6 +6512,8 @@ int kvm_init(unsigned vcpu_size, unsigned vcpu_align, struct module *module)
err_register:
kvm_uninit_virtualization();
err_virt:
+ kvm_gmem_exit();
+err_gmem:
kvm_vfio_ops_exit();
err_vfio:
kvm_async_pf_deinit();
@@ -6541,6 +6545,7 @@ void kvm_exit(void)
for_each_possible_cpu(cpu)
free_cpumask_var(per_cpu(cpu_kick_mask, cpu));
kmem_cache_destroy(kvm_vcpu_cache);
+ kvm_gmem_exit();
kvm_vfio_ops_exit();
kvm_async_pf_deinit();
kvm_irqfd_exit();
diff --git a/virt/kvm/kvm_mm.h b/virt/kvm/kvm_mm.h
index 31defb08ccbab..9fcc5d5b7f8d0 100644
--- a/virt/kvm/kvm_mm.h
+++ b/virt/kvm/kvm_mm.h
@@ -68,17 +68,18 @@ static inline void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm,
#endif /* HAVE_KVM_PFNCACHE */
#ifdef CONFIG_KVM_GUEST_MEMFD
-void kvm_gmem_init(struct module *module);
+int kvm_gmem_init(struct module *module);
+void kvm_gmem_exit(void);
int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args);
int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
unsigned int fd, loff_t offset);
void kvm_gmem_unbind(struct kvm_memory_slot *slot);
#else
-static inline void kvm_gmem_init(struct module *module)
+static inline int kvm_gmem_init(struct module *module)
{
-
+ return 0;
}
-
+static inline void kvm_gmem_exit(void) {};
static inline int kvm_gmem_bind(struct kvm *kvm,
struct kvm_memory_slot *slot,
unsigned int fd, loff_t offset)
--
2.50.1.703.g449372360f-goog
^ permalink raw reply related
* Re: [PATCH V9 1/7] KVM: guest_memfd: Use guest mem inodes instead of anonymous inodes
From: Ackerley Tng @ 2025-08-07 21:34 UTC (permalink / raw)
To: David Hildenbrand, Shivank Garg, seanjc, vbabka, willy, akpm,
shuah, pbonzini, brauner, viro
Cc: paul, jmorris, serge, pvorel, bfoster, tabba, vannapurve,
chao.gao, bharata, nikunj, michael.day, shdhiman, yan.y.zhao,
Neeraj.Upadhyay, thomas.lendacky, michael.roth, aik, jgg,
kalyazin, peterx, jack, rppt, hch, cgzones, ira.weiny, rientjes,
roypat, ziy, matthew.brost, joshua.hahnjy, rakie.kim, byungchul,
gourry, kent.overstreet, ying.huang, apopple, chao.p.peng, amit,
ddutile, dan.j.williams, ashish.kalra, gshan, jgowans,
pankaj.gupta, papaluri, yuzhao, suzuki.poulose, quic_eberman,
aneeshkumar.kizhakeveetil, linux-fsdevel, linux-mm, linux-kernel,
linux-security-module, kvm, linux-kselftest, linux-coco
In-Reply-To: <1e37e4e7-aa7b-4a2a-b1aa-1243f8094dcb@redhat.com>
David Hildenbrand <david@redhat.com> writes:
> On 13.07.25 19:43, Shivank Garg wrote:
>> From: Ackerley Tng <ackerleytng@google.com>
>>
>> guest_memfd's inode represents memory the guest_memfd is
>> providing. guest_memfd's file represents a struct kvm's view of that
>> memory.
>>
>> Using a custom inode allows customization of the inode teardown
>> process via callbacks. For example, ->evict_inode() allows
>> customization of the truncation process on file close, and
>> ->destroy_inode() and ->free_inode() allow customization of the inode
>> freeing process.
>>
>> Customizing the truncation process allows flexibility in management of
>> guest_memfd memory and customization of the inode freeing process
>> allows proper cleanup of memory metadata stored on the inode.
>>
>> Memory metadata is more appropriately stored on the inode (as opposed
>> to the file), since the metadata is for the memory and is not unique
>> to a specific binding and struct kvm.
>>
>> Co-developed-by: Fuad Tabba <tabba@google.com>
>> Signed-off-by: Fuad Tabba <tabba@google.com>
>> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>> ---
>
> [...]
>
>>
>> #include "kvm_mm.h"
>>
>> +static struct vfsmount *kvm_gmem_mnt;
>> +
>> struct kvm_gmem {
>> struct kvm *kvm;
>> struct xarray bindings;
>> @@ -388,9 +392,51 @@ static struct file_operations kvm_gmem_fops = {
>> .fallocate = kvm_gmem_fallocate,
>> };
>>
>> -void kvm_gmem_init(struct module *module)
>> +static const struct super_operations kvm_gmem_super_operations = {
>> + .statfs = simple_statfs,
>> +};
>> +
>> +static int kvm_gmem_init_fs_context(struct fs_context *fc)
>> +{
>> + struct pseudo_fs_context *ctx;
>> +
>> + if (!init_pseudo(fc, GUEST_MEMFD_MAGIC))
>> + return -ENOMEM;
>> +
>> + ctx = fc->fs_private;
>> + ctx->ops = &kvm_gmem_super_operations;
>
> Curious, why is that required? (secretmem doesn't have it, so I wonder)
>
Good point! pseudo_fs_fill_super() fills in a struct super_operations
which already does simple_statfs, so guest_memfd doesn't need this.
>> +
>> + return 0;
>> +}
>> +
>> +static struct file_system_type kvm_gmem_fs = {
>> + .name = "kvm_guest_memory",
>
> It's GUEST_MEMFD_MAGIC but here "kvm_guest_memory".
>
> For secretmem it's SECRETMEM_MAGIC vs. "secretmem".
>
> So naturally, I wonder if that is to be made consistent :)
>
I'll update this to "guest_memfd" to be consistent.
>> + .init_fs_context = kvm_gmem_init_fs_context,
>> + .kill_sb = kill_anon_super,
>> +};
>> +
>> +static int kvm_gmem_init_mount(void)
>> +{
>> + kvm_gmem_mnt = kern_mount(&kvm_gmem_fs);
>> +
>> + if (IS_ERR(kvm_gmem_mnt))
>> + return PTR_ERR(kvm_gmem_mnt);
>> +
>> + kvm_gmem_mnt->mnt_flags |= MNT_NOEXEC;
>> + return 0;
>> +}
>> +
>> +int kvm_gmem_init(struct module *module)
>> {
>> kvm_gmem_fops.owner = module;
>> +
>> + return kvm_gmem_init_mount();
>> +}
>> +
>> +void kvm_gmem_exit(void)
>> +{
>> + kern_unmount(kvm_gmem_mnt);
>> + kvm_gmem_mnt = NULL;
>> }
>>
>> static int kvm_gmem_migrate_folio(struct address_space *mapping,
>> @@ -472,11 +518,71 @@ static const struct inode_operations kvm_gmem_iops = {
>> .setattr = kvm_gmem_setattr,
>> };
>>
>> +static struct inode *kvm_gmem_inode_make_secure_inode(const char *name,
>> + loff_t size, u64 flags)
>> +{
>> + struct inode *inode;
>> +
>> + inode = anon_inode_make_secure_inode(kvm_gmem_mnt->mnt_sb, name, NULL);
>> + if (IS_ERR(inode))
>> + return inode;
>> +
>> + inode->i_private = (void *)(unsigned long)flags;
>> + inode->i_op = &kvm_gmem_iops;
>> + inode->i_mapping->a_ops = &kvm_gmem_aops;
>> + inode->i_mode |= S_IFREG;
>> + inode->i_size = size;
>> + mapping_set_gfp_mask(inode->i_mapping, GFP_HIGHUSER);
>> + mapping_set_inaccessible(inode->i_mapping);
>> + /* Unmovable mappings are supposed to be marked unevictable as well. */
>> + WARN_ON_ONCE(!mapping_unevictable(inode->i_mapping));
>> +
>> + return inode;
>> +}
>> +
>> +static struct file *kvm_gmem_inode_create_getfile(void *priv, loff_t size,
>> + u64 flags)
>> +{
>> + static const char *name = "[kvm-gmem]";
>> + struct inode *inode;
>> + struct file *file;
>> + int err;
>> +
>> + err = -ENOENT;
>> + if (!try_module_get(kvm_gmem_fops.owner))
>> + goto err;
>
> Curious, shouldn't there be a module_put() somewhere after this function
> returned a file?
>
This was interesting indeed, but IIUC this is correct.
I think this flow was basically copied from __anon_inode_getfile(),
which does this try_module_get().
The corresponding module_put() is in __fput(), which calls fops_put()
and calls module_put() on the owner.
>> +
>> + inode = kvm_gmem_inode_make_secure_inode(name, size, flags);
>> + if (IS_ERR(inode)) {
>> + err = PTR_ERR(inode);
>> + goto err_put_module;
>> + }
>> +
>> + file = alloc_file_pseudo(inode, kvm_gmem_mnt, name, O_RDWR,
>> + &kvm_gmem_fops);
>> + if (IS_ERR(file)) {
>> + err = PTR_ERR(file);
>> + goto err_put_inode;
>> + }
>> +
>> + file->f_flags |= O_LARGEFILE;
>> + file->private_data = priv;
>> +
>>
>
> Nothing else jumped at me.
>
Thanks for the review!
Since we're going to submit this patch through Shivank's mempolicy
support series, I'll follow up soon by sending a replacement patch in
reply to this series so Shivank could build on top of that?
> --
> Cheers,
>
> David / dhildenb
^ permalink raw reply
* Re: [PATCH v2] lsm,selinux: Add LSM blob support for BPF objects
From: Paul Moore @ 2025-08-07 17:21 UTC (permalink / raw)
To: Blaise Boscaccy
Cc: James Morris, Serge E. Hallyn, Stephen Smalley, Ondrej Mosnacek,
Casey Schaufler, John Johansen, Christian Göttsche, Song Liu,
linux-security-module, linux-kernel, selinux
In-Reply-To: <87pld788yu.fsf@microsoft.com>
On Thu, Aug 7, 2025 at 11:09 AM Blaise Boscaccy
<bboscaccy@linux.microsoft.com> wrote:
> Paul Moore <paul@paul-moore.com> writes:
>
> > On Jul 22, 2025 Blaise Boscaccy <bboscaccy@linux.microsoft.com> wrote:
> >>
> >> This patch introduces LSM blob support for BPF maps, programs, and
> >> tokens to enable LSM stacking and multiplexing of LSM modules that
> >> govern BPF objects. Additionally, the existing BPF hooks used by
> >> SELinux have been updated to utilize the new blob infrastructure,
> >> removing the assumption of exclusive ownership of the security
> >> pointer.
> >>
> >> Signed-off-by: Blaise Boscaccy <bboscaccy@linux.microsoft.com>
> >> ---
> >> v2:
> >> - Use lsm_blob_alloc
> >> - Remove unneded null check
> >> - ifdef guard bpf alloc helpers
> >> ---
> >> include/linux/lsm_hooks.h | 3 ++
> >> security/security.c | 86 +++++++++++++++++++++++++++++--
> >> security/selinux/hooks.c | 56 ++++----------------
> >> security/selinux/include/objsec.h | 17 ++++++
> >> 4 files changed, 113 insertions(+), 49 deletions(-)
> >
> > This looks good to me, one nit/question below ...
> >
> >> @@ -5684,7 +5731,16 @@ int security_bpf_prog(struct bpf_prog *prog)
> >> int security_bpf_map_create(struct bpf_map *map, union bpf_attr *attr,
> >> struct bpf_token *token, bool kernel)
> >> {
> >> - return call_int_hook(bpf_map_create, map, attr, token, kernel);
> >> + int rc = 0;
> >
> > I understand the motivation behind initializing @rc to zero, but to be
> > honest it is redundant and will surely result in a follow on patch from
> > someone to remove the initialization.
> >
> > Do you have any objection to me removing the initialization during the
> > merge? This would obviously apply to the other two as well.
> >
>
> No objections on my end. Thanks.
Okay, merged to lsm/dev-staging with plans to move it to lsm/dev once
the merge window closes.
--
paul-moore.com
^ permalink raw reply
* [RFC PATCH 2/2] memfd: call security_inode_init_security_anon
From: Thiébaud Weksteen @ 2025-08-07 7:57 UTC (permalink / raw)
To: Paul Moore, James Morris, Stephen Smalley, Hugh Dickins,
Jeff Vander Stoep, Nick Kralevich, Jeff Xu
Cc: Thiébaud Weksteen, linux-kernel, linux-security-module,
selinux
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.
[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>
---
mm/memfd.c | 16 ++++++++++++++--
security/selinux/hooks.c | 15 +++++++++++----
security/selinux/include/classmap.h | 2 ++
3 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/mm/memfd.c b/mm/memfd.c
index bbe679895ef6..13bff0e91816 100644
--- a/mm/memfd.c
+++ b/mm/memfd.c
@@ -433,6 +433,9 @@ static struct file *alloc_file(const char *name, unsigned int flags)
{
unsigned int *file_seals;
struct file *file;
+ struct inode *inode;
+ int err = 0;
+ const char *anon_name = "[memfd]";
if (flags & MFD_HUGETLB) {
file = hugetlb_file_setup(name, 0, VM_NORESERVE,
@@ -444,12 +447,21 @@ static struct file *alloc_file(const char *name, unsigned int flags)
}
if (IS_ERR(file))
return file;
+
+ inode = file_inode(file);
+ err = security_inode_init_security_anon(inode,
+ LSM_ANON_INODE_MEMFD,
+ &QSTR(anon_name), NULL);
+ if (err) {
+ fput(file);
+ file = ERR_PTR(err);
+ return file;
+ }
+
file->f_mode |= FMODE_LSEEK | FMODE_PREAD | FMODE_PWRITE;
file->f_flags |= O_LARGEFILE;
if (flags & MFD_NOEXEC_SEAL) {
- struct inode *inode = file_inode(file);
-
inode->i_mode &= ~0111;
file_seals = memfd_file_seals_ptr(file);
if (file_seals) {
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 8d36d5ebb6e5..49742930e706 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2367,8 +2367,8 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm)
ad.u.file = bprm->file;
if (new_tsec->sid == old_tsec->sid) {
- rc = avc_has_perm(old_tsec->sid, isec->sid,
- SECCLASS_FILE, FILE__EXECUTE_NO_TRANS, &ad);
+ rc = avc_has_perm(old_tsec->sid, isec->sid, isec->sclass,
+ FILE__EXECUTE_NO_TRANS, &ad);
if (rc)
return rc;
} else {
@@ -2378,8 +2378,8 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm)
if (rc)
return rc;
- rc = avc_has_perm(new_tsec->sid, isec->sid,
- SECCLASS_FILE, FILE__ENTRYPOINT, &ad);
+ rc = avc_has_perm(new_tsec->sid, isec->sid, isec->sclass,
+ FILE__ENTRYPOINT, &ad);
if (rc)
return rc;
@@ -2997,6 +2997,13 @@ static int selinux_inode_init_security_anon(struct inode *inode,
isec->sclass = context_isec->sclass;
isec->sid = context_isec->sid;
+ } else if (type == LSM_ANON_INODE_MEMFD) {
+ isec->sclass = SECCLASS_MEMFD_FILE;
+ rc = security_transition_sid(
+ sid, sid,
+ isec->sclass, name, &isec->sid);
+ if (rc)
+ return rc;
} else {
isec->sclass = SECCLASS_ANON_INODE;
rc = security_transition_sid(
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index 5665aa5e7853..3ec85142771f 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -179,6 +179,8 @@ const struct security_class_mapping secclass_map[] = {
{ "anon_inode", { COMMON_FILE_PERMS, NULL } },
{ "io_uring", { "override_creds", "sqpoll", "cmd", "allowed", NULL } },
{ "user_namespace", { "create", NULL } },
+ { "memfd_file",
+ { COMMON_FILE_PERMS, "execute_no_trans", "entrypoint", NULL } },
/* last one */ { NULL, {} }
};
--
2.50.1.703.g449372360f-goog
^ permalink raw reply related
* [RFC PATCH 1/2] lsm: add type to security_inode_init_security_anon
From: Thiébaud Weksteen @ 2025-08-07 7:56 UTC (permalink / raw)
To: Paul Moore, James Morris, Stephen Smalley, Hugh Dickins,
Jeff Vander Stoep, Nick Kralevich, Jeff Xu
Cc: Thiébaud Weksteen, linux-kernel, linux-security-module,
selinux
Introduce a new enum (lsm_anon_inode_id) to identify the type of
anonymous inode being created. This enum is passed down to the
security_inode_init_security_anon LSM hook.
Signed-off-by: Thiébaud Weksteen <tweek@google.com>
---
fs/anon_inodes.c | 5 +++--
include/linux/lsm_hook_defs.h | 3 ++-
include/linux/security.h | 8 ++++++++
security/security.c | 4 +++-
security/selinux/hooks.c | 1 +
5 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index 1d847a939f29..9a2f09808f86 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -21,6 +21,7 @@
#include <linux/magic.h>
#include <linux/anon_inodes.h>
#include <linux/pseudo_fs.h>
+#include <linux/security.h>
#include <linux/uaccess.h>
@@ -121,8 +122,8 @@ struct inode *anon_inode_make_secure_inode(struct super_block *sb, const char *n
return inode;
inode->i_flags &= ~S_PRIVATE;
inode->i_op = &anon_inode_operations;
- error = security_inode_init_security_anon(inode, &QSTR(name),
- context_inode);
+ error = security_inode_init_security_anon(inode, LSM_ANON_INODE_GENERIC,
+ &QSTR(name), context_inode);
if (error) {
iput(inode);
return ERR_PTR(error);
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index fd11fffdd3c3..1634f41f7a3c 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -119,7 +119,8 @@ LSM_HOOK(int, -EOPNOTSUPP, inode_init_security, struct inode *inode,
struct inode *dir, const struct qstr *qstr, struct xattr *xattrs,
int *xattr_count)
LSM_HOOK(int, 0, inode_init_security_anon, struct inode *inode,
- const struct qstr *name, const struct inode *context_inode)
+ enum lsm_anon_inode_id type, const struct qstr *name,
+ const struct inode *context_inode)
LSM_HOOK(int, 0, inode_create, struct inode *dir, struct dentry *dentry,
umode_t mode)
LSM_HOOK(void, LSM_RET_VOID, inode_post_create_tmpfile, struct mnt_idmap *idmap,
diff --git a/include/linux/security.h b/include/linux/security.h
index 521bcb5b9717..98a97b8a1093 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -263,6 +263,12 @@ struct request_sock;
#define LSM_UNSAFE_PTRACE 2
#define LSM_UNSAFE_NO_NEW_PRIVS 4
+/* anon_inode types */
+enum lsm_anon_inode_id {
+ LSM_ANON_INODE_GENERIC,
+ LSM_ANON_INODE_MEMFD
+};
+
#ifdef CONFIG_MMU
extern int mmap_min_addr_handler(const struct ctl_table *table, int write,
void *buffer, size_t *lenp, loff_t *ppos);
@@ -402,6 +408,7 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
const struct qstr *qstr,
initxattrs initxattrs, void *fs_data);
int security_inode_init_security_anon(struct inode *inode,
+ enum lsm_anon_inode_id type,
const struct qstr *name,
const struct inode *context_inode);
int security_inode_create(struct inode *dir, struct dentry *dentry, umode_t mode);
@@ -889,6 +896,7 @@ static inline int security_inode_init_security(struct inode *inode,
}
static inline int security_inode_init_security_anon(struct inode *inode,
+ enum lsm_anon_inode_id type,
const struct qstr *name,
const struct inode *context_inode)
{
diff --git a/security/security.c b/security/security.c
index ad163f06bf7a..09aa858819a2 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1861,6 +1861,7 @@ EXPORT_SYMBOL(security_inode_init_security);
/**
* security_inode_init_security_anon() - Initialize an anonymous inode
* @inode: the inode
+ * @type: the type of anonymous inode
* @name: the anonymous inode class
* @context_inode: an optional related inode
*
@@ -1871,10 +1872,11 @@ EXPORT_SYMBOL(security_inode_init_security);
* creation of this inode, or another -errno upon other errors.
*/
int security_inode_init_security_anon(struct inode *inode,
+ enum lsm_anon_inode_id type,
const struct qstr *name,
const struct inode *context_inode)
{
- return call_int_hook(inode_init_security_anon, inode, name,
+ return call_int_hook(inode_init_security_anon, inode, type, name,
context_inode);
}
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index c95a5874bf7d..8d36d5ebb6e5 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2967,6 +2967,7 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
}
static int selinux_inode_init_security_anon(struct inode *inode,
+ enum lsm_anon_inode_id type,
const struct qstr *name,
const struct inode *context_inode)
{
--
2.50.1.703.g449372360f-goog
^ permalink raw reply related
* Re: [PATCH] lsm: use lsm_blob_alloc() in lsm_bdev_alloc()
From: Paul Moore @ 2025-08-07 1:49 UTC (permalink / raw)
To: linux-security-module
In-Reply-To: <20250806212552.240730-2-paul@paul-moore.com>
On Wed, Aug 6, 2025 at 5:26 PM Paul Moore <paul@paul-moore.com> wrote:
>
> Convert the lsm_bdev_alloc() function to use the lsm_blob_alloc() helper
> like all of the other LSM security blob allocators.
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
> security/security.c | 12 ++----------
> 1 file changed, 2 insertions(+), 10 deletions(-)
Normally I might let this sit a bit longer on the mailing list, but
considering how trivial this is along with a Reviewed-by and an ACK
I'm going to go ahead and merge this into lsm/dev-staging for testing
and lsm/dev once the merge window closes.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v2] lsm,selinux: Add LSM blob support for BPF objects
From: Paul Moore @ 2025-08-07 1:44 UTC (permalink / raw)
To: Blaise Boscaccy, James Morris, Serge E. Hallyn, Stephen Smalley,
Ondrej Mosnacek, Casey Schaufler, John Johansen, Blaise Boscaccy,
Christian Göttsche, Song Liu, linux-security-module,
linux-kernel, selinux
In-Reply-To: <20250722212139.1666060-1-bboscaccy@linux.microsoft.com>
On Jul 22, 2025 Blaise Boscaccy <bboscaccy@linux.microsoft.com> wrote:
>
> This patch introduces LSM blob support for BPF maps, programs, and
> tokens to enable LSM stacking and multiplexing of LSM modules that
> govern BPF objects. Additionally, the existing BPF hooks used by
> SELinux have been updated to utilize the new blob infrastructure,
> removing the assumption of exclusive ownership of the security
> pointer.
>
> Signed-off-by: Blaise Boscaccy <bboscaccy@linux.microsoft.com>
> ---
> v2:
> - Use lsm_blob_alloc
> - Remove unneded null check
> - ifdef guard bpf alloc helpers
> ---
> include/linux/lsm_hooks.h | 3 ++
> security/security.c | 86 +++++++++++++++++++++++++++++--
> security/selinux/hooks.c | 56 ++++----------------
> security/selinux/include/objsec.h | 17 ++++++
> 4 files changed, 113 insertions(+), 49 deletions(-)
This looks good to me, one nit/question below ...
> @@ -5684,7 +5731,16 @@ int security_bpf_prog(struct bpf_prog *prog)
> int security_bpf_map_create(struct bpf_map *map, union bpf_attr *attr,
> struct bpf_token *token, bool kernel)
> {
> - return call_int_hook(bpf_map_create, map, attr, token, kernel);
> + int rc = 0;
I understand the motivation behind initializing @rc to zero, but to be
honest it is redundant and will surely result in a follow on patch from
someone to remove the initialization.
Do you have any objection to me removing the initialization during the
merge? This would obviously apply to the other two as well.
> + rc = lsm_bpf_map_alloc(map);
> + if (unlikely(rc))
> + return rc;
> +
> + rc = call_int_hook(bpf_map_create, map, attr, token, kernel);
> + if (unlikely(rc))
> + security_bpf_map_free(map);
> + return rc;
> }
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] lsm: use lsm_blob_alloc() in lsm_bdev_alloc()
From: Serge E. Hallyn @ 2025-08-07 1:13 UTC (permalink / raw)
To: Paul Moore; +Cc: linux-security-module
In-Reply-To: <20250806212552.240730-2-paul@paul-moore.com>
On Wed, Aug 06, 2025 at 05:25:53PM -0400, Paul Moore wrote:
> Convert the lsm_bdev_alloc() function to use the lsm_blob_alloc() helper
> like all of the other LSM security blob allocators.
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
Acked-by: Serge Hallyn <serge@hallyn.com>
> ---
> security/security.c | 12 ++----------
> 1 file changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/security/security.c b/security/security.c
> index ad163f06bf7a..a88ebfca3224 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -823,16 +823,8 @@ static int lsm_msg_msg_alloc(struct msg_msg *mp)
> */
> static int lsm_bdev_alloc(struct block_device *bdev)
> {
> - if (blob_sizes.lbs_bdev == 0) {
> - bdev->bd_security = NULL;
> - return 0;
> - }
> -
> - bdev->bd_security = kzalloc(blob_sizes.lbs_bdev, GFP_KERNEL);
> - if (!bdev->bd_security)
> - return -ENOMEM;
> -
> - return 0;
> + return lsm_blob_alloc(&bdev->bd_security, blob_sizes.lbs_bdev,
> + GFP_KERNEL);
> }
>
> /**
> --
> 2.50.1
>
^ permalink raw reply
* Re: [PATCH] x86/bpf: use bpf_capable() instead of capable(CAP_SYS_ADMIN)
From: Serge E. Hallyn @ 2025-08-07 1:18 UTC (permalink / raw)
To: Ondrej Mosnacek
Cc: Alexei Starovoitov, Daniel Borkmann, bpf, selinux,
linux-security-module
In-Reply-To: <20250806143105.915748-1-omosnace@redhat.com>
On Wed, Aug 06, 2025 at 04:31:05PM +0200, Ondrej Mosnacek wrote:
> Don't check against the overloaded CAP_SYS_ADMINin do_jit(), but instead
> use bpf_capable(), which checks against the more granular CAP_BPF first.
> Going straight to CAP_SYS_ADMIN may cause unnecessary audit log spam
> under SELinux, as privileged domains using BPF would usually only be
> allowed CAP_BPF and not CAP_SYS_ADMIN.
>
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2369326
> Fixes: d4e89d212d40 ("x86/bpf: Call branch history clearing sequence on exit")
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
So this seems correct, *provided* that we consider it within the purview of
CAP_BPF to be able to avoid clearing the branch history buffer.
I suspect that's the case, but it might warrant discussion.
Reviewed-by: Serge Hallyn <serge@hallyn.com>
> ---
> arch/x86/net/bpf_jit_comp.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 15672cb926fc1..2a825e5745ca1 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -2591,8 +2591,7 @@ emit_jmp:
> seen_exit = true;
> /* Update cleanup_addr */
> ctx->cleanup_addr = proglen;
> - if (bpf_prog_was_classic(bpf_prog) &&
> - !capable(CAP_SYS_ADMIN)) {
> + if (bpf_prog_was_classic(bpf_prog) && !bpf_capable()) {
> u8 *ip = image + addrs[i - 1];
>
> if (emit_spectre_bhb_barrier(&prog, ip, bpf_prog))
> --
> 2.50.1
>
^ permalink raw reply
* Re: [PATCH 2/3] LSM: allocate mnt_opts blobs instead of module specific data
From: Paul Moore @ 2025-08-07 0:40 UTC (permalink / raw)
To: Casey Schaufler
Cc: eparis, linux-security-module, jmorris, serge, keescook,
john.johansen, penguin-kernel, stephen.smalley.work, linux-kernel,
selinux
In-Reply-To: <f3a8cbc4-069f-4ec2-8bb5-708b90360b05@schaufler-ca.com>
On Wed, Aug 6, 2025 at 7:16 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 8/6/2025 3:06 PM, Paul Moore wrote:
> > On Jun 17, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> Replace allocations of LSM specific mount data with the
> >> shared mnt_opts blob.
> >>
> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> >> ---
> >> include/linux/lsm_hooks.h | 1 +
> >> security/security.c | 12 ++++++++++++
> >> security/selinux/hooks.c | 10 +++++++---
> >> security/smack/smack_lsm.c | 4 ++--
> >> 4 files changed, 22 insertions(+), 5 deletions(-)
> > ..
> >
> >> diff --git a/security/security.c b/security/security.c
> >> index 8a4e0f70e49d..ec61fb7e6492 100644
> >> --- a/security/security.c
> >> +++ b/security/security.c
> >> @@ -904,6 +904,18 @@ void security_sb_free(struct super_block *sb)
> >> sb->s_security = NULL;
> >> }
> >>
> >> +/**
> >> + * lsm_mnt_opts_alloc - allocate a mnt_opts blob
> >> + * @priority: memory allocation priority
> >> + *
> >> + * Returns a newly allocated mnt_opts blob or NULL if
> >> + * memory isn't available.
> >> + */
> >> +void *lsm_mnt_opts_alloc(gfp_t priority)
> >> +{
> >> + return kzalloc(blob_sizes.lbs_mnt_opts, priority);
> >> +}
> > It's probably better to use lsm_blob_alloc() here so we have some
> > allocator consistency.
> >
> > Also, make this private/static as we should just handle the blob
> > allocation in the LSM framework (see below) just like everything else,
> > unless you can explain why the mount options need to be handled
> > differently?
>
> The mount blob is different from the other blobs in that it is
> only used if there are LSM specific mount options. If there aren't
> LSM specific mount options there is no reason to have a blob.
> I know it's not a huge deal, but there is a performance cost in
> allocating a blob that isn't used.
>
> If you'd really rather accept the overhead, I can make the blob
> always allocated. Let me know.
Well, this is happening at mount time, which should already have a
non-trivial amount of overhead (parsing options, doing the filesystem
setup, mount tree addition, etc.) so I'm not sure this will really be
noticeable in practice. I guess one could also make an argument about
additional memory pressure, but the mount options blob should have a
fairly short lifetime so I don't see that as a significant issue
either. If one, or both, of these becomes an issue we can look into
ways of mitigating them, but right now I'd just assume keep with the
existing LSM blob allocation pattern to simplify the code and make
life easier.
--
paul-moore.com
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox