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

[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@xxxxxxxx> 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