Re: [PATCH] binder: fix descriptor lookup for context manager

[Alice Ryhl]

On Tue, Jul 16, 2024 at 7:40 PM Todd Kjos <tkjos@xxxxxxxxxx> wrote:
>
> On Mon, Jul 15, 2024 at 9:29 PM Carlos Llamas <cmllamas@xxxxxxxxxx> wrote:
> >         /* 0 is reserved for the context manager */
> > -       if (node == proc->context->binder_context_mgr_node) {
> > -               *desc = 0;
> > -               return 0;
> > -       }
> > +       offset = (node == proc->context->binder_context_mgr_node) ? 0 : 1;
>
> If context manager doesn't need to be bit 0 anymore, then why do we
> bother to prefer bit 0? Does it matter?
>
> It would simplify the code below if the offset is always 0 since you
> wouldn't need an offset at all.

Userspace assumes that sending a message to handle 0 means that the
current context manager receives it. If we assign anything that is not
the context manager to bit 0, then libbinder will send ctxmgr messages
to random other processes. I don't think libbinder handles the case
where context manager is restarted well at all. Most likely, if we hit
this condition in real life, processes that had a non-zero refcount to
the context manager will lose the ability to interact with ctxmgr
until they are restarted.

I think this patch just needs to make sure that this scenario doesn't
lead to a UAF in the kernel. Ensuring that userspace handles it
gracefully is another matter.

Alice