Re: [PATCH] binder: prevent UAF read in print_binder_transaction_log_entry()

[Todd Kjos]

On Wed, Oct 9, 2019 at 3:40 AM Christian Brauner
<christian.brauner@xxxxxxxxxx> wrote:
>
> On Tue, Oct 08, 2019 at 02:05:16PM -0400, Joel Fernandes wrote:
> > On Tue, Oct 08, 2019 at 03:01:59PM +0200, Christian Brauner wrote:

[...]

> >
> > One more thought, this can be made dependent on CONFIG_BINDERFS since regular
> > binder devices cannot be unregistered AFAICS and as Jann said, the problem is
> > BINDERFS specific. That way we avoid the memcpy for _every_ transaction.
> > These can be thundering when Android starts up.
>
> Unless Todd sees this as a real performance problem I'm weary to
> introduce additional checking and record a pointer for non-binderfs and
> a memcpy() for binderfs devices. :)
>

I don't see this as a real problem. In practice, memcpy will be moving
< 10 bytes. Also, by the time this code is in an android device,
CONFIG_BINDERFS will always be enabled since this is how we are
removing binder's use of debugfs. So a micro-optimization of the
!BINDERFS case will not be meaningful.

[...]