Rust for Linux List
 help / color / mirror / Atom feed
* [PATCH 0/4] binder: cap max_threads and reject duplicate looper entry
@ 2026-06-03 18:01 Yunseong Kim
  2026-06-03 18:01 ` [PATCH 1/4] binder: cap BINDER_SET_MAX_THREADS at RLIMIT_NPROC Yunseong Kim
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Yunseong Kim @ 2026-06-03 18:01 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
	Christian Brauner, Carlos Llamas, Alice Ryhl, Brian Swetland,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich
  Cc: Greg Kroah-Hartman, linux-kernel, rust-for-linux, Yunseong Kim,
	Yunseong Kim

Two logic bugs in the Android binder driver (both C and Rust implementations)
allow an unprivileged userspace process to bypass RLIMIT_NPROC and exhaust
kernel memory, leading to system-wide denial of service.

Bug 1: BINDER_SET_MAX_THREADS has no upper bound check
Bug 2: BC_ENTER_LOOPER accepts duplicates without error

These were discovered using kcov-dataflow [1], a per-task function boundary
extraction tool that captures argument values at -O2 where other tools
(KASAN, ftrace, edge coverage) cannot detect logic errors.

[1] https://github.com/yskzalloc/kcov-dataflow
[2] https://lore.kernel.org/all/20260603-kcov-dataflow-next-20260603-v2-0-fee0939de2c4@est.tech/
[3] https://github.com/llvm/llvm-project/pull/201410

--- Userspace PoC (To-Ulimit-and-Beyond.c) ---

/*
 * To-Ulimit-and-Beyond: Demonstrates both bugs from unprivileged userspace.
 * Build: gcc -static -o To-Ulimit-and-Beyond To-Ulimit-and-Beyond.c
 * Run:   ulimit -u 50; ./To-Ulimit-and-Beyond
 *
 * Expected (before fix):
 *   VULNERABLE: kernel accepted max_threads=4294967295!
 *   VULNERABLE: duplicate BC_ENTER_LOOPER accepted!
 *
 * Expected (after fix):
 *   PATCHED: kernel rejected the value
 *   (second BC_ENTER_LOOPER marks thread INVALID internally)
 */

struct binder_write_read {
    int64_t write_size, write_consumed;
    uint64_t write_buffer;
    int64_t read_size, read_consumed;
    uint64_t read_buffer;
};

int main(void)
{
    int fd = open("/dev/binderfs/binder", O_RDWR);
    if (fd < 0) fd = open("/dev/binder", O_RDWR);
    if (fd < 0) { perror("open binder"); return 1; }
    mmap(NULL, 128*1024, PROT_READ, MAP_PRIVATE, fd, 0);

    printf("pid=%d uid=%d\n", getpid(), getuid());

    /* Bug 1: SET_MAX_THREADS with no upper bound */
    uint32_t max = 0xFFFFFFFF;
    int ret = ioctl(fd, BINDER_SET_MAX_THREADS, &max);
    printf("[Bug 1] SET_MAX_THREADS=0xFFFFFFFF: %s (errno=%d)\n",
           ret == 0 ? "VULNERABLE" : "PATCHED", ret < 0 ? errno : 0);

    /* Bug 2: BC_ENTER_LOOPER duplicate */
    uint32_t cmd = BC_ENTER_LOOPER;
    struct binder_write_read bwr = {
        .write_size = sizeof(cmd),
        .write_buffer = (uint64_t)(unsigned long)&cmd,
    };
    ioctl(fd, BINDER_WRITE_READ, &bwr);
    bwr.write_consumed = 0;
    ret = ioctl(fd, BINDER_WRITE_READ, &bwr);
    printf("[Bug 2] BC_ENTER_LOOPER x2: ret=%d (thread now INVALID if patched)\n", ret);

    close(fd);
    return 0;
}

--- ulimit bypass PoC (beyond_ulimit.c) ---

/*
 * Demonstrates RLIMIT_NPROC bypass via binder.
 * With ulimit -u 50, creates 300 threads.
 * Build: gcc -static -pthread -o beyond_ulimit beyond_ulimit.c
 * Run:   ulimit -u 50; ./beyond_ulimit
 */

struct binder_write_read {
    int64_t write_size, write_consumed;
    uint64_t write_buffer;
    int64_t read_size, read_consumed;
    uint64_t read_buffer;
};

static void *binder_thread(void *arg)
{
    int fd = open("/dev/binderfs/binder", O_RDWR);
    if (fd < 0) fd = open("/dev/binder", O_RDWR);
    if (fd < 0) return NULL;
    mmap(NULL, 128*1024, PROT_READ, MAP_PRIVATE, fd, 0);
    uint32_t max = 0xFFFFFFFF;
    ioctl(fd, BINDER_SET_MAX_THREADS, &max);
    uint32_t cmd = BC_REGISTER_LOOPER;
    struct binder_write_read bwr = {
        .write_size = sizeof(cmd),
        .write_buffer = (uint64_t)(unsigned long)&cmd,
    };
    ioctl(fd, BINDER_WRITE_READ, &bwr);
    close(fd);
    return NULL;
}

int main(void)
{
    printf("pid=%d uid=%d\n", getpid(), getuid());
    int created = 0;
    pthread_attr_t attr;
    pthread_attr_init(&attr);
    pthread_attr_setstacksize(&attr, 16384);
    for (int i = 0; i < 300; i++) {
        pthread_t t;
        if (pthread_create(&t, &attr, binder_thread, NULL)) break;
        pthread_detach(t);
        created++;
    }
    usleep(500000);
    printf("Threads created: %d (ulimit was 50)\n", created);
    if (created > 50)
        printf("VULNERABLE: RLIMIT_NPROC bypassed!\n");
    return 0;
}

--- Test results ---

Kernel: 7.1.0-rc5-next-20260528 (CONFIG_ANDROID_BINDER_IPC_RUST=y)
VM: virtme-ng, 1GB RAM, single vCPU

Before fix:
  $ su -s /bin/bash nobody -c 'ulimit -u 50; ./To-Ulimit-and-Beyond'
  pid=288 uid=65534
  [Bug 1] SET_MAX_THREADS=0xFFFFFFFF: VULNERABLE (errno=0)
  [Bug 2] BC_ENTER_LOOPER x2: ret=0

  $ su -s /bin/bash nobody -c 'ulimit -u 50; ./beyond_ulimit'
  Threads created: 300 (ulimit was 50)
  VULNERABLE: RLIMIT_NPROC bypassed!

After fix:
  $ su -s /bin/bash nobody -c 'ulimit -u 50; ./To-Ulimit-and-Beyond'
  pid=283 uid=65534
  [Bug 1] SET_MAX_THREADS=0xFFFFFFFF: PATCHED (errno=22)
  [Bug 2] BC_ENTER_LOOPER x2: ret=0 (thread marked INVALID)

Signed-off-by: Yunseong Kim <yunseong.kim@est.tech>
---
Yunseong Kim (4):
      binder: cap BINDER_SET_MAX_THREADS at RLIMIT_NPROC
      binder: reject duplicate BC_ENTER_LOOPER commands
      rust_binder: cap set_max_threads at RLIMIT_NPROC
      rust_binder: reject duplicate BC_ENTER_LOOPER in looper_enter

 drivers/android/binder.c          | 10 ++++++++++
 drivers/android/binder/process.rs | 15 +++++++++++++--
 drivers/android/binder/thread.rs  |  4 ++++
 3 files changed, 27 insertions(+), 2 deletions(-)
---
base-commit: f7af91adc230aa99e23330ecf85bc9badd9780ad
change-id: 20260603-b4-binder-hardening-2442c7463839

Best regards,
--  
Yunseong Kim <yunseong.kim@est.tech>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/4] binder: cap BINDER_SET_MAX_THREADS at RLIMIT_NPROC
  2026-06-03 18:01 [PATCH 0/4] binder: cap max_threads and reject duplicate looper entry Yunseong Kim
@ 2026-06-03 18:01 ` Yunseong Kim
  2026-06-03 18:01 ` [PATCH 2/4] binder: reject duplicate BC_ENTER_LOOPER commands Yunseong Kim
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Yunseong Kim @ 2026-06-03 18:01 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
	Christian Brauner, Carlos Llamas, Alice Ryhl, Brian Swetland,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich
  Cc: Greg Kroah-Hartman, linux-kernel, rust-for-linux, Yunseong Kim,
	Yunseong Kim

BINDER_SET_MAX_THREADS accepts any u32 value from userspace without
validation. An unprivileged process can set max_threads to 0xFFFFFFFF,
allowing unlimited binder thread spawning via BR_SPAWN_LOOPER. This
bypasses RLIMIT_NPROC because binder thread pool management happens
in kernel context, and exhausts system memory leading to OOM.

Cap max_threads at the calling task's RLIMIT_NPROC, following the same
pattern used by io_uring (io-wq.c) to limit its worker thread count.
This ensures binder respects the per-user resource limits set by the
system administrator.

kcov-dataflow tracking (before):
  ENTRY binder_ioctl(cmd=BINDER_SET_MAX_THREADS, arg=0x7ffc...)
    ENTRY set_max_threads(max=0xffffffff)
    RET   set_max_threads() = 0             ← accepted, no validation
  RET   binder_ioctl() = 0

kcov-dataflow tracking (after):
  ENTRY binder_ioctl(cmd=BINDER_SET_MAX_THREADS, arg=0x7ffc...)
    ENTRY set_max_threads(max=0xffffffff)
    RET   set_max_threads() = -EINVAL       ← rejected, exceeds RLIMIT_NPROC
  RET   binder_ioctl() = -EINVAL

Reproduction:
  $ ulimit -u 50
  $ ./To-Ulimit-and-Beyond  # uid=65534, creates 300+ threads bypassing limit

Fixes: 457b9a6f09f0 ("Staging: android: add binder driver")
Link: https://lore.kernel.org/all/20260603-kcov-dataflow-next-20260603-v2-0-fee0939de2c4@est.tech/
Signed-off-by: Yunseong Kim <yunseong.kim@est.tech>
---
 drivers/android/binder.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index ec0ab4f28530..0f3fc293cdf0 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -5801,6 +5801,10 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 			ret = -EINVAL;
 			goto err;
 		}
+		if (max_threads > rlimit(RLIMIT_NPROC)) {
+			ret = -EINVAL;
+			goto err;
+		}
 		binder_inner_proc_lock(proc);
 		proc->max_threads = max_threads;
 		binder_inner_proc_unlock(proc);

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/4] binder: reject duplicate BC_ENTER_LOOPER commands
  2026-06-03 18:01 [PATCH 0/4] binder: cap max_threads and reject duplicate looper entry Yunseong Kim
  2026-06-03 18:01 ` [PATCH 1/4] binder: cap BINDER_SET_MAX_THREADS at RLIMIT_NPROC Yunseong Kim
@ 2026-06-03 18:01 ` Yunseong Kim
  2026-06-03 18:01 ` [PATCH 3/4] rust_binder: cap set_max_threads at RLIMIT_NPROC Yunseong Kim
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Yunseong Kim @ 2026-06-03 18:01 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
	Christian Brauner, Carlos Llamas, Alice Ryhl, Brian Swetland,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich
  Cc: Greg Kroah-Hartman, linux-kernel, rust-for-linux, Yunseong Kim,
	Yunseong Kim

BC_ENTER_LOOPER can be sent multiple times by a userspace process
without any error. The handler unconditionally ORs in the
BINDER_LOOPER_STATE_ENTERED flag without checking if already set.

While the existing code checks for the REGISTERED→ENTERED conflict,
it does not check for ENTERED→ENTERED. Sending BC_ENTER_LOOPER twice
corrupts the thread pool accounting because the kernel believes it has
more available looper threads than actually exist.

Add a check: if BINDER_LOOPER_STATE_ENTERED is already set, mark the
thread as INVALID and break without re-entering.

kcov-dataflow tracking (before):
  ENTRY binder_thread_write(cmd=BC_ENTER_LOOPER)
    thread->looper |= LOOPER_ENTERED
  RET   binder_thread_write() = 0           ← first call, OK
  ENTRY binder_thread_write(cmd=BC_ENTER_LOOPER)
    thread->looper |= LOOPER_ENTERED        ← duplicate, no check!
  RET   binder_thread_write() = 0           ← silently accepted

kcov-dataflow tracking (after):
  ENTRY binder_thread_write(cmd=BC_ENTER_LOOPER)
    thread->looper |= LOOPER_ENTERED
  RET   binder_thread_write() = 0           ← first call, OK
  ENTRY binder_thread_write(cmd=BC_ENTER_LOOPER)
    thread->looper |= LOOPER_INVALID        ← marked invalid, break
  RET   binder_thread_write() = 0           ← logged, thread invalidated

Reproduction:
  $ ./To-Ulimit-and-Beyond
  First  BC_ENTER_LOOPER: ret=0
  Second BC_ENTER_LOOPER: ret=0  ← both succeed (should reject 2nd)

Fixes: 457b9a6f09f0 ("Staging: android: add binder driver")
Link: https://lore.kernel.org/all/20260603-kcov-dataflow-next-20260603-v2-0-fee0939de2c4@est.tech/
Signed-off-by: Yunseong Kim <yunseong.kim@est.tech>
---
 drivers/android/binder.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 0f3fc293cdf0..0430be814175 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -4365,6 +4365,12 @@ static int binder_thread_write(struct binder_proc *proc,
 				binder_user_error("%d:%d ERROR: BC_ENTER_LOOPER called after BC_REGISTER_LOOPER\n",
 					proc->pid, thread->pid);
 			}
+			if (thread->looper & BINDER_LOOPER_STATE_ENTERED) {
+				thread->looper |= BINDER_LOOPER_STATE_INVALID;
+				binder_user_error("%d:%d ERROR: BC_ENTER_LOOPER called twice\n",
+					proc->pid, thread->pid);
+				break;
+			}
 			thread->looper |= BINDER_LOOPER_STATE_ENTERED;
 			break;
 		case BC_EXIT_LOOPER:

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/4] rust_binder: cap set_max_threads at RLIMIT_NPROC
  2026-06-03 18:01 [PATCH 0/4] binder: cap max_threads and reject duplicate looper entry Yunseong Kim
  2026-06-03 18:01 ` [PATCH 1/4] binder: cap BINDER_SET_MAX_THREADS at RLIMIT_NPROC Yunseong Kim
  2026-06-03 18:01 ` [PATCH 2/4] binder: reject duplicate BC_ENTER_LOOPER commands Yunseong Kim
@ 2026-06-03 18:01 ` Yunseong Kim
  2026-06-03 18:01 ` [PATCH 4/4] rust_binder: reject duplicate BC_ENTER_LOOPER in looper_enter Yunseong Kim
  2026-06-03 18:57 ` [PATCH 0/4] binder: cap max_threads and reject duplicate looper entry Alice Ryhl
  4 siblings, 0 replies; 8+ messages in thread
From: Yunseong Kim @ 2026-06-03 18:01 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
	Christian Brauner, Carlos Llamas, Alice Ryhl, Brian Swetland,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich
  Cc: Greg Kroah-Hartman, linux-kernel, rust-for-linux, Yunseong Kim,
	Yunseong Kim

The Rust binder's set_max_threads() stores the user-provided u32 value
directly into proc.max_threads without any bounds checking. This is the
same bug as in the C binder: an unprivileged process can set max_threads
to 0xFFFFFFFF, enabling unlimited binder thread spawning that bypasses
RLIMIT_NPROC and leads to OOM.

Cap max_threads at the calling task's RLIMIT_NPROC by reading
current->signal->rlim[RLIMIT_NPROC].rlim_cur, following the same
pattern used by io_uring (io-wq.c:1488) to limit its worker threads.

kcov-dataflow tracking (before):
  ENTRY Process::ioctl(cmd=BINDER_SET_MAX_THREADS)
    ENTRY set_max_threads(max=0xffffffff)
      self.inner.lock().max_threads = 0xffffffff  ← stored directly
    RET   set_max_threads()                       ← void, no error

kcov-dataflow tracking (after):
  ENTRY Process::ioctl(cmd=BINDER_SET_MAX_THREADS)
    ENTRY set_max_threads(max=0xffffffff)
      nproc_limit = current->signal->rlim[NPROC] = 50
      0xffffffff > 50                             ← rejected
    RET   set_max_threads() = Err(EINVAL)

Reproduction:
  # CONFIG_ANDROID_BINDER_IPC_RUST=y
  $ ulimit -u 50
  $ ./To-Ulimit-and-Beyond
  BINDER_SET_MAX_THREADS = 0xFFFFFFFF: accepted (before) / rejected (after)

Link: https://lore.kernel.org/all/20260603-kcov-dataflow-next-20260603-v2-0-fee0939de2c4@est.tech/
Signed-off-by: Yunseong Kim <yunseong.kim@est.tech>
---
 drivers/android/binder/process.rs | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
index 96b8440ceac6..4752b84505bd 100644
--- a/drivers/android/binder/process.rs
+++ b/drivers/android/binder/process.rs
@@ -1139,8 +1139,19 @@ fn remove_thread(&self, thread: Arc<Thread>) {
         thread.release();
     }
 
-    fn set_max_threads(&self, max: u32) {
+    fn set_max_threads(&self, max: u32) -> Result {
+        // Cap at the calling task's RLIMIT_NPROC, following the same pattern
+        // as io_uring (io-wq.c) for limiting kernel-side thread creation.
+        // SAFETY: current is valid, signal and rlim are always initialized.
+        let nproc_limit = unsafe {
+            let task = kernel::current!().as_ptr();
+            (*(*(*task).signal).rlim.as_ptr().add(bindings::RLIMIT_NPROC as usize)).rlim_cur
+        };
+        if (max as u64) > (nproc_limit as u64) {
+            return Err(EINVAL);
+        }
         self.inner.lock().max_threads = max;
+        Ok(())
     }
 
     fn set_oneway_spam_detection_enabled(&self, enabled: u32) {
@@ -1574,7 +1585,7 @@ fn ioctl_write_only(
     ) -> Result {
         let thread = this.get_current_thread()?;
         match cmd {
-            uapi::BINDER_SET_MAX_THREADS => this.set_max_threads(reader.read()?),
+            uapi::BINDER_SET_MAX_THREADS => this.set_max_threads(reader.read()?)?,
             uapi::BINDER_THREAD_EXIT => this.remove_thread(thread),
             uapi::BINDER_SET_CONTEXT_MGR => this.set_as_manager(None, &thread)?,
             uapi::BINDER_SET_CONTEXT_MGR_EXT => {

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/4] rust_binder: reject duplicate BC_ENTER_LOOPER in looper_enter
  2026-06-03 18:01 [PATCH 0/4] binder: cap max_threads and reject duplicate looper entry Yunseong Kim
                   ` (2 preceding siblings ...)
  2026-06-03 18:01 ` [PATCH 3/4] rust_binder: cap set_max_threads at RLIMIT_NPROC Yunseong Kim
@ 2026-06-03 18:01 ` Yunseong Kim
  2026-06-03 18:57 ` [PATCH 0/4] binder: cap max_threads and reject duplicate looper entry Alice Ryhl
  4 siblings, 0 replies; 8+ messages in thread
From: Yunseong Kim @ 2026-06-03 18:01 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
	Christian Brauner, Carlos Llamas, Alice Ryhl, Brian Swetland,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich
  Cc: Greg Kroah-Hartman, linux-kernel, rust-for-linux, Yunseong Kim,
	Yunseong Kim

The Rust binder's looper_enter() unconditionally sets LOOPER_ENTERED
without checking whether the flag is already set. A userspace process
can send BC_ENTER_LOOPER multiple times, corrupting the thread pool
state: the kernel believes more looper threads are available than
actually exist, leading to incorrect thread pool management.

Add a guard: if LOOPER_ENTERED is already set, mark the thread as
LOOPER_INVALID and return early without re-entering. This matches the
fix applied to the C binder.

kcov-dataflow tracking (before):
  ENTRY ThreadInner::looper_enter()
    self.looper_flags = 0x00
    self.looper_flags |= LOOPER_ENTERED     ← first call: 0x02
  RET   looper_enter()
  ENTRY ThreadInner::looper_enter()
    self.looper_flags = 0x02
    self.looper_flags |= LOOPER_ENTERED     ← duplicate: still 0x02, no error
  RET   looper_enter()

kcov-dataflow tracking (after):
  ENTRY ThreadInner::looper_enter()
    self.looper_flags = 0x00
    self.looper_flags |= LOOPER_ENTERED     ← first call: 0x02
  RET   looper_enter()
  ENTRY ThreadInner::looper_enter()
    self.looper_flags = 0x02
    LOOPER_ENTERED already set → |= LOOPER_INVALID, return
  RET   looper_enter()                      ← early exit, thread invalidated

Reproduction:
  # CONFIG_ANDROID_BINDER_IPC_RUST=y
  $ ./To-Ulimit-and-Beyond
  First  BC_ENTER_LOOPER: ret=0 (OK)
  Second BC_ENTER_LOOPER: ret=0 (thread now INVALID internally)

Link: https://lore.kernel.org/all/20260603-kcov-dataflow-next-20260603-v2-0-fee0939de2c4@est.tech/
Signed-off-by: Yunseong Kim <yunseong.kim@est.tech>
---
 drivers/android/binder/thread.rs | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/android/binder/thread.rs b/drivers/android/binder/thread.rs
index 97d5f31e8fe3..1d114fa2b4e6 100644
--- a/drivers/android/binder/thread.rs
+++ b/drivers/android/binder/thread.rs
@@ -373,6 +373,10 @@ fn pop_transaction_replied(&mut self, transaction: &DArc<Transaction>) -> bool {
     }
 
     fn looper_enter(&mut self) {
+        if self.looper_flags & LOOPER_ENTERED != 0 {
+            self.looper_flags |= LOOPER_INVALID;
+            return;
+        }
         self.looper_flags |= LOOPER_ENTERED;
         if self.looper_flags & LOOPER_REGISTERED != 0 {
             self.looper_flags |= LOOPER_INVALID;

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/4] binder: cap max_threads and reject duplicate looper entry
  2026-06-03 18:01 [PATCH 0/4] binder: cap max_threads and reject duplicate looper entry Yunseong Kim
                   ` (3 preceding siblings ...)
  2026-06-03 18:01 ` [PATCH 4/4] rust_binder: reject duplicate BC_ENTER_LOOPER in looper_enter Yunseong Kim
@ 2026-06-03 18:57 ` Alice Ryhl
  2026-06-04 20:27   ` Yunseong Kim
  4 siblings, 1 reply; 8+ messages in thread
From: Alice Ryhl @ 2026-06-03 18:57 UTC (permalink / raw)
  To: Yunseong Kim
  Cc: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
	Christian Brauner, Carlos Llamas, Brian Swetland, Miguel Ojeda,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Trevor Gross, Danilo Krummrich,
	Greg Kroah-Hartman, linux-kernel, rust-for-linux, Yunseong Kim

On Wed, Jun 3, 2026 at 8:02 PM Yunseong Kim <yunseong.kim@est.tech> wrote:
>
> Two logic bugs in the Android binder driver (both C and Rust implementations)
> allow an unprivileged userspace process to bypass RLIMIT_NPROC and exhaust
> kernel memory, leading to system-wide denial of service.
>
> Bug 1: BINDER_SET_MAX_THREADS has no upper bound check
> Bug 2: BC_ENTER_LOOPER accepts duplicates without error
>
> These were discovered using kcov-dataflow [1], a per-task function boundary
> extraction tool that captures argument values at -O2 where other tools
> (KASAN, ftrace, edge coverage) cannot detect logic errors.
>
> [1] https://github.com/yskzalloc/kcov-dataflow
> [2] https://lore.kernel.org/all/20260603-kcov-dataflow-next-20260603-v2-0-fee0939de2c4@est.tech/
> [3] https://github.com/llvm/llvm-project/pull/201410
>
> --- Userspace PoC (To-Ulimit-and-Beyond.c) ---
>
> /*
>  * To-Ulimit-and-Beyond: Demonstrates both bugs from unprivileged userspace.
>  * Build: gcc -static -o To-Ulimit-and-Beyond To-Ulimit-and-Beyond.c
>  * Run:   ulimit -u 50; ./To-Ulimit-and-Beyond
>  *
>  * Expected (before fix):
>  *   VULNERABLE: kernel accepted max_threads=4294967295!
>  *   VULNERABLE: duplicate BC_ENTER_LOOPER accepted!
>  *
>  * Expected (after fix):
>  *   PATCHED: kernel rejected the value
>  *   (second BC_ENTER_LOOPER marks thread INVALID internally)
>  */
>
> struct binder_write_read {
>     int64_t write_size, write_consumed;
>     uint64_t write_buffer;
>     int64_t read_size, read_consumed;
>     uint64_t read_buffer;
> };
>
> int main(void)
> {
>     int fd = open("/dev/binderfs/binder", O_RDWR);
>     if (fd < 0) fd = open("/dev/binder", O_RDWR);
>     if (fd < 0) { perror("open binder"); return 1; }
>     mmap(NULL, 128*1024, PROT_READ, MAP_PRIVATE, fd, 0);
>
>     printf("pid=%d uid=%d\n", getpid(), getuid());
>
>     /* Bug 1: SET_MAX_THREADS with no upper bound */
>     uint32_t max = 0xFFFFFFFF;
>     int ret = ioctl(fd, BINDER_SET_MAX_THREADS, &max);
>     printf("[Bug 1] SET_MAX_THREADS=0xFFFFFFFF: %s (errno=%d)\n",
>            ret == 0 ? "VULNERABLE" : "PATCHED", ret < 0 ? errno : 0);
>
>     /* Bug 2: BC_ENTER_LOOPER duplicate */
>     uint32_t cmd = BC_ENTER_LOOPER;
>     struct binder_write_read bwr = {
>         .write_size = sizeof(cmd),
>         .write_buffer = (uint64_t)(unsigned long)&cmd,
>     };
>     ioctl(fd, BINDER_WRITE_READ, &bwr);
>     bwr.write_consumed = 0;
>     ret = ioctl(fd, BINDER_WRITE_READ, &bwr);
>     printf("[Bug 2] BC_ENTER_LOOPER x2: ret=%d (thread now INVALID if patched)\n", ret);
>
>     close(fd);
>     return 0;
> }

So invoking BC_ENTER_LOOPER twice doesn't error and the second call is
a no-op. What's the problem?

> --- ulimit bypass PoC (beyond_ulimit.c) ---
>
> /*
>  * Demonstrates RLIMIT_NPROC bypass via binder.
>  * With ulimit -u 50, creates 300 threads.
>  * Build: gcc -static -pthread -o beyond_ulimit beyond_ulimit.c
>  * Run:   ulimit -u 50; ./beyond_ulimit
>  */
>
> struct binder_write_read {
>     int64_t write_size, write_consumed;
>     uint64_t write_buffer;
>     int64_t read_size, read_consumed;
>     uint64_t read_buffer;
> };
>
> static void *binder_thread(void *arg)
> {
>     int fd = open("/dev/binderfs/binder", O_RDWR);
>     if (fd < 0) fd = open("/dev/binder", O_RDWR);
>     if (fd < 0) return NULL;
>     mmap(NULL, 128*1024, PROT_READ, MAP_PRIVATE, fd, 0);
>     uint32_t max = 0xFFFFFFFF;
>     ioctl(fd, BINDER_SET_MAX_THREADS, &max);
>     uint32_t cmd = BC_REGISTER_LOOPER;
>     struct binder_write_read bwr = {
>         .write_size = sizeof(cmd),
>         .write_buffer = (uint64_t)(unsigned long)&cmd,
>     };
>     ioctl(fd, BINDER_WRITE_READ, &bwr);
>     close(fd);
>     return NULL;
> }
>
> int main(void)
> {
>     printf("pid=%d uid=%d\n", getpid(), getuid());
>     int created = 0;
>     pthread_attr_t attr;
>     pthread_attr_init(&attr);
>     pthread_attr_setstacksize(&attr, 16384);
>     for (int i = 0; i < 300; i++) {
>         pthread_t t;
>         if (pthread_create(&t, &attr, binder_thread, NULL)) break;
>         pthread_detach(t);
>         created++;
>     }
>     usleep(500000);
>     printf("Threads created: %d (ulimit was 50)\n", created);
>     if (created > 50)
>         printf("VULNERABLE: RLIMIT_NPROC bypassed!\n");
>     return 0;
> }

My understanding is that the only thing BINDER_SET_MAX_THREADS does is
cause the kernel to tell userspace "please spawn more threads" when
all threads are in use and there are incoming transactions. I don't
understand how it helps by pass ulimit. Did you try running your test
with the Binder ioctl removed? I'd guess that if it passes now, it
still passes with the Binder ioctl deleted.

Alice

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/4] binder: cap max_threads and reject duplicate looper entry
  2026-06-03 18:57 ` [PATCH 0/4] binder: cap max_threads and reject duplicate looper entry Alice Ryhl
@ 2026-06-04 20:27   ` Yunseong Kim
  2026-06-05  9:55     ` Alice Ryhl
  0 siblings, 1 reply; 8+ messages in thread
From: Yunseong Kim @ 2026-06-04 20:27 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
	Christian Brauner, Carlos Llamas, Brian Swetland, Miguel Ojeda,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Trevor Gross, Danilo Krummrich,
	Greg Kroah-Hartman, linux-kernel, rust-for-linux, Yunseong Kim

Hi Alice,

On 6/3/26 20:57, Alice Ryhl wrote:
> On Wed, Jun 3, 2026 at 8:02 PM Yunseong Kim <yunseong.kim@est.tech> wrote:
>> [snip..]
> 
> So invoking BC_ENTER_LOOPER twice doesn't error and the second call is
> a no-op. What's the problem?
You're right — |= ENTERED when already ENTERED is idempotent at the
bit level. There's no corruption. I'll drop this patch.

>> --- ulimit bypass PoC (beyond_ulimit.c) ---
>>
>> /*
>>  * Demonstrates RLIMIT_NPROC bypass via binder.
>>  * With ulimit -u 50, creates 300 threads.
>>  * Build: gcc -static -pthread -o beyond_ulimit beyond_ulimit.c
>>  * Run:   ulimit -u 50; ./beyond_ulimit
>>  */
>>
>> struct binder_write_read {
>>     int64_t write_size, write_consumed;
>>     uint64_t write_buffer;
>>     int64_t read_size, read_consumed;
>>     uint64_t read_buffer;
>> };
>>
>> static void *binder_thread(void *arg)
>> {
>>     int fd = open("/dev/binderfs/binder", O_RDWR);
>>     if (fd < 0) fd = open("/dev/binder", O_RDWR);
>>     if (fd < 0) return NULL;
>>     mmap(NULL, 128*1024, PROT_READ, MAP_PRIVATE, fd, 0);
>>     uint32_t max = 0xFFFFFFFF;
>>     ioctl(fd, BINDER_SET_MAX_THREADS, &max);
>>     uint32_t cmd = BC_REGISTER_LOOPER;
>>     struct binder_write_read bwr = {
>>         .write_size = sizeof(cmd),
>>         .write_buffer = (uint64_t)(unsigned long)&cmd,
>>     };
>>     ioctl(fd, BINDER_WRITE_READ, &bwr);
>>     close(fd);
>>     return NULL;
>> }
>>
>> int main(void)
>> {
>>     printf("pid=%d uid=%d\n", getpid(), getuid());
>>     int created = 0;
>>     pthread_attr_t attr;
>>     pthread_attr_init(&attr);
>>     pthread_attr_setstacksize(&attr, 16384);
>>     for (int i = 0; i < 300; i++) {
>>         pthread_t t;
>>         if (pthread_create(&t, &attr, binder_thread, NULL)) break;
>>         pthread_detach(t);
>>         created++;
>>     }
>>     usleep(500000);
>>     printf("Threads created: %d (ulimit was 50)\n", created);
>>     if (created > 50)
>>         printf("VULNERABLE: RLIMIT_NPROC bypassed!\n");
>>     return 0;
>> }
> 
> My understanding is that the only thing BINDER_SET_MAX_THREADS does is
> cause the kernel to tell userspace "please spawn more threads" when
> all threads are in use and there are incoming transactions. I don't
> understand how it helps by pass ulimit. Did you try running your test
> with the Binder ioctl removed? I'd guess that if it passes now, it
> still passes with the Binder ioctl deleted.
You're right. I ran the test you suggested and confirmed there is no
bypass — RLIMIT_NPROC is enforced by copy_process() at clone() time,
regardless of binder's max_threads value:

// Test: uid=65534, RLIMIT_NPROC=50, with and without SET_MAX_THREADS
#define _GNU_SOURCE
#include <stdio.h>
#include <pthread.h>
#include <unistd.h>
#include <fcntl.h>
#include <string.h>
#include <sys/ioctl.h>
#include <sys/mman.h>
#include <sys/mount.h>
#include <sys/stat.h>
#include <sys/resource.h>
#include <stdint.h>
#include <linux/android/binder.h>

static void *thread_fn(void *arg) { pause(); return NULL; }

int main(int argc, char **argv)
{
    int use_binder = (argc > 1 && strcmp(argv[1], "binder") == 0);

    mkdir("/dev/binderfs", 0755);
    mount("binder", "/dev/binderfs", "binder", 0, NULL);
    chmod("/dev/binderfs/binder", 0666);

    struct rlimit rl = { .rlim_cur = 50, .rlim_max = 50 };
    setrlimit(RLIMIT_NPROC, &rl);
    setgid(65534);
    setuid(65534);

    if (use_binder) {
        int fd = open("/dev/binderfs/binder", O_RDWR);
        if (fd >= 0) {
            mmap(NULL, 128*1024, PROT_READ, MAP_PRIVATE, fd, 0);
            uint32_t max = 0xFFFFFFFF;
            ioctl(fd, BINDER_SET_MAX_THREADS, &max);
        }
    }

    int created = 0;
    pthread_attr_t attr;
    pthread_attr_init(&attr);
    pthread_attr_setstacksize(&attr, 16384);
    for (int i = 0; i < 300; i++) {
        pthread_t t;
        if (pthread_create(&t, &attr, thread_fn, NULL)) break;
        pthread_detach(t);
        created++;
    }
    printf("[%s] uid=%d RLIMIT_NPROC=50 -> threads: %d\n",
           use_binder ? "WITH binder" : "NO binder", getuid(), created);
    return 0;
}

Result:

    [NO binder]   uid=65534 RLIMIT_NPROC=50 -> threads: 49
    [WITH binder] uid=65534 RLIMIT_NPROC=50 -> threads: 49

Identical. SET_MAX_THREADS has no effect on the thread creation limit.
My original PoC was flawed — it ran as root where RLIMIT_NPROC is not
enforced, making the binder ioctl irrelevant.

I think accepting 0xFFFFFFFF for a thread pool size is arguably poor input
validation. no sane userspace would request 4 billion threads.

Would a separate patch (without Fixes tag) that caps max_threads at a
reasonable upper bound be welcome, or is it not worth the churn? this is
hardening, not a security fix.

> Alice

Thanks again Alice for the careful review.

Kind regards,  
Yunseong

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/4] binder: cap max_threads and reject duplicate looper entry
  2026-06-04 20:27   ` Yunseong Kim
@ 2026-06-05  9:55     ` Alice Ryhl
  0 siblings, 0 replies; 8+ messages in thread
From: Alice Ryhl @ 2026-06-05  9:55 UTC (permalink / raw)
  To: Yunseong Kim
  Cc: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
	Christian Brauner, Carlos Llamas, Brian Swetland, Miguel Ojeda,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Trevor Gross, Danilo Krummrich,
	Greg Kroah-Hartman, linux-kernel, rust-for-linux, Yunseong Kim

On Thu, Jun 04, 2026 at 10:27:19PM +0200, Yunseong Kim wrote:
> Hi Alice,
> 
> On 6/3/26 20:57, Alice Ryhl wrote:
> > On Wed, Jun 3, 2026 at 8:02 PM Yunseong Kim <yunseong.kim@est.tech> wrote:
> > 
> > My understanding is that the only thing BINDER_SET_MAX_THREADS does is
> > cause the kernel to tell userspace "please spawn more threads" when
> > all threads are in use and there are incoming transactions. I don't
> > understand how it helps by pass ulimit. Did you try running your test
> > with the Binder ioctl removed? I'd guess that if it passes now, it
> > still passes with the Binder ioctl deleted.
> You're right. I ran the test you suggested and confirmed there is no
> bypass — RLIMIT_NPROC is enforced by copy_process() at clone() time,
> regardless of binder's max_threads value:
> 
> // Test: uid=65534, RLIMIT_NPROC=50, with and without SET_MAX_THREADS
> #define _GNU_SOURCE
> #include <stdio.h>
> #include <pthread.h>
> #include <unistd.h>
> #include <fcntl.h>
> #include <string.h>
> #include <sys/ioctl.h>
> #include <sys/mman.h>
> #include <sys/mount.h>
> #include <sys/stat.h>
> #include <sys/resource.h>
> #include <stdint.h>
> #include <linux/android/binder.h>
> 
> static void *thread_fn(void *arg) { pause(); return NULL; }
> 
> int main(int argc, char **argv)
> {
>     int use_binder = (argc > 1 && strcmp(argv[1], "binder") == 0);
> 
>     mkdir("/dev/binderfs", 0755);
>     mount("binder", "/dev/binderfs", "binder", 0, NULL);
>     chmod("/dev/binderfs/binder", 0666);
> 
>     struct rlimit rl = { .rlim_cur = 50, .rlim_max = 50 };
>     setrlimit(RLIMIT_NPROC, &rl);
>     setgid(65534);
>     setuid(65534);
> 
>     if (use_binder) {
>         int fd = open("/dev/binderfs/binder", O_RDWR);
>         if (fd >= 0) {
>             mmap(NULL, 128*1024, PROT_READ, MAP_PRIVATE, fd, 0);
>             uint32_t max = 0xFFFFFFFF;
>             ioctl(fd, BINDER_SET_MAX_THREADS, &max);
>         }
>     }
> 
>     int created = 0;
>     pthread_attr_t attr;
>     pthread_attr_init(&attr);
>     pthread_attr_setstacksize(&attr, 16384);
>     for (int i = 0; i < 300; i++) {
>         pthread_t t;
>         if (pthread_create(&t, &attr, thread_fn, NULL)) break;
>         pthread_detach(t);
>         created++;
>     }
>     printf("[%s] uid=%d RLIMIT_NPROC=50 -> threads: %d\n",
>            use_binder ? "WITH binder" : "NO binder", getuid(), created);
>     return 0;
> }
> 
> Result:
> 
>     [NO binder]   uid=65534 RLIMIT_NPROC=50 -> threads: 49
>     [WITH binder] uid=65534 RLIMIT_NPROC=50 -> threads: 49
> 
> Identical. SET_MAX_THREADS has no effect on the thread creation limit.
> My original PoC was flawed — it ran as root where RLIMIT_NPROC is not
> enforced, making the binder ioctl irrelevant.
> 
> I think accepting 0xFFFFFFFF for a thread pool size is arguably poor input
> validation. no sane userspace would request 4 billion threads.
>
> Would a separate patch (without Fixes tag) that caps max_threads at a
> reasonable upper bound be welcome, or is it not worth the churn? this is
> hardening, not a security fix.

I don't think that's useful.

Alice

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-06-05  9:55 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-03 18:01 [PATCH 0/4] binder: cap max_threads and reject duplicate looper entry Yunseong Kim
2026-06-03 18:01 ` [PATCH 1/4] binder: cap BINDER_SET_MAX_THREADS at RLIMIT_NPROC Yunseong Kim
2026-06-03 18:01 ` [PATCH 2/4] binder: reject duplicate BC_ENTER_LOOPER commands Yunseong Kim
2026-06-03 18:01 ` [PATCH 3/4] rust_binder: cap set_max_threads at RLIMIT_NPROC Yunseong Kim
2026-06-03 18:01 ` [PATCH 4/4] rust_binder: reject duplicate BC_ENTER_LOOPER in looper_enter Yunseong Kim
2026-06-03 18:57 ` [PATCH 0/4] binder: cap max_threads and reject duplicate looper entry Alice Ryhl
2026-06-04 20:27   ` Yunseong Kim
2026-06-05  9:55     ` Alice Ryhl

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox