rust_binder: use KVVec for files_to_translate

scadastrangelove posted 1 patch 1 month ago
There is a newer version of this series
drivers/android/binder/allocation.rs | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
rust_binder: use KVVec for files_to_translate
Posted by scadastrangelove 1 month ago
From: Sergey Gordeychik <scadastrangelove@gmail.com>

The num_fds value in a binder_fd_array_object is bounded by the
transaction buffer. However, its in-kernel metadata is larger than the
u32 array on the wire.

On 64-bit systems, FileEntry occupies 24 bytes. About 900,000 entries
therefore make files_to_translate request roughly 20.6 MiB of
physically contiguous memory, triggering a warning in
__alloc_frozen_pages_noprof.

translate_fds() later allocates Reservation entries from the same
count. At 16 bytes per entry, this requires another 13.7 MiB contiguous
allocation.

Neither vector requires physical contiguity. Use KVVec for both so
large allocations can fall back to vmalloc.

Keep close_on_free as KVec because its u32 storage matches the wire
representation and does not reach the allocation sizes above.

Tested under QEMU/KVM. The 900,000-entry reproducer no longer triggers
a page allocator warning, and a 300,000-entry transaction that repeats
one valid fd reaches translate_fds() without WARN or BUG.

Suggested-by: rust-in-peace agentic pipeline
Signed-off-by: Sergey Gordeychik <scadastrangelove@gmail.com>
---
 drivers/android/binder/allocation.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/android/binder/allocation.rs b/drivers/android/binder/allocation.rs
index ea5846e4d..6a95298f2 100644
--- a/drivers/android/binder/allocation.rs
+++ b/drivers/android/binder/allocation.rs
@@ -208,7 +208,7 @@ pub(crate) fn translate_fds(&mut self) -> Result<TranslatedFds> {
         let num_close_on_free = files.iter().filter(|entry| entry.close_on_free).count();
         let mut close_on_free = KVec::with_capacity(num_close_on_free, GFP_KERNEL)?;

-        let mut reservations = KVec::with_capacity(files.len(), GFP_KERNEL)?;
+        let mut reservations = KVVec::with_capacity(files.len(), GFP_KERNEL)?;
         for file_info in files {
             let res = FileDescriptorReservation::get_unused_fd_flags(bindings::O_CLOEXEC)?;
             let fd = res.reserved_fd();
@@ -567,7 +567,7 @@ fn type_to_size(type_: u32) -> Option<usize> {

 #[derive(Default)]
 struct FileList {
-    files_to_translate: KVec<FileEntry>,
+    files_to_translate: KVVec<FileEntry>,
     close_on_free: KVec<u32>,
 }

@@ -581,7 +581,7 @@ struct FileEntry {
 }

 pub(crate) struct TranslatedFds {
-    reservations: KVec<Reservation>,
+    reservations: KVVec<Reservation>,
     /// If commit is called, then these fds should be closed. (If commit is not called, then they
     /// shouldn't be closed.)
     close_on_free: FdsCloseOnFree,
@@ -595,7 +595,7 @@ struct Reservation {
 impl TranslatedFds {
     pub(crate) fn new() -> Self {
         Self {
-            reservations: KVec::new(),
+            reservations: KVVec::new(),
             close_on_free: FdsCloseOnFree(KVec::new()),
         }
     }
--
2.43.0
Re: rust_binder: use KVVec for files_to_translate
Posted by Alice Ryhl 1 month ago
On Tue, Aug 25, 2026 at 07:17:12PM +0300, scadastrangelove wrote:
> From: Sergey Gordeychik <scadastrangelove@gmail.com>
> 
> The num_fds value in a binder_fd_array_object is bounded by the
> transaction buffer. However, its in-kernel metadata is larger than the
> u32 array on the wire.
> 
> On 64-bit systems, FileEntry occupies 24 bytes. About 900,000 entries
> therefore make files_to_translate request roughly 20.6 MiB of
> physically contiguous memory, triggering a warning in
> __alloc_frozen_pages_noprof.
> 
> translate_fds() later allocates Reservation entries from the same
> count. At 16 bytes per entry, this requires another 13.7 MiB contiguous
> allocation.
> 
> Neither vector requires physical contiguity. Use KVVec for both so
> large allocations can fall back to vmalloc.
> 
> Keep close_on_free as KVec because its u32 storage matches the wire
> representation and does not reach the allocation sizes above.
> 
> Tested under QEMU/KVM. The 900,000-entry reproducer no longer triggers
> a page allocator warning, and a 300,000-entry transaction that repeats
> one valid fd reaches translate_fds() without WARN or BUG.
> 
> Suggested-by: rust-in-peace agentic pipeline
> Signed-off-by: Sergey Gordeychik <scadastrangelove@gmail.com>

A few things. One it looks like you're missing [PATCH] in the email
title. Also, as checkpatch points out, Suggested-by must be an email
address.

Also, should the close_on_free vector also be a KVVec?

Otherwise this looks good to me.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Alice
Re: rust_binder: use KVVec for files_to_translate
Posted by SCADA StrangeLove 1 month ago
Thanks for the review!

> One it looks like you're missing [PATCH] in the email title.
> Also, as checkpatch points out, Suggested-by must be an email
> address.

Both fixed in v2 (sending shortly).

> Also, should the close_on_free vector also be a KVVec?

No -- close_on_free is 4 bytes/entry, matching the wire format, so its
worst case is ~1,048,554 entries (~4 MiB, exactly SZ_4M - 88 bytes).
That fits under KMALLOC_MAX_SIZE on every config rust_binder ships on
today, but only because SZ_4M and KMALLOC_MAX_SIZE happen to coincide,
not because anything enforces it -- if either constant changes, this
stops being true. files_to_translate and reservations don't have that
margin at all: they blow past 4 MiB at far lower fd counts because
their per-entry kernel size is larger than the wire size, which is why
those two need the fix regardless.

Sergey

чт, 27 авг. 2026 г. в 10:07, Alice Ryhl <aliceryhl@google.com>:
>
> On Tue, Aug 25, 2026 at 07:17:12PM +0300, scadastrangelove wrote:
> > From: Sergey Gordeychik <scadastrangelove@gmail.com>
> >
> > The num_fds value in a binder_fd_array_object is bounded by the
> > transaction buffer. However, its in-kernel metadata is larger than the
> > u32 array on the wire.
> >
> > On 64-bit systems, FileEntry occupies 24 bytes. About 900,000 entries
> > therefore make files_to_translate request roughly 20.6 MiB of
> > physically contiguous memory, triggering a warning in
> > __alloc_frozen_pages_noprof.
> >
> > translate_fds() later allocates Reservation entries from the same
> > count. At 16 bytes per entry, this requires another 13.7 MiB contiguous
> > allocation.
> >
> > Neither vector requires physical contiguity. Use KVVec for both so
> > large allocations can fall back to vmalloc.
> >
> > Keep close_on_free as KVec because its u32 storage matches the wire
> > representation and does not reach the allocation sizes above.
> >
> > Tested under QEMU/KVM. The 900,000-entry reproducer no longer triggers
> > a page allocator warning, and a 300,000-entry transaction that repeats
> > one valid fd reaches translate_fds() without WARN or BUG.
> >
> > Suggested-by: rust-in-peace agentic pipeline
> > Signed-off-by: Sergey Gordeychik <scadastrangelove@gmail.com>
>
> A few things. One it looks like you're missing [PATCH] in the email
> title. Also, as checkpatch points out, Suggested-by must be an email
> address.
>
> Also, should the close_on_free vector also be a KVVec?
>
> Otherwise this looks good to me.
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>
> Alice
Re: rust_binder: use KVVec for files_to_translate
Posted by Alice Ryhl 4 weeks, 1 day ago
On Fri, Aug 28, 2026 at 11:06 AM SCADA StrangeLove
<scadastrangelove@gmail.com> wrote:
>
> Thanks for the review!
>
> > One it looks like you're missing [PATCH] in the email title.
> > Also, as checkpatch points out, Suggested-by must be an email
> > address.
>
> Both fixed in v2 (sending shortly).
>
> > Also, should the close_on_free vector also be a KVVec?
>
> No -- close_on_free is 4 bytes/entry, matching the wire format, so its
> worst case is ~1,048,554 entries (~4 MiB, exactly SZ_4M - 88 bytes).
> That fits under KMALLOC_MAX_SIZE on every config rust_binder ships on
> today, but only because SZ_4M and KMALLOC_MAX_SIZE happen to coincide,
> not because anything enforces it -- if either constant changes, this
> stops being true. files_to_translate and reservations don't have that
> margin at all: they blow past 4 MiB at far lower fd counts because
> their per-entry kernel size is larger than the wire size, which is why
> those two need the fix regardless.

Sounds pretty fragile to me. Would be simpler to just use KVVec anyway
in case we add a target where KMALLOC_MAX_SIZE is smaller.

Alice