All of lore.kernel.org
 help / color / mirror / Atom feed
From: Carlos Llamas <cmllamas@google.com>
To: Alice Ryhl <aliceryhl@google.com>
Cc: "Yu-Ting Tseng" <yutingtseng@google.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Arve Hjønnevåg" <arve@android.com>,
	"Todd Kjos" <tkjos@android.com>,
	"Martijn Coenen" <maco@android.com>,
	"Joel Fernandes" <joel@joelfernandes.org>,
	"Christian Brauner" <brauner@kernel.org>,
	"Suren Baghdasaryan" <surenb@google.com>,
	linux-kernel@vger.kernel.org, kernel-team@android.com,
	stable@vger.kernel.org
Subject: Re: [PATCH v2 6/8] binder: allow freeze notification for dead nodes
Date: Tue, 8 Oct 2024 18:12:10 +0000	[thread overview]
Message-ID: <ZwV1-svdRZCF2x2H@google.com> (raw)
In-Reply-To: <CAH5fLgjB-ia+UhE1P8gOxHTdjSJJ1=xKSS0c75AvGA91uo_fEw@mail.gmail.com>

On Mon, Sep 30, 2024 at 03:30:01PM +0200, Alice Ryhl wrote:
> On Fri, Sep 27, 2024 at 6:32 PM Carlos Llamas <cmllamas@google.com> wrote:
> >
> > There are different ways to proceed with this dead node scenario:
> >
> > 1. return ESRCH
> > 2. silently fail and don't allocate a ref->freeze
> > 3. allocate a ref->freeze but don't notify the current state
> > 4. allocate and send a "fake" state notification.
> >
> > I like 1 just because it is technically the correct thing to do from the
> > driver's perspective. However, it does complicate things in userspace as
> > we've discussed. Option 2, could work but it would also fail with EINVAL
> > if a "clear notification" is sent later anyway. Option 3 changes the
> > behavior of guaranteeing a notification upon success. Option 4 can cause
> > trouble on how a "not-frozen" notification is handled in userspace e.g
> > start sending transactions.
> >
> > As you can see there is no clear winner here, we have to compromise
> > something and option #3 is the best we can do IMO.
> 
> I am happy with both #3 and #4. I think #1 and #2 are problematic
> because they will lead to userspace getting errors on correct use of
> Binder.

After talking with userspace folks it seems that #3 would be their
preferred approach. So this v2 patch it the way to go then!

Thanks,
Carlos Llamas

  reply	other threads:[~2024-10-08 18:12 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-26 23:36 [PATCH v2 0/8] binder: several fixes for frozen notification Carlos Llamas
2024-09-26 23:36 ` [PATCH v2 1/8] binder: fix node UAF in binder_add_freeze_work() Carlos Llamas
2024-09-26 23:36 ` [PATCH v2 2/8] binder: fix OOB " Carlos Llamas
2024-09-26 23:36 ` [PATCH v2 3/8] binder: fix freeze UAF in binder_release_work() Carlos Llamas
2024-09-26 23:36 ` [PATCH v2 4/8] binder: fix BINDER_WORK_FROZEN_BINDER debug logs Carlos Llamas
2024-09-27  7:07   ` Alice Ryhl
2024-09-26 23:36 ` [PATCH v2 5/8] binder: fix BINDER_WORK_CLEAR_FREEZE_NOTIFICATION " Carlos Llamas
2024-09-27  0:34   ` Todd Kjos
2024-09-27  7:20   ` Alice Ryhl
2024-09-26 23:36 ` [PATCH v2 6/8] binder: allow freeze notification for dead nodes Carlos Llamas
2024-09-27  0:48   ` Todd Kjos
2024-09-27  7:19   ` Alice Ryhl
2024-09-27 16:13     ` Yu-Ting Tseng
2024-09-27 16:15       ` Alice Ryhl
2024-09-27 16:32         ` Carlos Llamas
2024-09-30 13:30           ` Alice Ryhl
2024-10-08 18:12             ` Carlos Llamas [this message]
2024-09-26 23:36 ` [PATCH v2 7/8] binder: fix memleak of proc->delivered_freeze Carlos Llamas
2024-09-27  0:52   ` Todd Kjos
2024-09-27 10:19   ` Alice Ryhl
2024-09-26 23:36 ` [PATCH v2 8/8] binder: add delivered_freeze to debugfs output Carlos Llamas
2024-09-27  0:38   ` Todd Kjos
2024-09-27  7:19   ` Alice Ryhl
2024-09-27 10:20 ` [PATCH v2 0/8] binder: several fixes for frozen notification Alice Ryhl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZwV1-svdRZCF2x2H@google.com \
    --to=cmllamas@google.com \
    --cc=aliceryhl@google.com \
    --cc=arve@android.com \
    --cc=brauner@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=joel@joelfernandes.org \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maco@android.com \
    --cc=stable@vger.kernel.org \
    --cc=surenb@google.com \
    --cc=tkjos@android.com \
    --cc=yutingtseng@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.