Re: [PATCH 0/4] binder: cap max_threads and reject duplicate looper entry

[Alice Ryhl <aliceryhl@google.com>]

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