* Re: [PATCH 3/5] mm: export zap_page_range_single and list_lru_add/del
From: Alice Ryhl @ 2026-02-05 11:58 UTC (permalink / raw)
To: David Hildenbrand (arm)
Cc: Lorenzo Stoakes, Greg Kroah-Hartman, Carlos Llamas,
Alexander Viro, Christian Brauner, Jan Kara, Paul Moore,
James Morris, Serge E. Hallyn, Andrew Morton, Dave Chinner,
Qi Zheng, Roman Gushchin, Muchun Song, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
kernel-team, linux-fsdevel, linux-kernel, linux-security-module,
linux-mm, rust-for-linux
In-Reply-To: <21d90844-1cb1-46ab-a2bb-62f2478b7dfb@kernel.org>
On Thu, Feb 05, 2026 at 12:43:03PM +0100, David Hildenbrand (arm) wrote:
> On 2/5/26 12:29, Lorenzo Stoakes wrote:
> > On Thu, Feb 05, 2026 at 10:51:28AM +0000, Alice Ryhl wrote:
> > > bool list_lru_del_obj(struct list_lru *lru, struct list_head *item)
> > > {
> > > diff --git a/mm/memory.c b/mm/memory.c
> > > index da360a6eb8a48e29293430d0c577fb4b6ec58099..64083ace239a2caf58e1645dd5d91a41d61492c4 100644
> > > --- a/mm/memory.c
> > > +++ b/mm/memory.c
> > > @@ -2168,6 +2168,7 @@ void zap_page_range_single(struct vm_area_struct *vma, unsigned long address,
> > > zap_page_range_single_batched(&tlb, vma, address, size, details);
> > > tlb_finish_mmu(&tlb);
> > > }
> > > +EXPORT_SYMBOL(zap_page_range_single);
> >
> > Sorry but I don't want this exported at all.
> >
> > This is an internal implementation detail which allows fine-grained control of
> > behaviour via struct zap_details (which binder doesn't use, of course :)
>
> I don't expect anybody to set zap_details, but yeah, it could be abused.
> It could be abused right now from anywhere else in the kernel
> where we don't build as a module :)
>
> Apparently we export a similar function in rust where we just removed the last parameter.
To clarify, said Rust function gets inlined into Rust Binder, so Rust
Binder calls the zap_page_range_single() symbol directly.
Alice
^ permalink raw reply
* Re: [PATCH 3/5] mm: export zap_page_range_single and list_lru_add/del
From: David Hildenbrand (arm) @ 2026-02-05 11:57 UTC (permalink / raw)
To: Lorenzo Stoakes, Alice Ryhl
Cc: Greg Kroah-Hartman, Carlos Llamas, Alexander Viro,
Christian Brauner, Jan Kara, Paul Moore, James Morris,
Serge E. Hallyn, Andrew Morton, Dave Chinner, Qi Zheng,
Roman Gushchin, Muchun Song, Liam R. Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Danilo Krummrich, kernel-team,
linux-fsdevel, linux-kernel, linux-security-module, linux-mm,
rust-for-linux
In-Reply-To: <21d90844-1cb1-46ab-a2bb-62f2478b7dfb@kernel.org>
On 2/5/26 12:43, David Hildenbrand (arm) wrote:
> On 2/5/26 12:29, Lorenzo Stoakes wrote:
>> On Thu, Feb 05, 2026 at 10:51:28AM +0000, Alice Ryhl wrote:
>>> These are the functions needed by Binder's shrinker.
>>>
>>> Binder uses zap_page_range_single in the shrinker path to remove an
>>> unused page from the mmap'd region. Note that pages are only removed
>>> from the mmap'd region lazily when shrinker asks for it.
>>>
>>> Binder uses list_lru_add/del to keep track of the shrinker lru list, and
>>> it can't use _obj because the list head is not stored inline in the page
>>> actually being lru freed, so page_to_nid(virt_to_page(item)) on the list
>>> head computes the nid of the wrong page.
>>>
>>> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
>>> ---
>>> mm/list_lru.c | 2 ++
>>> mm/memory.c | 1 +
>>> 2 files changed, 3 insertions(+)
>>>
>>> diff --git a/mm/list_lru.c b/mm/list_lru.c
>>> index
>>> ec48b5dadf519a5296ac14cda035c067f9e448f8..bf95d73c9815548a19db6345f856cee9baad22e3 100644
>>> --- a/mm/list_lru.c
>>> +++ b/mm/list_lru.c
>>> @@ -179,6 +179,7 @@ bool list_lru_add(struct list_lru *lru, struct
>>> list_head *item, int nid,
>>> unlock_list_lru(l, false);
>>> return false;
>>> }
>>> +EXPORT_SYMBOL_GPL(list_lru_add);
>>>
>>> bool list_lru_add_obj(struct list_lru *lru, struct list_head *item)
>>> {
>>> @@ -216,6 +217,7 @@ bool list_lru_del(struct list_lru *lru, struct
>>> list_head *item, int nid,
>>> unlock_list_lru(l, false);
>>> return false;
>>> }
>>> +EXPORT_SYMBOL_GPL(list_lru_del);
>>
>> Same point as before about exporting symbols, but given the _obj
>> variants are
>> exported already this one is more valid.
>>
>>>
>>> bool list_lru_del_obj(struct list_lru *lru, struct list_head *item)
>>> {
>>> diff --git a/mm/memory.c b/mm/memory.c
>>> index
>>> da360a6eb8a48e29293430d0c577fb4b6ec58099..64083ace239a2caf58e1645dd5d91a41d61492c4 100644
>>> --- a/mm/memory.c
>>> +++ b/mm/memory.c
>>> @@ -2168,6 +2168,7 @@ void zap_page_range_single(struct
>>> vm_area_struct *vma, unsigned long address,
>>> zap_page_range_single_batched(&tlb, vma, address, size, details);
>>> tlb_finish_mmu(&tlb);
>>> }
>>> +EXPORT_SYMBOL(zap_page_range_single);
>>
>> Sorry but I don't want this exported at all.
>>
>> This is an internal implementation detail which allows fine-grained
>> control of
>> behaviour via struct zap_details (which binder doesn't use, of course :)
>
> I don't expect anybody to set zap_details, but yeah, it could be abused.
> It could be abused right now from anywhere else in the kernel
> where we don't build as a module :)
>
> Apparently we export a similar function in rust where we just removed
> the last parameter.
>
> I think zap_page_range_single() is only called with non-NULL from mm/
> memory.c.
>
> So the following makes likely sense even outside of the context of this
> series:
The following should compile :)
From b1c35afb1b819a42f4ec1119564b3b37cceb9968 Mon Sep 17 00:00:00 2001
From: "David Hildenbrand (arm)" <david@kernel.org>
Date: Thu, 5 Feb 2026 12:42:09 +0100
Subject: [PATCH] mm/memory: remove "zap_details" parameter from
zap_page_range_single()
Nobody except memory.c should really set that parameter to non-NULL. So
let's just drop it and make unmap_mapping_range_vma() use
zap_page_range_single_batched() instead.
Signed-off-by: David Hildenbrand (arm) <david@kernel.org>
---
arch/s390/mm/gmap_helpers.c | 2 +-
drivers/android/binder_alloc.c | 2 +-
include/linux/mm.h | 5 ++---
kernel/bpf/arena.c | 3 +--
kernel/events/core.c | 2 +-
mm/madvise.c | 3 +--
mm/memory.c | 16 ++++++++++------
net/ipv4/tcp.c | 5 ++---
rust/kernel/mm/virt.rs | 2 +-
9 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/arch/s390/mm/gmap_helpers.c b/arch/s390/mm/gmap_helpers.c
index d41b19925a5a..859f5570c3dc 100644
--- a/arch/s390/mm/gmap_helpers.c
+++ b/arch/s390/mm/gmap_helpers.c
@@ -102,7 +102,7 @@ void gmap_helper_discard(struct mm_struct *mm, unsigned long vmaddr, unsigned lo
if (!vma)
return;
if (!is_vm_hugetlb_page(vma))
- zap_page_range_single(vma, vmaddr, min(end, vma->vm_end) - vmaddr, NULL);
+ zap_page_range_single(vma, vmaddr, min(end, vma->vm_end) - vmaddr);
vmaddr = vma->vm_end;
}
}
diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 979c96b74cad..b0201bc6893a 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -1186,7 +1186,7 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
if (vma) {
trace_binder_unmap_user_start(alloc, index);
- zap_page_range_single(vma, page_addr, PAGE_SIZE, NULL);
+ zap_page_range_single(vma, page_addr, PAGE_SIZE);
trace_binder_unmap_user_end(alloc, index);
}
diff --git a/include/linux/mm.h b/include/linux/mm.h
index f0d5be9dc736..5764991546bb 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2621,11 +2621,10 @@ struct page *vm_normal_page_pud(struct vm_area_struct *vma, unsigned long addr,
void zap_vma_ptes(struct vm_area_struct *vma, unsigned long address,
unsigned long size);
void zap_page_range_single(struct vm_area_struct *vma, unsigned long address,
- unsigned long size, struct zap_details *details);
+ unsigned long size);
static inline void zap_vma_pages(struct vm_area_struct *vma)
{
- zap_page_range_single(vma, vma->vm_start,
- vma->vm_end - vma->vm_start, NULL);
+ zap_page_range_single(vma, vma->vm_start, vma->vm_end - vma->vm_start);
}
void unmap_vmas(struct mmu_gather *tlb, struct ma_state *mas,
struct vm_area_struct *start_vma, unsigned long start,
diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
index 872dc0e41c65..242c931d3740 100644
--- a/kernel/bpf/arena.c
+++ b/kernel/bpf/arena.c
@@ -503,8 +503,7 @@ static void zap_pages(struct bpf_arena *arena, long uaddr, long page_cnt)
struct vma_list *vml;
list_for_each_entry(vml, &arena->vma_list, head)
- zap_page_range_single(vml->vma, uaddr,
- PAGE_SIZE * page_cnt, NULL);
+ zap_page_range_single(vml->vma, uaddr, PAGE_SIZE * page_cnt);
}
static void arena_free_pages(struct bpf_arena *arena, long uaddr, long page_cnt)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 8cca80094624..1dfb33c39c2f 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6926,7 +6926,7 @@ static int map_range(struct perf_buffer *rb, struct vm_area_struct *vma)
#ifdef CONFIG_MMU
/* Clear any partial mappings on error. */
if (err)
- zap_page_range_single(vma, vma->vm_start, nr_pages * PAGE_SIZE, NULL);
+ zap_page_range_single(vma, vma->vm_start, nr_pages * PAGE_SIZE);
#endif
return err;
diff --git a/mm/madvise.c b/mm/madvise.c
index b617b1be0f53..abcbfd1f0662 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -1200,8 +1200,7 @@ static long madvise_guard_install(struct madvise_behavior *madv_behavior)
* OK some of the range have non-guard pages mapped, zap
* them. This leaves existing guard pages in place.
*/
- zap_page_range_single(vma, range->start,
- range->end - range->start, NULL);
+ zap_page_range_single(vma, range->start, range->end - range->start);
}
/*
diff --git a/mm/memory.c b/mm/memory.c
index da360a6eb8a4..82985da5f7e6 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2155,17 +2155,16 @@ void zap_page_range_single_batched(struct mmu_gather *tlb,
* @vma: vm_area_struct holding the applicable pages
* @address: starting address of pages to zap
* @size: number of bytes to zap
- * @details: details of shared cache invalidation
*
* The range must fit into one VMA.
*/
void zap_page_range_single(struct vm_area_struct *vma, unsigned long address,
- unsigned long size, struct zap_details *details)
+ unsigned long size)
{
struct mmu_gather tlb;
tlb_gather_mmu(&tlb, vma->vm_mm);
- zap_page_range_single_batched(&tlb, vma, address, size, details);
+ zap_page_range_single_batched(&tlb, vma, address, size, NULL);
tlb_finish_mmu(&tlb);
}
@@ -2187,7 +2186,7 @@ void zap_vma_ptes(struct vm_area_struct *vma, unsigned long address,
!(vma->vm_flags & VM_PFNMAP))
return;
- zap_page_range_single(vma, address, size, NULL);
+ zap_page_range_single(vma, address, size);
}
EXPORT_SYMBOL_GPL(zap_vma_ptes);
@@ -2963,7 +2962,7 @@ static int remap_pfn_range_notrack(struct vm_area_struct *vma, unsigned long add
* maintain page reference counts, and callers may free
* pages due to the error. So zap it early.
*/
- zap_page_range_single(vma, addr, size, NULL);
+ zap_page_range_single(vma, addr, size);
return error;
}
@@ -4187,7 +4186,12 @@ static void unmap_mapping_range_vma(struct vm_area_struct *vma,
unsigned long start_addr, unsigned long end_addr,
struct zap_details *details)
{
- zap_page_range_single(vma, start_addr, end_addr - start_addr, details);
+ struct mmu_gather tlb;
+
+ tlb_gather_mmu(&tlb, vma->vm_mm);
+ zap_page_range_single_batched(&tlb, vma, start_addr,
+ end_addr - start_addr, details);
+ tlb_finish_mmu(&tlb);
}
static inline void unmap_mapping_range_tree(struct rb_root_cached *root,
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index d5319ebe2452..9e92c71389f3 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2052,7 +2052,7 @@ static int tcp_zerocopy_vm_insert_batch_error(struct vm_area_struct *vma,
maybe_zap_len = total_bytes_to_map - /* All bytes to map */
*length + /* Mapped or pending */
(pages_remaining * PAGE_SIZE); /* Failed map. */
- zap_page_range_single(vma, *address, maybe_zap_len, NULL);
+ zap_page_range_single(vma, *address, maybe_zap_len);
err = 0;
}
@@ -2217,8 +2217,7 @@ static int tcp_zerocopy_receive(struct sock *sk,
total_bytes_to_map = avail_len & ~(PAGE_SIZE - 1);
if (total_bytes_to_map) {
if (!(zc->flags & TCP_RECEIVE_ZEROCOPY_FLAG_TLB_CLEAN_HINT))
- zap_page_range_single(vma, address, total_bytes_to_map,
- NULL);
+ zap_page_range_single(vma, address, total_bytes_to_map);
zc->length = total_bytes_to_map;
zc->recv_skip_hint = 0;
} else {
diff --git a/rust/kernel/mm/virt.rs b/rust/kernel/mm/virt.rs
index da21d65ccd20..b8e59e4420f3 100644
--- a/rust/kernel/mm/virt.rs
+++ b/rust/kernel/mm/virt.rs
@@ -124,7 +124,7 @@ pub fn zap_page_range_single(&self, address: usize, size: usize) {
// sufficient for this method call. This method has no requirements on the vma flags. The
// address range is checked to be within the vma.
unsafe {
- bindings::zap_page_range_single(self.as_ptr(), address, size, core::ptr::null_mut())
+ bindings::zap_page_range_single(self.as_ptr(), address, size)
};
}
--
2.43.0
--
Cheers,
David
^ permalink raw reply related
* Re: [PATCH 1/5] export file_close_fd and task_work_add
From: Lorenzo Stoakes @ 2026-02-05 11:53 UTC (permalink / raw)
To: Alice Ryhl
Cc: Greg Kroah-Hartman, Carlos Llamas, Alexander Viro,
Christian Brauner, Jan Kara, Paul Moore, James Morris,
Serge E. Hallyn, Andrew Morton, Dave Chinner, Qi Zheng,
Roman Gushchin, Muchun Song, David Hildenbrand, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
kernel-team, linux-fsdevel, linux-kernel, linux-security-module,
linux-mm, rust-for-linux
In-Reply-To: <aYSCNur71BJJeB2Q@google.com>
On Thu, Feb 05, 2026 at 11:42:46AM +0000, Alice Ryhl wrote:
> On Thu, Feb 05, 2026 at 11:20:33AM +0000, Lorenzo Stoakes wrote:
> > On Thu, Feb 05, 2026 at 10:51:26AM +0000, Alice Ryhl wrote:
> > > This exports the functionality needed by Binder to close file
> > > descriptors.
> > >
> > > When you send a fd over Binder, what happens is this:
> > >
> > > 1. The sending process turns the fd into a struct file and stores it in
> > > the transaction object.
> > > 2. When the receiving process gets the message, the fd is installed as a
> > > fd into the current process.
> > > 3. When the receiving process is done handling the message, it tells
> > > Binder to clean up the transaction. As part of this, fds embedded in
> > > the transaction are closed.
> > >
> > > Note that it was not always implemented like this. Previously the
> > > sending process would install the fd directly into the receiving proc in
> > > step 1, but as discussed previously [1] this is not ideal and has since
> > > been changed so that fd install happens during receive.
> > >
> > > The functions being exported here are for closing the fd in step 3. They
> > > are required because closing a fd from an ioctl is in general not safe.
> > > This is to meet the requirements for using fdget(), which is used by the
> > > ioctl framework code before calling into the driver's implementation of
> > > the ioctl. Binder works around this with this sequence of operations:
> > >
> > > 1. file_close_fd()
> > > 2. get_file()
> > > 3. filp_close()
> > > 4. task_work_add(current, TWA_RESUME)
> > > 5. <binder returns from ioctl>
> > > 6. fput()
> > >
> > > This ensures that when fput() is called in the task work, the fdget()
> > > that the ioctl framework code uses has already been fdput(), so if the
> > > fd being closed happens to be the same fd, then the fd is not closed
> > > in violation of the fdget() rules.
> >
> > I'm not really familiar with this mechanism but you're already talking about
> > this being a workaround so strikes me the correct thing to do is to find a way
> > to do this in the kernel sensibly rather than exporting internal implementation
> > details and doing it in binder.
>
> I did previously submit a patch that implemented this logic outside of
> Binder, but I was advised to move it into Binder.
Right yeah that's just odd to me, we really do not want to be adding internal
implementation details to drivers.
This is based on bitter experience of bugs being caused by drivers abusing every
interface they get, which is basically exactly what always happens, sadly.
And out-of-tree is heavily discouraged.
Also can we use EXPORT_SYMBOL_FOR_MODULES() for anything we do need to export to
make it explicitly only for binder, perhaps?
>
> But I'm happy to submit a patch to extract this logic into some sort of
> close_fd_safe() method that can be called even if said fd is currently
> held using fdget().
Yup, especially given Christian's view on the kernel task export here I think
that's a more sensible approach.
But obviously I defer the sensible-ness of this to him as I am but an mm dev :)
>
> > > Link: https://lore.kernel.org/all/20180730203633.GC12962@bombadil.infradead.org/ [1]
> > > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > > ---
> > > fs/file.c | 1 +
> > > kernel/task_work.c | 1 +
> > > 2 files changed, 2 insertions(+)
> > >
> > > diff --git a/fs/file.c b/fs/file.c
> > > index 0a4f3bdb2dec6284a0c7b9687213137f2eecb250..0046d0034bf16270cdea7e30a86866ebea3a5a81 100644
> > > --- a/fs/file.c
> > > +++ b/fs/file.c
> > > @@ -881,6 +881,7 @@ struct file *file_close_fd(unsigned int fd)
> > >
> > > return file;
> > > }
> > > +EXPORT_SYMBOL(file_close_fd);
> >
> > As a matter of policy we generally don't like to export without GPL like this
> > unless there's a _really_ good reason.
> >
> > Christian or Al may have a different viewpoint but generally this should be an
> > EXPORT_SYMBOL_GPL() and also - there has to be a _really_ good reason to export
> > it.
>
> Sorry I should just have done _GPL from the beginning. My mistake.
Thanks!
>
> Alice
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH 1/5] export file_close_fd and task_work_add
From: Jan Kara @ 2026-02-05 11:52 UTC (permalink / raw)
To: Christian Brauner
Cc: Alice Ryhl, Greg Kroah-Hartman, Carlos Llamas, Alexander Viro,
Jan Kara, Paul Moore, James Morris, Serge E. Hallyn,
Andrew Morton, Dave Chinner, Qi Zheng, Roman Gushchin,
Muchun Song, David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
kernel-team, linux-fsdevel, linux-kernel, linux-security-module,
linux-mm, rust-for-linux, Christoph Hellwig
In-Reply-To: <20260205-mitschnitt-pfirsich-148a5026fc36@brauner>
On Thu 05-02-26 12:38:22, Christian Brauner wrote:
> On Thu, Feb 05, 2026 at 10:51:26AM +0000, Alice Ryhl wrote:
> > This exports the functionality needed by Binder to close file
> > descriptors.
> >
> > When you send a fd over Binder, what happens is this:
> >
> > 1. The sending process turns the fd into a struct file and stores it in
> > the transaction object.
> > 2. When the receiving process gets the message, the fd is installed as a
> > fd into the current process.
> > 3. When the receiving process is done handling the message, it tells
> > Binder to clean up the transaction. As part of this, fds embedded in
> > the transaction are closed.
> >
> > Note that it was not always implemented like this. Previously the
> > sending process would install the fd directly into the receiving proc in
> > step 1, but as discussed previously [1] this is not ideal and has since
> > been changed so that fd install happens during receive.
> >
> > The functions being exported here are for closing the fd in step 3. They
> > are required because closing a fd from an ioctl is in general not safe.
> > This is to meet the requirements for using fdget(), which is used by the
> > ioctl framework code before calling into the driver's implementation of
> > the ioctl. Binder works around this with this sequence of operations:
> >
> > 1. file_close_fd()
> > 2. get_file()
> > 3. filp_close()
> > 4. task_work_add(current, TWA_RESUME)
> > 5. <binder returns from ioctl>
> > 6. fput()
> >
> > This ensures that when fput() is called in the task work, the fdget()
> > that the ioctl framework code uses has already been fdput(), so if the
> > fd being closed happens to be the same fd, then the fd is not closed
> > in violation of the fdget() rules.
> >
> > Link: https://lore.kernel.org/all/20180730203633.GC12962@bombadil.infradead.org/ [1]
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
> > fs/file.c | 1 +
> > kernel/task_work.c | 1 +
> > 2 files changed, 2 insertions(+)
> >
> > diff --git a/fs/file.c b/fs/file.c
> > index 0a4f3bdb2dec6284a0c7b9687213137f2eecb250..0046d0034bf16270cdea7e30a86866ebea3a5a81 100644
> > --- a/fs/file.c
> > +++ b/fs/file.c
> > @@ -881,6 +881,7 @@ struct file *file_close_fd(unsigned int fd)
> >
> > return file;
> > }
> > +EXPORT_SYMBOL(file_close_fd);
> >
> > void do_close_on_exec(struct files_struct *files)
> > {
> > diff --git a/kernel/task_work.c b/kernel/task_work.c
> > index 0f7519f8e7c93f9a4536c26a341255799c320432..08eb29abaea6b98cc443d1087ddb1d0f1a38c9ae 100644
> > --- a/kernel/task_work.c
> > +++ b/kernel/task_work.c
> > @@ -102,6 +102,7 @@ int task_work_add(struct task_struct *task, struct callback_head *work,
> >
> > return 0;
> > }
> > +EXPORT_SYMBOL(task_work_add);
>
> Uhm, no. We're not going to export task_work_add() to let random drivers
> queue up work for a task when it returns to userspace. That just screams
> bugs and deadlocks at full capacity. Sorry, no.
Agreed. And just to demonstrate the point binder's use would become the
first of such bugs because it is prone to the module being removed while
the task work is in flight and thus do_close_fd() code can be freed by the
time it gets executed.
Generally, making some code modular usually requires more effort than just
flipping the Kconfig to tristate. You usually need to make sure all objects
and queued work is flushed before the module can be removed. Not sure how
much of this is taken care of by Rust though...
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH] man/man7/kernel_lockdown.7: remove Secure Boot untruth
From: Xiu Jianfeng @ 2026-02-05 11:48 UTC (permalink / raw)
To: Alyssa Ross, Alejandro Colomar
Cc: Heinrich Schuchardt, David Howells, Nicolas Bouchinet,
linux-security-module, linux-man
In-Reply-To: <20260203195001.20131-1-hi@alyssa.is>
On 2/4/2026 3:50 AM, Alyssa Ross wrote:
> This is true for Fedora, where this page was sourced from, but I don't
> believe it has ever been true for the mainline kernel, because Linus
> rejected it.
Yeah, I also found this issue not long ago, but I haven't had time to
submit a fix patch yet.
>
> Link: https://bbs.archlinux.org/viewtopic.php?pid=2088704#p2088704
> Link: https://lore.kernel.org/lkml/CA+55aFzYbpRAdma0PvqE+9ygySuKzNKByqOzzMufBoovXVnfPw@mail.gmail.com/
> Fixes: bb509e6fc ("kernel_lockdown.7: New page documenting the Kernel Lockdown feature")
> Signed-off-by: Alyssa Ross <hi@alyssa.is>
I am not sure if appropriate to add my ACK here, if needed, feel free to
add:
Acked-by: Xiu Jianfeng <xiujianfeng@huawei.com>
> ---
> man/man7/kernel_lockdown.7 | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/man/man7/kernel_lockdown.7 b/man/man7/kernel_lockdown.7
> index 5090484ea..5986c8f01 100644
> --- a/man/man7/kernel_lockdown.7
> +++ b/man/man7/kernel_lockdown.7
> @@ -23,9 +23,6 @@ Lockdown: X: Y is restricted, see man kernel_lockdown.7
> .in
> .P
> where X indicates the process name and Y indicates what is restricted.
> -.P
> -On an EFI-enabled x86 or arm64 machine, lockdown will be automatically enabled
> -if the system boots in EFI Secure Boot mode.
> .\"
> .SS Coverage
> When lockdown is in effect, a number of features are disabled or have their
^ permalink raw reply
* Re: [PATCH 3/5] mm: export zap_page_range_single and list_lru_add/del
From: David Hildenbrand (arm) @ 2026-02-05 11:43 UTC (permalink / raw)
To: Lorenzo Stoakes, Alice Ryhl
Cc: Greg Kroah-Hartman, Carlos Llamas, Alexander Viro,
Christian Brauner, Jan Kara, Paul Moore, James Morris,
Serge E. Hallyn, Andrew Morton, Dave Chinner, Qi Zheng,
Roman Gushchin, Muchun Song, Liam R. Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Danilo Krummrich, kernel-team,
linux-fsdevel, linux-kernel, linux-security-module, linux-mm,
rust-for-linux
In-Reply-To: <02801464-f4cb-4e38-8269-f8b9cf0a5965@lucifer.local>
On 2/5/26 12:29, Lorenzo Stoakes wrote:
> On Thu, Feb 05, 2026 at 10:51:28AM +0000, Alice Ryhl wrote:
>> These are the functions needed by Binder's shrinker.
>>
>> Binder uses zap_page_range_single in the shrinker path to remove an
>> unused page from the mmap'd region. Note that pages are only removed
>> from the mmap'd region lazily when shrinker asks for it.
>>
>> Binder uses list_lru_add/del to keep track of the shrinker lru list, and
>> it can't use _obj because the list head is not stored inline in the page
>> actually being lru freed, so page_to_nid(virt_to_page(item)) on the list
>> head computes the nid of the wrong page.
>>
>> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
>> ---
>> mm/list_lru.c | 2 ++
>> mm/memory.c | 1 +
>> 2 files changed, 3 insertions(+)
>>
>> diff --git a/mm/list_lru.c b/mm/list_lru.c
>> index ec48b5dadf519a5296ac14cda035c067f9e448f8..bf95d73c9815548a19db6345f856cee9baad22e3 100644
>> --- a/mm/list_lru.c
>> +++ b/mm/list_lru.c
>> @@ -179,6 +179,7 @@ bool list_lru_add(struct list_lru *lru, struct list_head *item, int nid,
>> unlock_list_lru(l, false);
>> return false;
>> }
>> +EXPORT_SYMBOL_GPL(list_lru_add);
>>
>> bool list_lru_add_obj(struct list_lru *lru, struct list_head *item)
>> {
>> @@ -216,6 +217,7 @@ bool list_lru_del(struct list_lru *lru, struct list_head *item, int nid,
>> unlock_list_lru(l, false);
>> return false;
>> }
>> +EXPORT_SYMBOL_GPL(list_lru_del);
>
> Same point as before about exporting symbols, but given the _obj variants are
> exported already this one is more valid.
>
>>
>> bool list_lru_del_obj(struct list_lru *lru, struct list_head *item)
>> {
>> diff --git a/mm/memory.c b/mm/memory.c
>> index da360a6eb8a48e29293430d0c577fb4b6ec58099..64083ace239a2caf58e1645dd5d91a41d61492c4 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -2168,6 +2168,7 @@ void zap_page_range_single(struct vm_area_struct *vma, unsigned long address,
>> zap_page_range_single_batched(&tlb, vma, address, size, details);
>> tlb_finish_mmu(&tlb);
>> }
>> +EXPORT_SYMBOL(zap_page_range_single);
>
> Sorry but I don't want this exported at all.
>
> This is an internal implementation detail which allows fine-grained control of
> behaviour via struct zap_details (which binder doesn't use, of course :)
I don't expect anybody to set zap_details, but yeah, it could be abused.
It could be abused right now from anywhere else in the kernel
where we don't build as a module :)
Apparently we export a similar function in rust where we just removed the last parameter.
I think zap_page_range_single() is only called with non-NULL from mm/memory.c.
So the following makes likely sense even outside of the context of this series:
From d2a2d20994456b9a66008b7fef12e379e76fc9f8 Mon Sep 17 00:00:00 2001
From: "David Hildenbrand (arm)" <david@kernel.org>
Date: Thu, 5 Feb 2026 12:42:09 +0100
Subject: [PATCH] tmp
Signed-off-by: David Hildenbrand (arm) <david@kernel.org>
---
arch/s390/mm/gmap_helpers.c | 2 +-
drivers/android/binder_alloc.c | 2 +-
include/linux/mm.h | 4 ++--
kernel/bpf/arena.c | 3 +--
kernel/events/core.c | 2 +-
mm/memory.c | 15 +++++++++------
net/ipv4/tcp.c | 5 ++---
rust/kernel/mm/virt.rs | 2 +-
8 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/arch/s390/mm/gmap_helpers.c b/arch/s390/mm/gmap_helpers.c
index d41b19925a5a..859f5570c3dc 100644
--- a/arch/s390/mm/gmap_helpers.c
+++ b/arch/s390/mm/gmap_helpers.c
@@ -102,7 +102,7 @@ void gmap_helper_discard(struct mm_struct *mm, unsigned long vmaddr, unsigned lo
if (!vma)
return;
if (!is_vm_hugetlb_page(vma))
- zap_page_range_single(vma, vmaddr, min(end, vma->vm_end) - vmaddr, NULL);
+ zap_page_range_single(vma, vmaddr, min(end, vma->vm_end) - vmaddr);
vmaddr = vma->vm_end;
}
}
diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 979c96b74cad..b0201bc6893a 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -1186,7 +1186,7 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
if (vma) {
trace_binder_unmap_user_start(alloc, index);
- zap_page_range_single(vma, page_addr, PAGE_SIZE, NULL);
+ zap_page_range_single(vma, page_addr, PAGE_SIZE);
trace_binder_unmap_user_end(alloc, index);
}
diff --git a/include/linux/mm.h b/include/linux/mm.h
index f0d5be9dc736..b7cc6ef49917 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2621,11 +2621,11 @@ struct page *vm_normal_page_pud(struct vm_area_struct *vma, unsigned long addr,
void zap_vma_ptes(struct vm_area_struct *vma, unsigned long address,
unsigned long size);
void zap_page_range_single(struct vm_area_struct *vma, unsigned long address,
- unsigned long size, struct zap_details *details);
+ unsigned long size);
static inline void zap_vma_pages(struct vm_area_struct *vma)
{
zap_page_range_single(vma, vma->vm_start,
- vma->vm_end - vma->vm_start, NULL);
+ vma->vm_end - vma->vm_start);
}
void unmap_vmas(struct mmu_gather *tlb, struct ma_state *mas,
struct vm_area_struct *start_vma, unsigned long start,
diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
index 872dc0e41c65..242c931d3740 100644
--- a/kernel/bpf/arena.c
+++ b/kernel/bpf/arena.c
@@ -503,8 +503,7 @@ static void zap_pages(struct bpf_arena *arena, long uaddr, long page_cnt)
struct vma_list *vml;
list_for_each_entry(vml, &arena->vma_list, head)
- zap_page_range_single(vml->vma, uaddr,
- PAGE_SIZE * page_cnt, NULL);
+ zap_page_range_single(vml->vma, uaddr, PAGE_SIZE * page_cnt);
}
static void arena_free_pages(struct bpf_arena *arena, long uaddr, long page_cnt)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 8cca80094624..1dfb33c39c2f 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6926,7 +6926,7 @@ static int map_range(struct perf_buffer *rb, struct vm_area_struct *vma)
#ifdef CONFIG_MMU
/* Clear any partial mappings on error. */
if (err)
- zap_page_range_single(vma, vma->vm_start, nr_pages * PAGE_SIZE, NULL);
+ zap_page_range_single(vma, vma->vm_start, nr_pages * PAGE_SIZE);
#endif
return err;
diff --git a/mm/memory.c b/mm/memory.c
index da360a6eb8a4..4f8dcdcd20f3 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2155,17 +2155,16 @@ void zap_page_range_single_batched(struct mmu_gather *tlb,
* @vma: vm_area_struct holding the applicable pages
* @address: starting address of pages to zap
* @size: number of bytes to zap
- * @details: details of shared cache invalidation
*
* The range must fit into one VMA.
*/
void zap_page_range_single(struct vm_area_struct *vma, unsigned long address,
- unsigned long size, struct zap_details *details)
+ unsigned long size)
{
struct mmu_gather tlb;
tlb_gather_mmu(&tlb, vma->vm_mm);
- zap_page_range_single_batched(&tlb, vma, address, size, details);
+ zap_page_range_single_batched(&tlb, vma, address, size, NULL);
tlb_finish_mmu(&tlb);
}
@@ -2187,7 +2186,7 @@ void zap_vma_ptes(struct vm_area_struct *vma, unsigned long address,
!(vma->vm_flags & VM_PFNMAP))
return;
- zap_page_range_single(vma, address, size, NULL);
+ zap_page_range_single(vma, address, size);
}
EXPORT_SYMBOL_GPL(zap_vma_ptes);
@@ -2963,7 +2962,7 @@ static int remap_pfn_range_notrack(struct vm_area_struct *vma, unsigned long add
* maintain page reference counts, and callers may free
* pages due to the error. So zap it early.
*/
- zap_page_range_single(vma, addr, size, NULL);
+ zap_page_range_single(vma, addr, size);
return error;
}
@@ -4187,7 +4186,11 @@ static void unmap_mapping_range_vma(struct vm_area_struct *vma,
unsigned long start_addr, unsigned long end_addr,
struct zap_details *details)
{
- zap_page_range_single(vma, start_addr, end_addr - start_addr, details);
+ struct mmu_gather tlb;
+
+ tlb_gather_mmu(&tlb, vma->vm_mm);
+ zap_page_range_single_batched(&tlb, vma, address, size, details);
+ tlb_finish_mmu(&tlb);
}
static inline void unmap_mapping_range_tree(struct rb_root_cached *root,
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index d5319ebe2452..9e92c71389f3 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2052,7 +2052,7 @@ static int tcp_zerocopy_vm_insert_batch_error(struct vm_area_struct *vma,
maybe_zap_len = total_bytes_to_map - /* All bytes to map */
*length + /* Mapped or pending */
(pages_remaining * PAGE_SIZE); /* Failed map. */
- zap_page_range_single(vma, *address, maybe_zap_len, NULL);
+ zap_page_range_single(vma, *address, maybe_zap_len);
err = 0;
}
@@ -2217,8 +2217,7 @@ static int tcp_zerocopy_receive(struct sock *sk,
total_bytes_to_map = avail_len & ~(PAGE_SIZE - 1);
if (total_bytes_to_map) {
if (!(zc->flags & TCP_RECEIVE_ZEROCOPY_FLAG_TLB_CLEAN_HINT))
- zap_page_range_single(vma, address, total_bytes_to_map,
- NULL);
+ zap_page_range_single(vma, address, total_bytes_to_map);
zc->length = total_bytes_to_map;
zc->recv_skip_hint = 0;
} else {
diff --git a/rust/kernel/mm/virt.rs b/rust/kernel/mm/virt.rs
index da21d65ccd20..b8e59e4420f3 100644
--- a/rust/kernel/mm/virt.rs
+++ b/rust/kernel/mm/virt.rs
@@ -124,7 +124,7 @@ pub fn zap_page_range_single(&self, address: usize, size: usize) {
// sufficient for this method call. This method has no requirements on the vma flags. The
// address range is checked to be within the vma.
unsafe {
- bindings::zap_page_range_single(self.as_ptr(), address, size, core::ptr::null_mut())
+ bindings::zap_page_range_single(self.as_ptr(), address, size)
};
}
--
2.43.0
--
Cheers,
David
^ permalink raw reply related
* Re: [PATCH 1/5] export file_close_fd and task_work_add
From: Alice Ryhl @ 2026-02-05 11:42 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Greg Kroah-Hartman, Carlos Llamas, Alexander Viro,
Christian Brauner, Jan Kara, Paul Moore, James Morris,
Serge E. Hallyn, Andrew Morton, Dave Chinner, Qi Zheng,
Roman Gushchin, Muchun Song, David Hildenbrand, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
kernel-team, linux-fsdevel, linux-kernel, linux-security-module,
linux-mm, rust-for-linux
In-Reply-To: <9d0d6edd-eab4-4f31-9691-78ed48e7ad5b@lucifer.local>
On Thu, Feb 05, 2026 at 11:20:33AM +0000, Lorenzo Stoakes wrote:
> On Thu, Feb 05, 2026 at 10:51:26AM +0000, Alice Ryhl wrote:
> > This exports the functionality needed by Binder to close file
> > descriptors.
> >
> > When you send a fd over Binder, what happens is this:
> >
> > 1. The sending process turns the fd into a struct file and stores it in
> > the transaction object.
> > 2. When the receiving process gets the message, the fd is installed as a
> > fd into the current process.
> > 3. When the receiving process is done handling the message, it tells
> > Binder to clean up the transaction. As part of this, fds embedded in
> > the transaction are closed.
> >
> > Note that it was not always implemented like this. Previously the
> > sending process would install the fd directly into the receiving proc in
> > step 1, but as discussed previously [1] this is not ideal and has since
> > been changed so that fd install happens during receive.
> >
> > The functions being exported here are for closing the fd in step 3. They
> > are required because closing a fd from an ioctl is in general not safe.
> > This is to meet the requirements for using fdget(), which is used by the
> > ioctl framework code before calling into the driver's implementation of
> > the ioctl. Binder works around this with this sequence of operations:
> >
> > 1. file_close_fd()
> > 2. get_file()
> > 3. filp_close()
> > 4. task_work_add(current, TWA_RESUME)
> > 5. <binder returns from ioctl>
> > 6. fput()
> >
> > This ensures that when fput() is called in the task work, the fdget()
> > that the ioctl framework code uses has already been fdput(), so if the
> > fd being closed happens to be the same fd, then the fd is not closed
> > in violation of the fdget() rules.
>
> I'm not really familiar with this mechanism but you're already talking about
> this being a workaround so strikes me the correct thing to do is to find a way
> to do this in the kernel sensibly rather than exporting internal implementation
> details and doing it in binder.
I did previously submit a patch that implemented this logic outside of
Binder, but I was advised to move it into Binder.
But I'm happy to submit a patch to extract this logic into some sort of
close_fd_safe() method that can be called even if said fd is currently
held using fdget().
> > Link: https://lore.kernel.org/all/20180730203633.GC12962@bombadil.infradead.org/ [1]
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
> > fs/file.c | 1 +
> > kernel/task_work.c | 1 +
> > 2 files changed, 2 insertions(+)
> >
> > diff --git a/fs/file.c b/fs/file.c
> > index 0a4f3bdb2dec6284a0c7b9687213137f2eecb250..0046d0034bf16270cdea7e30a86866ebea3a5a81 100644
> > --- a/fs/file.c
> > +++ b/fs/file.c
> > @@ -881,6 +881,7 @@ struct file *file_close_fd(unsigned int fd)
> >
> > return file;
> > }
> > +EXPORT_SYMBOL(file_close_fd);
>
> As a matter of policy we generally don't like to export without GPL like this
> unless there's a _really_ good reason.
>
> Christian or Al may have a different viewpoint but generally this should be an
> EXPORT_SYMBOL_GPL() and also - there has to be a _really_ good reason to export
> it.
Sorry I should just have done _GPL from the beginning. My mistake.
Alice
^ permalink raw reply
* Re: [PATCH 1/5] export file_close_fd and task_work_add
From: Christian Brauner @ 2026-02-05 11:38 UTC (permalink / raw)
To: Alice Ryhl
Cc: Greg Kroah-Hartman, Carlos Llamas, Alexander Viro, Jan Kara,
Paul Moore, James Morris, Serge E. Hallyn, Andrew Morton,
Dave Chinner, Qi Zheng, Roman Gushchin, Muchun Song,
David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
kernel-team, linux-fsdevel, linux-kernel, linux-security-module,
linux-mm, rust-for-linux, Christoph Hellwig
In-Reply-To: <20260205-binder-tristate-v1-1-dfc947c35d35@google.com>
On Thu, Feb 05, 2026 at 10:51:26AM +0000, Alice Ryhl wrote:
> This exports the functionality needed by Binder to close file
> descriptors.
>
> When you send a fd over Binder, what happens is this:
>
> 1. The sending process turns the fd into a struct file and stores it in
> the transaction object.
> 2. When the receiving process gets the message, the fd is installed as a
> fd into the current process.
> 3. When the receiving process is done handling the message, it tells
> Binder to clean up the transaction. As part of this, fds embedded in
> the transaction are closed.
>
> Note that it was not always implemented like this. Previously the
> sending process would install the fd directly into the receiving proc in
> step 1, but as discussed previously [1] this is not ideal and has since
> been changed so that fd install happens during receive.
>
> The functions being exported here are for closing the fd in step 3. They
> are required because closing a fd from an ioctl is in general not safe.
> This is to meet the requirements for using fdget(), which is used by the
> ioctl framework code before calling into the driver's implementation of
> the ioctl. Binder works around this with this sequence of operations:
>
> 1. file_close_fd()
> 2. get_file()
> 3. filp_close()
> 4. task_work_add(current, TWA_RESUME)
> 5. <binder returns from ioctl>
> 6. fput()
>
> This ensures that when fput() is called in the task work, the fdget()
> that the ioctl framework code uses has already been fdput(), so if the
> fd being closed happens to be the same fd, then the fd is not closed
> in violation of the fdget() rules.
>
> Link: https://lore.kernel.org/all/20180730203633.GC12962@bombadil.infradead.org/ [1]
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> fs/file.c | 1 +
> kernel/task_work.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/fs/file.c b/fs/file.c
> index 0a4f3bdb2dec6284a0c7b9687213137f2eecb250..0046d0034bf16270cdea7e30a86866ebea3a5a81 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -881,6 +881,7 @@ struct file *file_close_fd(unsigned int fd)
>
> return file;
> }
> +EXPORT_SYMBOL(file_close_fd);
>
> void do_close_on_exec(struct files_struct *files)
> {
> diff --git a/kernel/task_work.c b/kernel/task_work.c
> index 0f7519f8e7c93f9a4536c26a341255799c320432..08eb29abaea6b98cc443d1087ddb1d0f1a38c9ae 100644
> --- a/kernel/task_work.c
> +++ b/kernel/task_work.c
> @@ -102,6 +102,7 @@ int task_work_add(struct task_struct *task, struct callback_head *work,
>
> return 0;
> }
> +EXPORT_SYMBOL(task_work_add);
Uhm, no. We're not going to export task_work_add() to let random drivers
queue up work for a task when it returns to userspace. That just screams
bugs and deadlocks at full capacity. Sorry, no.
^ permalink raw reply
* Re: [PATCH 3/5] mm: export zap_page_range_single and list_lru_add/del
From: Lorenzo Stoakes @ 2026-02-05 11:29 UTC (permalink / raw)
To: Alice Ryhl
Cc: Greg Kroah-Hartman, Carlos Llamas, Alexander Viro,
Christian Brauner, Jan Kara, Paul Moore, James Morris,
Serge E. Hallyn, Andrew Morton, Dave Chinner, Qi Zheng,
Roman Gushchin, Muchun Song, David Hildenbrand, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
kernel-team, linux-fsdevel, linux-kernel, linux-security-module,
linux-mm, rust-for-linux
In-Reply-To: <20260205-binder-tristate-v1-3-dfc947c35d35@google.com>
On Thu, Feb 05, 2026 at 10:51:28AM +0000, Alice Ryhl wrote:
> These are the functions needed by Binder's shrinker.
>
> Binder uses zap_page_range_single in the shrinker path to remove an
> unused page from the mmap'd region. Note that pages are only removed
> from the mmap'd region lazily when shrinker asks for it.
>
> Binder uses list_lru_add/del to keep track of the shrinker lru list, and
> it can't use _obj because the list head is not stored inline in the page
> actually being lru freed, so page_to_nid(virt_to_page(item)) on the list
> head computes the nid of the wrong page.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> mm/list_lru.c | 2 ++
> mm/memory.c | 1 +
> 2 files changed, 3 insertions(+)
>
> diff --git a/mm/list_lru.c b/mm/list_lru.c
> index ec48b5dadf519a5296ac14cda035c067f9e448f8..bf95d73c9815548a19db6345f856cee9baad22e3 100644
> --- a/mm/list_lru.c
> +++ b/mm/list_lru.c
> @@ -179,6 +179,7 @@ bool list_lru_add(struct list_lru *lru, struct list_head *item, int nid,
> unlock_list_lru(l, false);
> return false;
> }
> +EXPORT_SYMBOL_GPL(list_lru_add);
>
> bool list_lru_add_obj(struct list_lru *lru, struct list_head *item)
> {
> @@ -216,6 +217,7 @@ bool list_lru_del(struct list_lru *lru, struct list_head *item, int nid,
> unlock_list_lru(l, false);
> return false;
> }
> +EXPORT_SYMBOL_GPL(list_lru_del);
Same point as before about exporting symbols, but given the _obj variants are
exported already this one is more valid.
>
> bool list_lru_del_obj(struct list_lru *lru, struct list_head *item)
> {
> diff --git a/mm/memory.c b/mm/memory.c
> index da360a6eb8a48e29293430d0c577fb4b6ec58099..64083ace239a2caf58e1645dd5d91a41d61492c4 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2168,6 +2168,7 @@ void zap_page_range_single(struct vm_area_struct *vma, unsigned long address,
> zap_page_range_single_batched(&tlb, vma, address, size, details);
> tlb_finish_mmu(&tlb);
> }
> +EXPORT_SYMBOL(zap_page_range_single);
Sorry but I don't want this exported at all.
This is an internal implementation detail which allows fine-grained control of
behaviour via struct zap_details (which binder doesn't use, of course :)
We either need a wrapper that eliminates this parameter (but then we're adding a
wrapper to this behaviour that is literally for one driver that is _temporarily_
being modularised which is weak justifiction), or use of a function that invokes
it that is currently exported.
Again the general policy with exports is that we really don't want to provide
them at all if we can help it, and if we do, only when it's really justified.
Drivers by nature generally abuse any interfaces provided, we reduce the surface
area of bugs in the kernel by minimising what's available (even via header for
in-tree...)
>
> /**
> * zap_vma_ptes - remove ptes mapping the vma
>
> --
> 2.53.0.rc2.204.g2597b5adb4-goog
>
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH 3/5] mm: export zap_page_range_single and list_lru_add/del
From: David Hildenbrand (arm) @ 2026-02-05 11:30 UTC (permalink / raw)
To: Alice Ryhl
Cc: Greg Kroah-Hartman, Carlos Llamas, Alexander Viro,
Christian Brauner, Jan Kara, Paul Moore, James Morris,
Serge E. Hallyn, Andrew Morton, Dave Chinner, Qi Zheng,
Roman Gushchin, Muchun Song, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
kernel-team, linux-fsdevel, linux-kernel, linux-security-module,
linux-mm, rust-for-linux
In-Reply-To: <aYR8a09XUh74tg8l@google.com>
On 2/5/26 12:18, Alice Ryhl wrote:
> On Thu, Feb 05, 2026 at 12:12:23PM +0100, David Hildenbrand (arm) wrote:
>> On 2/5/26 12:04, Alice Ryhl wrote:
>>>
>>> I just tried to match other symbols in the same file.
>>
>> We were probably a bit too sloppy with some of these in the past. But:
>>
>> davhil01@e142025:~/git/linux$ grep -c "EXPORT_SYMBOL(" mm/memory.c
>> 12
>> davhil01@e142025:~/git/linux$ grep -c "EXPORT_SYMBOL_GPL(" mm/memory.c
>> 10
>>
>> So just go with EXPORT_SYMBOL_GPL unless there is a good reason why not.
>
> Sounds good, I'll do that in the next version.
Feel free to add my
Acked-by: David Hildenbrand (arm) <david@kernel.org>
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH 1/5] export file_close_fd and task_work_add
From: Lorenzo Stoakes @ 2026-02-05 11:20 UTC (permalink / raw)
To: Alice Ryhl
Cc: Greg Kroah-Hartman, Carlos Llamas, Alexander Viro,
Christian Brauner, Jan Kara, Paul Moore, James Morris,
Serge E. Hallyn, Andrew Morton, Dave Chinner, Qi Zheng,
Roman Gushchin, Muchun Song, David Hildenbrand, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
kernel-team, linux-fsdevel, linux-kernel, linux-security-module,
linux-mm, rust-for-linux
In-Reply-To: <20260205-binder-tristate-v1-1-dfc947c35d35@google.com>
On Thu, Feb 05, 2026 at 10:51:26AM +0000, Alice Ryhl wrote:
> This exports the functionality needed by Binder to close file
> descriptors.
>
> When you send a fd over Binder, what happens is this:
>
> 1. The sending process turns the fd into a struct file and stores it in
> the transaction object.
> 2. When the receiving process gets the message, the fd is installed as a
> fd into the current process.
> 3. When the receiving process is done handling the message, it tells
> Binder to clean up the transaction. As part of this, fds embedded in
> the transaction are closed.
>
> Note that it was not always implemented like this. Previously the
> sending process would install the fd directly into the receiving proc in
> step 1, but as discussed previously [1] this is not ideal and has since
> been changed so that fd install happens during receive.
>
> The functions being exported here are for closing the fd in step 3. They
> are required because closing a fd from an ioctl is in general not safe.
> This is to meet the requirements for using fdget(), which is used by the
> ioctl framework code before calling into the driver's implementation of
> the ioctl. Binder works around this with this sequence of operations:
>
> 1. file_close_fd()
> 2. get_file()
> 3. filp_close()
> 4. task_work_add(current, TWA_RESUME)
> 5. <binder returns from ioctl>
> 6. fput()
>
> This ensures that when fput() is called in the task work, the fdget()
> that the ioctl framework code uses has already been fdput(), so if the
> fd being closed happens to be the same fd, then the fd is not closed
> in violation of the fdget() rules.
I'm not really familiar with this mechanism but you're already talking about
this being a workaround so strikes me the correct thing to do is to find a way
to do this in the kernel sensibly rather than exporting internal implementation
details and doing it in binder.
But on this I defer to Christian and Al.
>
> Link: https://lore.kernel.org/all/20180730203633.GC12962@bombadil.infradead.org/ [1]
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> fs/file.c | 1 +
> kernel/task_work.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/fs/file.c b/fs/file.c
> index 0a4f3bdb2dec6284a0c7b9687213137f2eecb250..0046d0034bf16270cdea7e30a86866ebea3a5a81 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -881,6 +881,7 @@ struct file *file_close_fd(unsigned int fd)
>
> return file;
> }
> +EXPORT_SYMBOL(file_close_fd);
As a matter of policy we generally don't like to export without GPL like this
unless there's a _really_ good reason.
Christian or Al may have a different viewpoint but generally this should be an
EXPORT_SYMBOL_GPL() and also - there has to be a _really_ good reason to export
it.
>
> void do_close_on_exec(struct files_struct *files)
> {
> diff --git a/kernel/task_work.c b/kernel/task_work.c
> index 0f7519f8e7c93f9a4536c26a341255799c320432..08eb29abaea6b98cc443d1087ddb1d0f1a38c9ae 100644
> --- a/kernel/task_work.c
> +++ b/kernel/task_work.c
> @@ -102,6 +102,7 @@ int task_work_add(struct task_struct *task, struct callback_head *work,
>
> return 0;
> }
> +EXPORT_SYMBOL(task_work_add);
Same here obviously.
There's nothing else exported here so this is even more questionable.
We want to export as little as possible, and I'm not modularising a driver,
_temporarily_ is a great justification for doing that.
Sadly the moment you export something people start using it :)
>
> /**
> * task_work_cancel_match - cancel a pending work added by task_work_add()
>
> --
> 2.53.0.rc2.204.g2597b5adb4-goog
>
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH 3/5] mm: export zap_page_range_single and list_lru_add/del
From: Alice Ryhl @ 2026-02-05 11:18 UTC (permalink / raw)
To: David Hildenbrand (arm)
Cc: Greg Kroah-Hartman, Carlos Llamas, Alexander Viro,
Christian Brauner, Jan Kara, Paul Moore, James Morris,
Serge E. Hallyn, Andrew Morton, Dave Chinner, Qi Zheng,
Roman Gushchin, Muchun Song, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
kernel-team, linux-fsdevel, linux-kernel, linux-security-module,
linux-mm, rust-for-linux
In-Reply-To: <2f908340-a983-42c7-bb59-2d6f3a80c834@kernel.org>
On Thu, Feb 05, 2026 at 12:12:23PM +0100, David Hildenbrand (arm) wrote:
> On 2/5/26 12:04, Alice Ryhl wrote:
> > On Thu, Feb 05, 2026 at 11:59:47AM +0100, David Hildenbrand (arm) wrote:
> > > On 2/5/26 11:51, Alice Ryhl wrote:
> > > > diff --git a/mm/list_lru.c b/mm/list_lru.c
> > > > index ec48b5dadf519a5296ac14cda035c067f9e448f8..bf95d73c9815548a19db6345f856cee9baad22e3 100644
> > > > --- a/mm/list_lru.c
> > > > +++ b/mm/list_lru.c
> > > > @@ -179,6 +179,7 @@ bool list_lru_add(struct list_lru *lru, struct list_head *item, int nid,
> > > > unlock_list_lru(l, false);
> > > > return false;
> > > > }
> > > > +EXPORT_SYMBOL_GPL(list_lru_add);
> > > > bool list_lru_add_obj(struct list_lru *lru, struct list_head *item)
> > > > {
> > > > @@ -216,6 +217,7 @@ bool list_lru_del(struct list_lru *lru, struct list_head *item, int nid,
> > > > unlock_list_lru(l, false);
> > > > return false;
> > > > }
> > > > +EXPORT_SYMBOL_GPL(list_lru_del);
> > > > bool list_lru_del_obj(struct list_lru *lru, struct list_head *item)
> > > > {
> > > > diff --git a/mm/memory.c b/mm/memory.c
> > > > index da360a6eb8a48e29293430d0c577fb4b6ec58099..64083ace239a2caf58e1645dd5d91a41d61492c4 100644
> > > > --- a/mm/memory.c
> > > > +++ b/mm/memory.c
> > > > @@ -2168,6 +2168,7 @@ void zap_page_range_single(struct vm_area_struct *vma, unsigned long address,
> > > > zap_page_range_single_batched(&tlb, vma, address, size, details);
> > > > tlb_finish_mmu(&tlb);
> > > > }
> > > > +EXPORT_SYMBOL(zap_page_range_single);
> > >
> > > Why not EXPORT_SYMBOL_GPL?
> >
> > I just tried to match other symbols in the same file.
>
> We were probably a bit too sloppy with some of these in the past. But:
>
> davhil01@e142025:~/git/linux$ grep -c "EXPORT_SYMBOL(" mm/memory.c
> 12
> davhil01@e142025:~/git/linux$ grep -c "EXPORT_SYMBOL_GPL(" mm/memory.c
> 10
>
> So just go with EXPORT_SYMBOL_GPL unless there is a good reason why not.
Sounds good, I'll do that in the next version.
Alice
^ permalink raw reply
* Re: [PATCH 3/5] mm: export zap_page_range_single and list_lru_add/del
From: David Hildenbrand (arm) @ 2026-02-05 11:12 UTC (permalink / raw)
To: Alice Ryhl
Cc: Greg Kroah-Hartman, Carlos Llamas, Alexander Viro,
Christian Brauner, Jan Kara, Paul Moore, James Morris,
Serge E. Hallyn, Andrew Morton, Dave Chinner, Qi Zheng,
Roman Gushchin, Muchun Song, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
kernel-team, linux-fsdevel, linux-kernel, linux-security-module,
linux-mm, rust-for-linux
In-Reply-To: <aYR5J0ip2MdD3nMP@google.com>
On 2/5/26 12:04, Alice Ryhl wrote:
> On Thu, Feb 05, 2026 at 11:59:47AM +0100, David Hildenbrand (arm) wrote:
>> On 2/5/26 11:51, Alice Ryhl wrote:
>>> These are the functions needed by Binder's shrinker.
>>>
>>> Binder uses zap_page_range_single in the shrinker path to remove an
>>> unused page from the mmap'd region. Note that pages are only removed
>>> from the mmap'd region lazily when shrinker asks for it.
>>>
>>> Binder uses list_lru_add/del to keep track of the shrinker lru list, and
>>> it can't use _obj because the list head is not stored inline in the page
>>> actually being lru freed, so page_to_nid(virt_to_page(item)) on the list
>>> head computes the nid of the wrong page.
>>>
>>> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
>>> ---
>>> mm/list_lru.c | 2 ++
>>> mm/memory.c | 1 +
>>> 2 files changed, 3 insertions(+)
>>>
>>> diff --git a/mm/list_lru.c b/mm/list_lru.c
>>> index ec48b5dadf519a5296ac14cda035c067f9e448f8..bf95d73c9815548a19db6345f856cee9baad22e3 100644
>>> --- a/mm/list_lru.c
>>> +++ b/mm/list_lru.c
>>> @@ -179,6 +179,7 @@ bool list_lru_add(struct list_lru *lru, struct list_head *item, int nid,
>>> unlock_list_lru(l, false);
>>> return false;
>>> }
>>> +EXPORT_SYMBOL_GPL(list_lru_add);
>>> bool list_lru_add_obj(struct list_lru *lru, struct list_head *item)
>>> {
>>> @@ -216,6 +217,7 @@ bool list_lru_del(struct list_lru *lru, struct list_head *item, int nid,
>>> unlock_list_lru(l, false);
>>> return false;
>>> }
>>> +EXPORT_SYMBOL_GPL(list_lru_del);
>>> bool list_lru_del_obj(struct list_lru *lru, struct list_head *item)
>>> {
>>> diff --git a/mm/memory.c b/mm/memory.c
>>> index da360a6eb8a48e29293430d0c577fb4b6ec58099..64083ace239a2caf58e1645dd5d91a41d61492c4 100644
>>> --- a/mm/memory.c
>>> +++ b/mm/memory.c
>>> @@ -2168,6 +2168,7 @@ void zap_page_range_single(struct vm_area_struct *vma, unsigned long address,
>>> zap_page_range_single_batched(&tlb, vma, address, size, details);
>>> tlb_finish_mmu(&tlb);
>>> }
>>> +EXPORT_SYMBOL(zap_page_range_single);
>>
>> Why not EXPORT_SYMBOL_GPL?
>
> I just tried to match other symbols in the same file.
We were probably a bit too sloppy with some of these in the past. But:
davhil01@e142025:~/git/linux$ grep -c "EXPORT_SYMBOL(" mm/memory.c
12
davhil01@e142025:~/git/linux$ grep -c "EXPORT_SYMBOL_GPL(" mm/memory.c
10
So just go with EXPORT_SYMBOL_GPL unless there is a good reason why not.
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH 3/5] mm: export zap_page_range_single and list_lru_add/del
From: Alice Ryhl @ 2026-02-05 11:04 UTC (permalink / raw)
To: David Hildenbrand (arm)
Cc: Greg Kroah-Hartman, Carlos Llamas, Alexander Viro,
Christian Brauner, Jan Kara, Paul Moore, James Morris,
Serge E. Hallyn, Andrew Morton, Dave Chinner, Qi Zheng,
Roman Gushchin, Muchun Song, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
kernel-team, linux-fsdevel, linux-kernel, linux-security-module,
linux-mm, rust-for-linux
In-Reply-To: <d3dca137-2e4c-4a1a-bdec-63cc2477bfda@kernel.org>
On Thu, Feb 05, 2026 at 11:59:47AM +0100, David Hildenbrand (arm) wrote:
> On 2/5/26 11:51, Alice Ryhl wrote:
> > These are the functions needed by Binder's shrinker.
> >
> > Binder uses zap_page_range_single in the shrinker path to remove an
> > unused page from the mmap'd region. Note that pages are only removed
> > from the mmap'd region lazily when shrinker asks for it.
> >
> > Binder uses list_lru_add/del to keep track of the shrinker lru list, and
> > it can't use _obj because the list head is not stored inline in the page
> > actually being lru freed, so page_to_nid(virt_to_page(item)) on the list
> > head computes the nid of the wrong page.
> >
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
> > mm/list_lru.c | 2 ++
> > mm/memory.c | 1 +
> > 2 files changed, 3 insertions(+)
> >
> > diff --git a/mm/list_lru.c b/mm/list_lru.c
> > index ec48b5dadf519a5296ac14cda035c067f9e448f8..bf95d73c9815548a19db6345f856cee9baad22e3 100644
> > --- a/mm/list_lru.c
> > +++ b/mm/list_lru.c
> > @@ -179,6 +179,7 @@ bool list_lru_add(struct list_lru *lru, struct list_head *item, int nid,
> > unlock_list_lru(l, false);
> > return false;
> > }
> > +EXPORT_SYMBOL_GPL(list_lru_add);
> > bool list_lru_add_obj(struct list_lru *lru, struct list_head *item)
> > {
> > @@ -216,6 +217,7 @@ bool list_lru_del(struct list_lru *lru, struct list_head *item, int nid,
> > unlock_list_lru(l, false);
> > return false;
> > }
> > +EXPORT_SYMBOL_GPL(list_lru_del);
> > bool list_lru_del_obj(struct list_lru *lru, struct list_head *item)
> > {
> > diff --git a/mm/memory.c b/mm/memory.c
> > index da360a6eb8a48e29293430d0c577fb4b6ec58099..64083ace239a2caf58e1645dd5d91a41d61492c4 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -2168,6 +2168,7 @@ void zap_page_range_single(struct vm_area_struct *vma, unsigned long address,
> > zap_page_range_single_batched(&tlb, vma, address, size, details);
> > tlb_finish_mmu(&tlb);
> > }
> > +EXPORT_SYMBOL(zap_page_range_single);
>
> Why not EXPORT_SYMBOL_GPL?
I just tried to match other symbols in the same file.
Alice
^ permalink raw reply
* Re: [PATCH 3/5] mm: export zap_page_range_single and list_lru_add/del
From: David Hildenbrand (arm) @ 2026-02-05 10:59 UTC (permalink / raw)
To: Alice Ryhl, Greg Kroah-Hartman, Carlos Llamas
Cc: Alexander Viro, Christian Brauner, Jan Kara, Paul Moore,
James Morris, Serge E. Hallyn, Andrew Morton, Dave Chinner,
Qi Zheng, Roman Gushchin, Muchun Song, Lorenzo Stoakes,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, kernel-team, linux-fsdevel,
linux-kernel, linux-security-module, linux-mm, rust-for-linux
In-Reply-To: <20260205-binder-tristate-v1-3-dfc947c35d35@google.com>
On 2/5/26 11:51, Alice Ryhl wrote:
> These are the functions needed by Binder's shrinker.
>
> Binder uses zap_page_range_single in the shrinker path to remove an
> unused page from the mmap'd region. Note that pages are only removed
> from the mmap'd region lazily when shrinker asks for it.
>
> Binder uses list_lru_add/del to keep track of the shrinker lru list, and
> it can't use _obj because the list head is not stored inline in the page
> actually being lru freed, so page_to_nid(virt_to_page(item)) on the list
> head computes the nid of the wrong page.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> mm/list_lru.c | 2 ++
> mm/memory.c | 1 +
> 2 files changed, 3 insertions(+)
>
> diff --git a/mm/list_lru.c b/mm/list_lru.c
> index ec48b5dadf519a5296ac14cda035c067f9e448f8..bf95d73c9815548a19db6345f856cee9baad22e3 100644
> --- a/mm/list_lru.c
> +++ b/mm/list_lru.c
> @@ -179,6 +179,7 @@ bool list_lru_add(struct list_lru *lru, struct list_head *item, int nid,
> unlock_list_lru(l, false);
> return false;
> }
> +EXPORT_SYMBOL_GPL(list_lru_add);
>
> bool list_lru_add_obj(struct list_lru *lru, struct list_head *item)
> {
> @@ -216,6 +217,7 @@ bool list_lru_del(struct list_lru *lru, struct list_head *item, int nid,
> unlock_list_lru(l, false);
> return false;
> }
> +EXPORT_SYMBOL_GPL(list_lru_del);
>
> bool list_lru_del_obj(struct list_lru *lru, struct list_head *item)
> {
> diff --git a/mm/memory.c b/mm/memory.c
> index da360a6eb8a48e29293430d0c577fb4b6ec58099..64083ace239a2caf58e1645dd5d91a41d61492c4 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2168,6 +2168,7 @@ void zap_page_range_single(struct vm_area_struct *vma, unsigned long address,
> zap_page_range_single_batched(&tlb, vma, address, size, details);
> tlb_finish_mmu(&tlb);
> }
> +EXPORT_SYMBOL(zap_page_range_single);
Why not EXPORT_SYMBOL_GPL?
--
Cheers,
David
^ permalink raw reply
* [PATCH 5/5] rust_binder: mark ANDROID_BINDER_IPC_RUST tristate
From: Alice Ryhl @ 2026-02-05 10:51 UTC (permalink / raw)
To: Greg Kroah-Hartman, Carlos Llamas
Cc: Alexander Viro, Christian Brauner, Jan Kara, Paul Moore,
James Morris, Serge E. Hallyn, Andrew Morton, Dave Chinner,
Qi Zheng, Roman Gushchin, Muchun Song, David Hildenbrand,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, kernel-team, linux-fsdevel,
linux-kernel, linux-security-module, linux-mm, rust-for-linux,
Alice Ryhl
In-Reply-To: <20260205-binder-tristate-v1-0-dfc947c35d35@google.com>
Currently Binder only builds as built-in module, but in downstream
Android branches we update the build system to make Rust Binder
buildable as a module. The same situation applies to distros, as there
are many distros that enable Binder for support of apps such as
waydroid, which would benefit from the ability to build Binder as a
module.
Note that although the situation in Android may be temporary - once we
no longer have a C implementation, it makes sense for Rust Binder to be
built-in. But that will both take a while, and in any case, distros
enabling Binder will benefit from it being a module even if Android goes
back to built-in.
This doesn't mark C Binder buildable as a module. That would require
more intrusive Makefile changes as it's built from multiple objects, and
I'm not sure there's any way to produce a file called 'binder.ko'
containing all of those objects linked together without renaming
'binder.c', as right now there will be naming conflicts between the
object built from binder.c, and the object that results from linking
binder.o,binderfs.o,binder_alloc.o and so on together. (As an aside,
this issue is why the Rust Binder entry-point is called
rust_binder_main.rs instead of just rust_binder.rs)
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
drivers/android/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig
index e2e402c9d1759c81591473ad02ab7ad011bc61d0..3c1755e53195b0160d0ed244f078eed96e16272c 100644
--- a/drivers/android/Kconfig
+++ b/drivers/android/Kconfig
@@ -15,7 +15,7 @@ config ANDROID_BINDER_IPC
between said processes.
config ANDROID_BINDER_IPC_RUST
- bool "Rust version of Android Binder IPC Driver"
+ tristate "Rust version of Android Binder IPC Driver"
depends on RUST && MMU && !ANDROID_BINDER_IPC
help
This enables the Rust implementation of the Binder driver.
--
2.53.0.rc2.204.g2597b5adb4-goog
^ permalink raw reply related
* [PATCH 4/5] ipc: export init_ipc_ns and put_ipc_ns
From: Alice Ryhl @ 2026-02-05 10:51 UTC (permalink / raw)
To: Greg Kroah-Hartman, Carlos Llamas
Cc: Alexander Viro, Christian Brauner, Jan Kara, Paul Moore,
James Morris, Serge E. Hallyn, Andrew Morton, Dave Chinner,
Qi Zheng, Roman Gushchin, Muchun Song, David Hildenbrand,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, kernel-team, linux-fsdevel,
linux-kernel, linux-security-module, linux-mm, rust-for-linux,
Alice Ryhl
In-Reply-To: <20260205-binder-tristate-v1-0-dfc947c35d35@google.com>
These symbols are used by binderfs as part of the info stored with the
file system.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
ipc/msgutil.c | 1 +
ipc/namespace.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/ipc/msgutil.c b/ipc/msgutil.c
index e28f0cecb2ec942a4f6ee93df8384716bd026011..024faedd07c333b23f1e8733833b84ecf5aed9a7 100644
--- a/ipc/msgutil.c
+++ b/ipc/msgutil.c
@@ -30,6 +30,7 @@ struct ipc_namespace init_ipc_ns = {
.ns = NS_COMMON_INIT(init_ipc_ns),
.user_ns = &init_user_ns,
};
+EXPORT_SYMBOL(init_ipc_ns);
struct msg_msgseg {
struct msg_msgseg *next;
diff --git a/ipc/namespace.c b/ipc/namespace.c
index 535f16ea40e187a9152a03a7345e00b6c5611dbe..c6355020641a74c3be7737b9da15022b961d8f2a 100644
--- a/ipc/namespace.c
+++ b/ipc/namespace.c
@@ -210,6 +210,7 @@ void put_ipc_ns(struct ipc_namespace *ns)
schedule_work(&free_ipc_work);
}
}
+EXPORT_SYMBOL(put_ipc_ns);
static struct ns_common *ipcns_get(struct task_struct *task)
{
--
2.53.0.rc2.204.g2597b5adb4-goog
^ permalink raw reply related
* [PATCH 3/5] mm: export zap_page_range_single and list_lru_add/del
From: Alice Ryhl @ 2026-02-05 10:51 UTC (permalink / raw)
To: Greg Kroah-Hartman, Carlos Llamas
Cc: Alexander Viro, Christian Brauner, Jan Kara, Paul Moore,
James Morris, Serge E. Hallyn, Andrew Morton, Dave Chinner,
Qi Zheng, Roman Gushchin, Muchun Song, David Hildenbrand,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, kernel-team, linux-fsdevel,
linux-kernel, linux-security-module, linux-mm, rust-for-linux,
Alice Ryhl
In-Reply-To: <20260205-binder-tristate-v1-0-dfc947c35d35@google.com>
These are the functions needed by Binder's shrinker.
Binder uses zap_page_range_single in the shrinker path to remove an
unused page from the mmap'd region. Note that pages are only removed
from the mmap'd region lazily when shrinker asks for it.
Binder uses list_lru_add/del to keep track of the shrinker lru list, and
it can't use _obj because the list head is not stored inline in the page
actually being lru freed, so page_to_nid(virt_to_page(item)) on the list
head computes the nid of the wrong page.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
mm/list_lru.c | 2 ++
mm/memory.c | 1 +
2 files changed, 3 insertions(+)
diff --git a/mm/list_lru.c b/mm/list_lru.c
index ec48b5dadf519a5296ac14cda035c067f9e448f8..bf95d73c9815548a19db6345f856cee9baad22e3 100644
--- a/mm/list_lru.c
+++ b/mm/list_lru.c
@@ -179,6 +179,7 @@ bool list_lru_add(struct list_lru *lru, struct list_head *item, int nid,
unlock_list_lru(l, false);
return false;
}
+EXPORT_SYMBOL_GPL(list_lru_add);
bool list_lru_add_obj(struct list_lru *lru, struct list_head *item)
{
@@ -216,6 +217,7 @@ bool list_lru_del(struct list_lru *lru, struct list_head *item, int nid,
unlock_list_lru(l, false);
return false;
}
+EXPORT_SYMBOL_GPL(list_lru_del);
bool list_lru_del_obj(struct list_lru *lru, struct list_head *item)
{
diff --git a/mm/memory.c b/mm/memory.c
index da360a6eb8a48e29293430d0c577fb4b6ec58099..64083ace239a2caf58e1645dd5d91a41d61492c4 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2168,6 +2168,7 @@ void zap_page_range_single(struct vm_area_struct *vma, unsigned long address,
zap_page_range_single_batched(&tlb, vma, address, size, details);
tlb_finish_mmu(&tlb);
}
+EXPORT_SYMBOL(zap_page_range_single);
/**
* zap_vma_ptes - remove ptes mapping the vma
--
2.53.0.rc2.204.g2597b5adb4-goog
^ permalink raw reply related
* [PATCH 2/5] security: export binder symbols
From: Alice Ryhl @ 2026-02-05 10:51 UTC (permalink / raw)
To: Greg Kroah-Hartman, Carlos Llamas
Cc: Alexander Viro, Christian Brauner, Jan Kara, Paul Moore,
James Morris, Serge E. Hallyn, Andrew Morton, Dave Chinner,
Qi Zheng, Roman Gushchin, Muchun Song, David Hildenbrand,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, kernel-team, linux-fsdevel,
linux-kernel, linux-security-module, linux-mm, rust-for-linux,
Alice Ryhl
In-Reply-To: <20260205-binder-tristate-v1-0-dfc947c35d35@google.com>
To enable building Binder as a module, export these symbols.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
security/security.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/security/security.c b/security/security.c
index 31a688650601b62df1e536bbe4407817edbd6707..b4776f0e25b33df95379a08a7659c0e5f767237b 100644
--- a/security/security.c
+++ b/security/security.c
@@ -488,6 +488,7 @@ int security_binder_set_context_mgr(const struct cred *mgr)
{
return call_int_hook(binder_set_context_mgr, mgr);
}
+EXPORT_SYMBOL_GPL(security_binder_set_context_mgr);
/**
* security_binder_transaction() - Check if a binder transaction is allowed
@@ -503,6 +504,7 @@ int security_binder_transaction(const struct cred *from,
{
return call_int_hook(binder_transaction, from, to);
}
+EXPORT_SYMBOL_GPL(security_binder_transaction);
/**
* security_binder_transfer_binder() - Check if a binder transfer is allowed
@@ -518,6 +520,7 @@ int security_binder_transfer_binder(const struct cred *from,
{
return call_int_hook(binder_transfer_binder, from, to);
}
+EXPORT_SYMBOL_GPL(security_binder_transfer_binder);
/**
* security_binder_transfer_file() - Check if a binder file xfer is allowed
@@ -534,6 +537,7 @@ int security_binder_transfer_file(const struct cred *from,
{
return call_int_hook(binder_transfer_file, from, to, file);
}
+EXPORT_SYMBOL_GPL(security_binder_transfer_file);
/**
* security_ptrace_access_check() - Check if tracing is allowed
--
2.53.0.rc2.204.g2597b5adb4-goog
^ permalink raw reply related
* [PATCH 1/5] export file_close_fd and task_work_add
From: Alice Ryhl @ 2026-02-05 10:51 UTC (permalink / raw)
To: Greg Kroah-Hartman, Carlos Llamas
Cc: Alexander Viro, Christian Brauner, Jan Kara, Paul Moore,
James Morris, Serge E. Hallyn, Andrew Morton, Dave Chinner,
Qi Zheng, Roman Gushchin, Muchun Song, David Hildenbrand,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, kernel-team, linux-fsdevel,
linux-kernel, linux-security-module, linux-mm, rust-for-linux,
Alice Ryhl
In-Reply-To: <20260205-binder-tristate-v1-0-dfc947c35d35@google.com>
This exports the functionality needed by Binder to close file
descriptors.
When you send a fd over Binder, what happens is this:
1. The sending process turns the fd into a struct file and stores it in
the transaction object.
2. When the receiving process gets the message, the fd is installed as a
fd into the current process.
3. When the receiving process is done handling the message, it tells
Binder to clean up the transaction. As part of this, fds embedded in
the transaction are closed.
Note that it was not always implemented like this. Previously the
sending process would install the fd directly into the receiving proc in
step 1, but as discussed previously [1] this is not ideal and has since
been changed so that fd install happens during receive.
The functions being exported here are for closing the fd in step 3. They
are required because closing a fd from an ioctl is in general not safe.
This is to meet the requirements for using fdget(), which is used by the
ioctl framework code before calling into the driver's implementation of
the ioctl. Binder works around this with this sequence of operations:
1. file_close_fd()
2. get_file()
3. filp_close()
4. task_work_add(current, TWA_RESUME)
5. <binder returns from ioctl>
6. fput()
This ensures that when fput() is called in the task work, the fdget()
that the ioctl framework code uses has already been fdput(), so if the
fd being closed happens to be the same fd, then the fd is not closed
in violation of the fdget() rules.
Link: https://lore.kernel.org/all/20180730203633.GC12962@bombadil.infradead.org/ [1]
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
fs/file.c | 1 +
kernel/task_work.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/fs/file.c b/fs/file.c
index 0a4f3bdb2dec6284a0c7b9687213137f2eecb250..0046d0034bf16270cdea7e30a86866ebea3a5a81 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -881,6 +881,7 @@ struct file *file_close_fd(unsigned int fd)
return file;
}
+EXPORT_SYMBOL(file_close_fd);
void do_close_on_exec(struct files_struct *files)
{
diff --git a/kernel/task_work.c b/kernel/task_work.c
index 0f7519f8e7c93f9a4536c26a341255799c320432..08eb29abaea6b98cc443d1087ddb1d0f1a38c9ae 100644
--- a/kernel/task_work.c
+++ b/kernel/task_work.c
@@ -102,6 +102,7 @@ int task_work_add(struct task_struct *task, struct callback_head *work,
return 0;
}
+EXPORT_SYMBOL(task_work_add);
/**
* task_work_cancel_match - cancel a pending work added by task_work_add()
--
2.53.0.rc2.204.g2597b5adb4-goog
^ permalink raw reply related
* [PATCH 0/5] Make Rust Binder build as a module
From: Alice Ryhl @ 2026-02-05 10:51 UTC (permalink / raw)
To: Greg Kroah-Hartman, Carlos Llamas
Cc: Alexander Viro, Christian Brauner, Jan Kara, Paul Moore,
James Morris, Serge E. Hallyn, Andrew Morton, Dave Chinner,
Qi Zheng, Roman Gushchin, Muchun Song, David Hildenbrand,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, kernel-team, linux-fsdevel,
linux-kernel, linux-security-module, linux-mm, rust-for-linux,
Alice Ryhl
Currently Binder only builds as built-in module, but in downstream
Android branches we update the build system to make Rust Binder
buildable as a module. The same situation applies to distros, as there
are many distros that enable Binder for support of apps such as
waydroid, which would benefit from the ability to build Binder as a
module.
Note that although the situation in Android may be temporary - once we
no longer have a C implementation, it makes sense for Rust Binder to be
built-in. But that will both take a while, and in any case, distros
enabling Binder will benefit from it being a module even if Android goes
back to built-in.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Alice Ryhl (5):
export file_close_fd and task_work_add
security: export binder symbols
mm: export zap_page_range_single and list_lru_add/del
ipc: export init_ipc_ns and put_ipc_ns
rust_binder: mark ANDROID_BINDER_IPC_RUST tristate
drivers/android/Kconfig | 2 +-
fs/file.c | 1 +
ipc/msgutil.c | 1 +
ipc/namespace.c | 1 +
kernel/task_work.c | 1 +
mm/list_lru.c | 2 ++
mm/memory.c | 1 +
security/security.c | 4 ++++
8 files changed, 12 insertions(+), 1 deletion(-)
---
base-commit: 4df29fb5bcebeea28b29386dec18355949512ca1
change-id: 20260204-binder-tristate-729ac021adca
Best regards,
--
Alice Ryhl <aliceryhl@google.com>
^ permalink raw reply
* Re: [PATCH v3 1/5] lsm: Add hook security_unix_find
From: Mickaël Salaün @ 2026-02-05 10:36 UTC (permalink / raw)
To: Günther Noack
Cc: Günther Noack, Paul Moore, John Johansen, Tingmao Wang,
James Morris, Serge E . Hallyn, Justin Suess,
linux-security-module, Samasth Norway Ananda, Matthieu Buffet,
Mikhail Ivanov, konstantin.meskhidze, Demi Marie Obenour,
Alyssa Ross, Jann Horn, Tahera Fahimi, Simon Horman, netdev,
Alexander Viro, Christian Brauner
In-Reply-To: <aYMenaSmBkAsFowd@google.com>
On Wed, Feb 04, 2026 at 11:25:33AM +0100, Günther Noack wrote:
> Hello!
>
>
> John:
>
> Friendly ping; as Paul said in [1], we would appreciate a look from
> the AppArmor side whether this path-based LSM hook makes sense for
> you.
FYI, we plan to merge this patch series with another one where this new
LSM hook will be used as Günther explained:
>
>
> Everyone:
>
> In [2], we are currently discussing how the UNIX restriction feature
> would work in the bigger scheme in Landlock, and the current plan is
> that long-term we would like to support semantics where a UNIX
> connection attempt is allowed if EITHER:
>
> (a) the path is allow-listed in the policy, OR
> (b) the server side we connect to is part of the same Landlock
> sandbox ("domain")
>
>
> With the currently proposed hook, (a) can be checked in the
> security_unix_find() hook, and (b) can be checked in the
> security_hook_socket_connect() hook. Unfortunately, it also would
> mean that if the (a) check fails, we would have to store that
> information on the side (struct sock LSM blob?), return 0 from (a) and
> then later use that information in hook (b), so that we can check
> whether maybe the second possible condition is met.
>
> Q: The passing of information across multiple LSM hooks is slightly
> more complicated than I had hoped; is this an approach that is
> recommended?
>
> Therefore, in [2], Tingmao is suggesting that we change the
> security_unix_find() hook and pass the "other" struct sock instead of
> the type.
This new approach is much more generic and should please any LSM wishing
to use it.
>
> There is obviously a balance between hooks that are very generic and
> usable across multiple LSMs and hooks that are convenient to use for
> every LSM.
>
> Paul:
>
> You have previously said that you would like hooks to be generic and
> ideally reflect the arguments of the same function that they are
> called from [3].
>
> Q: Would it be acceptable to change the hook arguments, if we can then
> avoid passing additional data between hooks through that side-storage?
> You can see Tingmao's proposal for that in [2]. TL;DR: It moves the
> call to security_unix_find() just after the place where the sk
> variable ("other"-side socket) is looked up and then calls the hook
> with the sk as argument instead of with the type. That way, we can do
> both check (a) and (b) from above in the same hook and do not need to
> store data on the side. Is that an acceptable trade-off for the LSM
> interface?
I think it's a good interface because it let any LSM check both the
resolved path and the resolved socket (without race condition), which
makes sense and align with most other hooks.
>
> Thanks,
> —Günther
>
> [1] https://lore.kernel.org/all/CAHC9VhQZ_J9316Us0squV_f-MjYXPcex34BnJ14vEBxS9Jyjbg@mail.gmail.com/
> [2] https://lore.kernel.org/all/e6b6b069-384c-4c45-a56b-fa54b26bc72a@maowtm.org/
> [3] https://lore.kernel.org/all/CAHC9VhQ234xihpndTs4e5ToNJ3tGCsP7AVtXuz8GajG-_jn3Ow@mail.gmail.com/
>
^ permalink raw reply
* Re: [PATCH v2 0/6] Landlock: Implement scope control for pathname Unix sockets
From: Mickaël Salaün @ 2026-02-05 10:27 UTC (permalink / raw)
To: Günther Noack, Paul Moore, John Johansen
Cc: Günther Noack, Tingmao Wang, Justin Suess,
Demi Marie Obenour, Alyssa Ross, Jann Horn, Tahera Fahimi,
linux-security-module, Matthieu Buffet
In-Reply-To: <20260205.8531e4005118@gnoack.org>
On Thu, Feb 05, 2026 at 09:02:19AM +0100, Günther Noack wrote:
> On Wed, Feb 04, 2026 at 06:43:24PM +0100, Mickaël Salaün wrote:
> > On Wed, Feb 04, 2026 at 12:44:29PM +0100, Günther Noack wrote:
> > > On Tue, Feb 03, 2026 at 09:53:11PM +0000, Tingmao Wang wrote:
> > > > On 2/3/26 17:54, Günther Noack wrote:
> > > > >> Furthermore, an application / Landlock config etc can always opt to not
> > > > >> use the scope bit at all, if it "knows" all the locations where the
> > > > >> application's sockets would be placed, and just use RESOLVE_UNIX access
> > > > >> right (or nothing if it is not supported).
> > > > >>
> > > > >> (The following is a bit of a side note, not terribly relevant if we're
> > > > >> deciding to go with the patch as is.)
> > > > >>
> > > > >>>> [...]
> > > > >>>> Another way to put it is that, if FS-based and scope-based controls
> > > > >>>> interacts in the above proposed way, both mechanisms feel like "poking
> > > > >>>> holes" in the other. But as Mickaël said, one can think of the two
> > > > >>>> mechanisms not as independent controls, but rather as two interfaces for
> > > > >>>> the same control. The socket access control is "enabled" if either the
> > > > >>>> LANDLOCK_ACCESS_FS_RESOLVE_UNIX access is handled, or the scope bit
> > > > >>>> proposed in this patch is enabled.
> > > > >>>>
> > > > >>>> With that said, I can think of some alternative ways that might make this
> > > > >>>> API look "better" (from a subjective point of view, feedback welcome),
> > > > >>>> however it does mean more delays, and specifically, these will depend on
> > > > >>>> LANDLOCK_ACCESS_FS_RESOLVE_UNIX:
> > > > >>>>
> > > > >>>> One possibility is to simply always allow a Landlock domain to connect to
> > > > >>>> its own sockets (in the case where LANDLOCK_ACCESS_FS_RESOLVE_UNIX is
> > > > >>>> handled, otherwise all sockets are allowed). This might be reasonable, as
> > > > >>>> one can only connect to a socket it creates if it has the permission to
> > > > >>>> create it in the first place, which is already controlled by
> > > > >>>> LANDLOCK_ACCESS_FS_MAKE_SOCK, so we don't really lose any policy
> > > > >>>> flexibility here - if for some reason the sandboxer don't want to allow
> > > > >>>> access to any (pathname) sockets, even the sandboxed app's own ones, it
> > > > >>>> can just not allow LANDLOCK_ACCESS_FS_MAKE_SOCK anywhere.
> > > > >>>
> > > > >>> LANDLOCK_ACCESS_FS_MAKE_SOCK is only required to bind/listen to a
> > > > >>> socket, not to connect. I guess you was thinking about
> > > > >>> LANDLOCK_ACCESS_FS_RESOLVE_UNIX in this case?
> > > > >>
> > > > >> In this "allow same-scope connect unconditionally" proposal, the
> > > > >> application would still be able to (bind to and) connect to its own
> > > > >> sockets, even if LANDLOCK_ACCESS_FS_RESOLVE_UNIX is handled and nothing is
> > > > >> allowed to have LANDLOCK_ACCESS_FS_RESOLVE_UNIX access. But a sandboxer
> > > > >> which for whatever reason doesn't want this "allow same scope" default can
> > > > >> still prevent the use of (pathname) sockets by restricting
> > > > >> LANDLOCK_ACCESS_FS_MAKE_SOCK, because if an app can't connect to any
> > > > >> sockets it doesn't own, and can't create any sockets itself either, then
> > > > >> it effectively can't connect to any sockets at all.
> > > > >>
> > > > >> (Although on second thought, I guess there could be a case where an app
> > > > >> first creates some socket files before doing landlock_restrict_self(),
> > > > >> then it might still be able to bind to these even without
> > > > >> LANDLOCK_ACCESS_FS_MAKE_SOCK?)
> > > > >
> > > > > FWIW, I also really liked Tingmao's first of the two listed
> > > > > possibilities in [1], where she proposed to introduce both rights
> > > > > together. In my understanding, the arguments we have discussed so far
> > > > > for that are:
> > > > >
> > > > > IN FAVOR:
> > > > >
> > > > > (pro1) Connecting to a UNIX socket in the same scope is always safe,
> > > > > and it makes it possible to use named UNIX sockets between the
> > > > > processes within a Landlock domains. (Mickaël convinced me in
> > > > > discussion at FOSDEM that this is true.)
> > > > >
> > > > > If someone absolutely does not want that, they can restrict
> > > > > LANDLOCK_ACCESS_FS_MAKE_SOCK and achieve the same effect (as
> > > > > Tingmao said above).
> > > > >
> > > > > (pro2) The implementation of this is simpler.
> > > > >
> > > > > (I attempted to understand how the "or" semantics would be
> > > > > implemented, and I found it non-trivial when you try to do it
> > > > > for all layers at once. (Kernighan's Law applies, IMHO))
> > > >
> > > > I think the logic would basically be:
> > > >
> > > > 1. if any layers deny the access due to handled RESOLVE_UNIX but does not
> > > > have the scope bit set, then we will deny rightaway, without calling
> > > > domain_is_scoped().
> > > >
> > > > 2. Call domain_is_scoped() with a bitmask of "rules_covered" layers where
> > > > there are RESOLVE_UNIX rules covering the socket being accessed, and
> > > > essentially ignore those layers in the scope violation check.
> > > >
> > > > I definitely agree that it is tricky, but making same-scope access be
> > > > allowed (i.e. the suggested idea above) would only get rid of step 1,
> > > > which I think is the "simpler" bit. The extra logic in step 2 is still
> > > > needed.
> > > >
> > > > I definitely agree with pro1 tho.
> > >
> > > Yes, you are describing the logic for the variant where
> > > LANDLOCK_ACCESS_FS_RESOLVE_UNIX does not implicitly permit access from
> > > within the same scope. In that variant, there can be situations where
> > > the first hook can deny the action immediately.
> > >
> > > In the variant where LANDLOCK_ACCESS_FS_RESOLVE_UNIX *does* implicitly
> > > allow access from within the same scope, that shortcutting is not
> > > possible. On the upside however, there is no need to distinguish
> > > whether the scope flag is set when we are in the security_unix_find()
> > > hook, because access from within the same scope is always permitted.
> > > (That is the simplification I meant.)
> >
> > This proposal make sense, improve IPC restriction consistency (by always
> > be able to connect to peers from the same domain), and would simplify
> > some checks.
>
> > What bothers me the most is the implicit scoping induced
> > by the FS_RESOLVE_UNIX handling.
>
> I see this as similar to other decisions we have taken, e.g. on the
> (in-review) socket type patch, we have also discussed at the time that
> socketpair() should always be allowed, even though it creates sockets,
> because this syscall only establishes IPC communication channels
> within the same Landlock domain.
>
> Admittedly, in this case it is a bit more clearly about the Landlock
> domain boundaries, but still OK in my mind. I think we are aligned
> though, see my answer to your proposal below.
>
>
> > A related issue is the mix of checks
> > for the FS_RESOLVE_UNIX implementation, which could make the code more
> > complex, but I don't see a better way. Landlock already has an implicit
> > ptrace scoping but it's not (and should never be) optional.
>
> I am afraid that we don't have a better option. Even if we only
> introduced one of the two patch sets in its original form now, we
> would have to introduce that implementation as soon as the other is
> added.
>
>
> > See my proposal below about implicit
> > LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET. That would not change the kernel
> > checks you described though.
>
>
> > > > > AGAINST:
> > > > >
> > > > > (con1) It would work differently than the other scoped access rights
> > > > > that we already have.
> > > > >
> > > > > A speculative feature that could potentially be built with the
> > > > > scoped access rights is that we could add a rule to permit IPC
> > > > > to other Landlock scopes, e.g. introducing a new rule type
> > > > >
> > > > > struct landlock_scope_attr {
> > > > > __u64 allowed_access; /* for "scoped" bits */
> > > > > /* some way to identify domains */
> > > > > }
> > > > >
> > > > > so that we could make IPC access to other Landlock domains
> > > > > configurable.
> > > > >
> > > > > If the scoped bit and the FS RESOLVE_UNIX bit were both
> > > > > conflated in RESOLVE_UNIX, it would not be possible to make
> > > > > UNIX connections configurable in such a way.
> > > >
> > > > This exact API would no longer work, but if we give up the equivalence
> > > > between scope bits and the landlock_scope_attr struct, then we can do
> > > > something like:
> > > >
> > > > struct landlock_scope_attr {
> > > > __u64 ptrace:1; /* Note that this is not a (user controllable) scope bit! */
> > > > __u64 abstract_unix_socket:1;
> > > > __u64 pathname_unix_socket:1;
> > > > /* ... */
> > > >
> > > > __u64 allowed_signals;
> > > >
> > > > /*
> > > > * some way to identify domains, maybe we could use the audit domain
> > > > * ID, with 0 denoting "allow access to non-Landlocked processes?
> > > > */
> > > > }
> > >
> > > Yes, it would be possible to use such a struct for that scenario where
> > > IPC access gets allowed for other Landlock scopes. It would mean that
> > > we would not need to introduce a scoped flag for the pathname UNIX
> > > socket connections. But the relationship between that struct
> > > landlock_scope_attr and the flags and access rights in struct
> > > landlock_ruleset_attr would become less clear, which is a slight
> > > downside, and maybe error prone for users to work with.
> > >
> > > If we introduced an additional scoped flag, it would also be
> > > consistent though.
> > >
> > > (con1) was written under the assumption that we do not have an
> > > additional scoped flag. If that is lacking, it is not possible to
> > > express UNIX connect() access to other Landlock domains with that
> > > struct. But as outlined in the proposal below, if we *do* (later)
> > > introduce the additional scoped flag *in addition* to the FS access
> > > right, this *both* stays consistent in semantics with the signal and
> > > abstract UNIX support, *and* it starts working in a world where ICP
> > > access can be allowed to talk to other Landlock domains.
> >
> > In this case, we need to assume the scoped flag is already there, and
> > because we have the implementation, to actually add it now.
> >
> > As a side note, I don't really like this landlock_scope_attr interface.
> > A dedicated attr per kind of IPC would be much more flexible (e.g. the
> > handled_signal_number you described below). For instance, even if at
> > first we don't make it possible to describe different signals within the
> > attr struct, this struct will be able to grow. So it's better to have a
> > dedicated interface per IPC type than a generic scope interface that can
> > only express a subset of it.
> >
> > Anyway, this proposal could work for pathname UNIX sockets, but a future
> > signal attr would be a bit inconsistent wrt to the pathname UNIX one.
>
> See my answer to your proposal below.
>
>
> > > > > (con2) Consistent behaviour between scoped flags and their
> > > > > interactions with other access rights:
> > > > >
> > > > > The existing scoped access rights (signal, abstract sockets)
> > > > > could hypothetically be extended with a related access right of
> > > > > another type. For instance, there could be an access right type
> > > > >
> > > > > __u64 handled_signal_number;
> > > > >
> > > > > and then you could add a rule to permit the use of certain
> > > > > signal numbers. The interaction between the scoped flags and
> > > > > other access rights should work the same.
> > > > >
> > > > >
> > > > > Constructive Proposal for consideration: Why not both?
> > > > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > >
> > > > I will think about the following a bit more but I'm afraid that I feel
> > > > like it might get slightly confusing. With this, the only reason for
> > > > having LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET is for API consistency when we
> > > > later enable allowing access to other domains (if I understood correctly),
> > > > in which case I personally feel like the suggestion on landlock_scope_attr
> > > > above, where we essentially accept that it is decoupled with the scope
> > > > bits in the ruleset, might be simpler...?
> > >
> > > Mickaël expressed the opinion to me that he would like to APIs to stay
> > > consistent between signals, abstract UNIX sockets, named UNIX sockets
> > > and other future "scoped" operations, in scenarios where:
> > >
> > > * the "scoped" (IPC) operations can be configured to give access to
> > > other Landlock domains (and that should work for UNIX connections too)
> > > * the existing "scoped" operations also start having matching access rights
> > >
> > > I think with the way I proposed, that would be consistent.
> >
> > What about always implicitly set LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET (in
> > the kernel) when LANDLOCK_ACCESS_FS_RESOLVE_UNIX is set?
> >
> > *Requiring* users to set LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET would add a
> > new way to trigger an error, but setting
> > LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET *implicitly* looks safer. This
> > might also help for the documentation BTW.
> >
> > If we agree on this, I could merge the scope_pathname_unix in 7.0, and
> > merge the resolve_unix in 7.1 (or later if something unexpected happen).
> > Otherwise, I'll merge both at the same time with the same ABI version
> > (with the risk to postpone again). WDYT?
>
> In my understanding, the only thing in which our two proposals differ
> is the order in which we introduce the
> LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET and
> LANDLOCK_ACCESS_FS_RESOLVE_UNIX bits to the UAPI.
>
> As soon as both bits are added, our two proposals do the same thing.
>
> My proposal (introduce LANDLOCK_ACCESS_FS_RESOLVE_UNIX first)
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> The reason why I have been arguing to introduce
> LANDLOCK_ACCESS_FS_RESOLVE_UNIX first is:
>
> (a) It is possible (which initially wasn't obvious to me):
>
> When implementing this, handling LANDLOCK_ACCESS_FS_RESOLVE_UNIX
> implies the LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET semantics(!), even
> when that bit is not exposed in the UAPI yet. (When
> LANDLOCK_ACCESS_FS_RESOLVE_UNIX is handled, UNIX-connect always
> works within the same Landlock domain, even for the time when the
> scoped flag is not exposed in the UAPI yet.)
>
> That way, everything stays compatible even when the scoped flag is
> later introduced.
>
> (b) When introducing LANDLOCK_ACCESS_FS_RESOLVE_UNIX first, the UAPI
> will initially only expose one of the two bits.
>
> This simplifies the UAPI because users initially do not need to
> deal with the fact that two bits in the UAPI "interact".
It simplifies but users can just ignore the bits for which they don't
care. They just have to set the one they want (either scope or resolve)
and Landlock will do the right thing. :)
The only corner case is that setting scope_pathname_unix or not when
resolve_unix is set will result to the same restrictions. In other
words, resolve_unix is a superset of scope_pathname_unix, so this scope
UAPI bit will be useless when resolve_unix will be introduced.
>
> We can still introduce LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET later.
> But the only ruleset-building feature for which this needed would
> be if we want to have rules that allow outwards IPC interactions
> with other Landlock domains.
>
> Possibility of YAGNI: I am not sure how certain you are that the
> speculative allowing-scope-access feature is needed; I mentioned
> it here only because it is an option we want to keep open. If we
> end up *not* needing this feature though, we will *not need to
> introduce the LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET bit at all*,
> because LANDLOCK_ACCESS_FS_RESOLVE_UNIX can already do all that is
> needed.
Indeed, I get your point. Please add this rationale in the "Design
choices" section in Documentation/security/landlock.rst
>
> The implementation of this approach would be that we would have to
> join the functionality from the scoped and FS-based patch set, but
> without introducing the LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET flag in
> the UAPI.
Right, this looks good to me. We'll need to sync both patch series and
remove the scope flag from UAPI. I'll let you and Tingmao work together
for the next series. The "IPC scoping" documentation section should
mention LANDLOCK_ACCESS_FS_RESOLVE_UNIX even if it's not a scope flag.
>
> Your proposal (introduce LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET first)
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> * Upside: users won't be surprised if UNIX-connect within
> the same Landlock domain works, because the bit is there for it
>
> * Downside: They'll have to understand that the FS bit implies the
> scope flag.
Yes, but this would be the same with your approach. Not sure which one
would be the less confusing though.
>
>
>
> But as I said in the beginning -- if we add both flags, these two
> proposals end up being the same in the end.
>
> Maybe the main point that is unclear to me is what I called the
> "Possibility of YAGNI" above: it is at this point not 100% clear to me
> that the feature for allowing IPC to other domains is going to happen
> (we don't have a bug tracker issue for it either, AFAICT). So in the
> case where this does not actually end up happening, I think that my
> approach has the advantage of introducing one flag less, so the UAPI
> stays simpler.
As I said earlier, I'd definitely prefer to have an IPC-specific attr
instead of a generic scope attr. So yes, this looks good to me.
>
> If you know that that feature will happen though, I think the two
> approaches are equally good in the long run.
>
> But either way, Mickaël, you are the maintainer here :); if you prefer
> to do the LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET patch set in its current
> form first, it's also OK with me. As I said, I think these two
> approaches are the same.
You convinced me. Let's go for the hybrid approach you proposed with
the implicit scope.
>
>
> P.S. Tingmao: One additional complication, in case the
> LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET patch set gets sent first -- your
> current patch set uses a different LSM hook than the final state which
> we are aiming for. We may need to double check that this will stay
> compatible even if we move the scoped-check into the new LSM hook in
> the future. (If we move the check into a different hook, there may be
> some obscure scenarios in which a denied connect()/send() results in a
> different error code.) I may comment on the patch set.
That's also a good point. I'd be curious to know if this changes
anything, but it should not except maybe some error codes.
Anyway, we'll need this new LSM hook.
>
> > > > > Why not do both what Tingmao proposed in [1] **and** reserve the
> > > > > option to add the matching "scoped flag" later?
> > > > >
> > > > > * Introduce LANDLOCK_ACCESS_FS_RESOLVE_UNIX.
> > > > >
> > > > > If it is handled, UNIX connections are allowed either:
> > > > >
> > > > > (1) if the connection is to a service in the same scope, or
> > > > > (2) if the path was allow-listed with a "path beneath" rule.
> > > > >
> > > > > * Add LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET later, if needed.
> > > > >
> > > > >
> > > > > Let's go through the arguments again:
> > > > >
> > > > > We have observed that it is harmless to allow connections to services
> > > > > in the same scope (1), and that if users absolutely don't want that,
> > > > > they can actually prohibit it through LANDLOCK_ACCESS_FS_MAKE_SOCK
> > > > > (pro1).
> > > > >
> > > > > (con1): Can we still implement the feature idea where we poke a hole
> > > > > to get UNIX-connect() access to other Landlock domains?
> > > > >
> > > > > I think the answer is yes. The implementation strategy is:
> > > > >
> > > > > * Add the scoped bit LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET
> > > > > * The scoped bit can now be used to allow-list connections to
> > > > > other Landlock domains.
> > > > >
> > > > > For users, just setting the scoped bit on its own does the same as
> > > > > handling LANDLOCK_ACCESS_FS_RESOLVE_UNIX. That way, the kernel-side
> > > > > implementation can also stay simple. The only reason why the scoped
> > > > > bit is needed is because it makes it possible to allow-list
> > > > > connections to other Landlock domains, but at the same time, it is
> > > > > safe if libraries set the scoped bit once it exists, as it does not
> > > > > have any bad runtime impact either.
> > > > >
> > > > > (con2): Consistency: Do all the scoped flags interact with their
> > > > > corresponding access rights in the same way?
> > > > >
> > > > > The other scope flags do not have corresponding access rights, so
> > > > > far.
> > > > >
> > > > > If we were to add corresponding access rights for the other scope
> > > > > flags, I would argue that we could apply a consistent logic there,
> > > > > because IPC access within the same scope is always safe:
> > > > >
> > > > > - A hypothetical access right type for "signal numbers" would only
> > > > > restrict signals that go beyond the current scope.
> > > > >
> > > > > - A hypothetical access right type for "abstract UNIX domain socket
> > > > > names" would only restrict connections to abstract UNIX domain
> > > > > servers that go beyond the current scope.
> > > > >
> > > > > I can not come up with a scenario where this doesn't work.
> > > > >
> > > > >
> > > > > In conclusion, I think the approach has significant upsides:
> > > > >
> > > > > * Simpler UAPI: Users only have one access bit to deal with, in the
> > > > > near future. Once we do add a scope flag for UNIX connections, it
> > > > > does not interact in a surprising way with the corresponding FS
> > > > > access right, because with either of these, scoped access is
> > > > > allowed.
> > > > >
> > > > > If users absolutely need to restrict scoped access, they can
> > > > > restrict LANDLOCK_ACCESS_FS_MAKE_SOCK. It is a slightly obscure
> > > > > API, but in line with the "make easy things easy, make hard things
> > > > > possible" API philosophy. And needing this should be the
> > > > > exception rather than the norm, after all.
> > > > >
> > > > > * Consistent behaviour between scoped flags and regular access
> > > > > rights, also for speculative access rights affecting the existing
> > > > > scoped flags for signals and abstract UNIX domain sockets.
> > > > >
> > > > > [1] https://lore.kernel.org/all/f07fe41a-96c5-4d3a-9966-35b30b3a71f1@maowtm.org/
> >
> > Thanks for this summary Günther!
>
^ permalink raw reply
* Re: [PATCH v14 2/9] rust: rename `AlwaysRefCounted` to `RefCounted`.
From: Viresh Kumar @ 2026-02-05 9:54 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Dave Ertman, Ira Weiny, Leon Romanovsky,
Paul Moore, Serge Hallyn, Rafael J. Wysocki, David Airlie,
Simona Vetter, Alexander Viro, Christian Brauner, Jan Kara,
Igor Korotin, Daniel Almeida, Lorenzo Stoakes, Liam R. Howlett,
Viresh Kumar, Nishanth Menon, Stephen Boyd, Bjorn Helgaas,
Krzysztof Wilczyński, linux-kernel, rust-for-linux,
linux-block, linux-security-module, dri-devel, linux-fsdevel,
linux-mm, linux-pm, linux-pci, Oliver Mangold
In-Reply-To: <20260204-unique-ref-v14-2-17cb29ebacbb@kernel.org>
On 04-02-26, 12:56, Andreas Hindborg wrote:
> diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
> index a760fac287655..06fe2ca776a4f 100644
> --- a/rust/kernel/opp.rs
> +++ b/rust/kernel/opp.rs
> @@ -16,8 +16,8 @@
> ffi::{c_char, c_ulong},
> prelude::*,
> str::CString,
> - sync::aref::{ARef, AlwaysRefCounted},
> - types::Opaque,
> + sync::aref::RefCounted,
> + types::{ARef, AlwaysRefCounted, Opaque},
> };
>
> #[cfg(CONFIG_CPU_FREQ)]
> @@ -1041,7 +1041,7 @@ unsafe impl Send for OPP {}
> unsafe impl Sync for OPP {}
>
> /// SAFETY: The type invariants guarantee that [`OPP`] is always refcounted.
> -unsafe impl AlwaysRefCounted for OPP {
> +unsafe impl RefCounted for OPP {
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference means that the refcount is nonzero.
> unsafe { bindings::dev_pm_opp_get(self.0.get()) };
> @@ -1053,6 +1053,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<OPP>` from an
> +// `&OPP`.
> +unsafe impl AlwaysRefCounted for OPP {}
> +
> impl OPP {
> /// Creates an owned reference to a [`OPP`] from a valid pointer.
> ///
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply
* Re: [PATCH 12/13] ovl: remove ovl_lock_rename_workdir()
From: Amir Goldstein @ 2026-02-05 9:45 UTC (permalink / raw)
To: NeilBrown
Cc: Christian Brauner, Alexander Viro, David Howells, Jan Kara,
Chuck Lever, Jeff Layton, Miklos Szeredi, John Johansen,
Paul Moore, James Morris, Serge E. Hallyn, Stephen Smalley,
linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260204050726.177283-13-neilb@ownmail.net>
On Wed, Feb 4, 2026 at 6:09 AM NeilBrown <neilb@ownmail.net> wrote:
>
> From: NeilBrown <neil@brown.name>
>
> This function is unused.
>
I am confused.
What was this "fix" fixing an unused function:
e9c70084a64e5 ovl: fail ovl_lock_rename_workdir() if either target is unhashed
What am I missing?
Otherwise, feel free to add:
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Thanks,
Amir.
^ 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