Re: [PATCH v1 1/3] binder: BINDER_FREEZE ioctl

From: Li Li
Date: Thu Mar 11 2021 - 04:37:21 EST


On Wed, Mar 10, 2021 at 11:33 PM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Wed, Mar 10, 2021 at 02:52:49PM -0800, Li Li wrote:
> > if (target_proc) {
> > binder_inner_proc_lock(target_proc);
> > + target_proc->outstanding_txns--;
> > + WARN_ON(target_proc->outstanding_txns < 0);
>
> WARN_* is a huge crutch, please just handle stuff like this properly and
> if you really need to, warn userspace (but what can they do about it?)
>
> You also just rebooted all systems that have panic-on-warn set, so if
> this can be triggered by userspace, you caused a DoS of things :(
>
> So please remove all of the WARN_ON() you add in this patch series to
> properly handle the error conditions and deal with them correctly.
>
> And if these were here just for debugging, hopefully the code works
> properly now and you do not need debugging anymore so they can all just
> be dropped.

When the target_proc is freed, there's no outstanding transactions already.
The FREEZE ioctl from userspace won't trigger this. It's for debugging.
And I'll remove it in v2. Thanks for the suggestion!