linux-kernel - Re: [PATCH v2 1/1] binder: fix freeze race

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANBPYPgEbFusdyrcV1EqridahQGTH7X=s8ufUA9SMR8SefAO2g@mail.gmail.com>
Date:   Thu, 9 Sep 2021 23:17:42 -0700
From:   Li Li <dualli@...omium.org>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     Li Li <dualli@...gle.com>, Todd Kjos <tkjos@...gle.com>,
        Christian Brauner <christian@...uner.io>,
        Arve Hjønnevåg <arve@...roid.com>,
        "open list:ANDROID DRIVERS" <devel@...verdev.osuosl.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Martijn Coenen <maco@...gle.com>,
        Hridya Valsaraju <hridya@...gle.com>,
        Suren Baghdasaryan <surenb@...gle.com>,
        "Joel Fernandes (Google)" <joel@...lfernandes.org>,
        Android Kernel Team <kernel-team@...roid.com>
Subject: Re: [PATCH v2 1/1] binder: fix freeze race

On Thu, Sep 9, 2021 at 10:38 PM Greg KH <gregkh@...uxfoundation.org> wrote:
>
> On Thu, Sep 09, 2021 at 08:53:16PM -0700, Li Li wrote:
> >  struct binder_frozen_status_info {
> >       __u32            pid;
> > +
> > +     /* process received sync transactions since last frozen
> > +      * bit 0: received sync transaction after being frozen
> > +      * bit 1: new pending sync transaction during freezing
> > +      */
> >       __u32            sync_recv;
>
> You just changed a user/kernel api here, did you just break existing
> userspace applications?  If not, how did that not happen?
>

That's a good question. This design does keep backward compatibility.

The existing userspace applications call ioctl(BINDER_GET_FROZEN_INFO)
to check if there's sync or async binder transactions sent to a frozen
process.

If the existing userspace app runs on a new kernel, a sync binder call
still sets bit 1 of sync_recv (as it's a bool variable) so the ioctl
will return the expected value (TRUE). The app just doesn't check bit
1 intentionally so it doesn't have the ability to tell if there's a
race - this behavior is aligned with what happens on an old kernel as
the old kernel doesn't have bit 1 set at all.

The bit 1 of sync_recv enables new userspace app the ability to tell
1) there's a sync binder transaction happened when being frozen - same
as before; and 2) if that sync binder transaction happens exactly when
there's a race - a new information for rollback decision.

> thanks,
>
> greg k-h

Thanks,
Li