From: Carlos Llamas <cmllamas@google.com>
To: "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>,
"Carlos Llamas" <cmllamas@google.com>,
"Suren Baghdasaryan" <surenb@google.com>,
"Yu-Ting Tseng" <yutingtseng@google.com>
Cc: linux-kernel@vger.kernel.org, kernel-team@android.com,
Alice Ryhl <aliceryhl@google.com>,
stable@vger.kernel.org
Subject: [PATCH v2 6/8] binder: allow freeze notification for dead nodes
Date: Thu, 26 Sep 2024 23:36:17 +0000 [thread overview]
Message-ID: <20240926233632.821189-7-cmllamas@google.com> (raw)
In-Reply-To: <20240926233632.821189-1-cmllamas@google.com>
Alice points out that binder_request_freeze_notification() should not
return EINVAL when the relevant node is dead [1]. The node can die at
any point even if the user input is valid. Instead, allow the request
to be allocated but skip the initial notification for dead nodes. This
avoids propagating unnecessary errors back to userspace.
Fixes: d579b04a52a1 ("binder: frozen notification")
Cc: stable@vger.kernel.org
Suggested-by: Alice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/all/CAH5fLghapZJ4PbbkC8V5A6Zay-_sgTzwVpwqk6RWWUNKKyJC_Q@mail.gmail.com/ [1]
Signed-off-by: Carlos Llamas <cmllamas@google.com>
---
drivers/android/binder.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 73dc6cbc1681..415fc9759249 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -3856,7 +3856,6 @@ binder_request_freeze_notification(struct binder_proc *proc,
{
struct binder_ref_freeze *freeze;
struct binder_ref *ref;
- bool is_frozen;
freeze = kzalloc(sizeof(*freeze), GFP_KERNEL);
if (!freeze)
@@ -3872,32 +3871,31 @@ binder_request_freeze_notification(struct binder_proc *proc,
}
binder_node_lock(ref->node);
-
- if (ref->freeze || !ref->node->proc) {
- binder_user_error("%d:%d invalid BC_REQUEST_FREEZE_NOTIFICATION %s\n",
- proc->pid, thread->pid,
- ref->freeze ? "already set" : "dead node");
+ if (ref->freeze) {
+ binder_user_error("%d:%d BC_REQUEST_FREEZE_NOTIFICATION already set\n",
+ proc->pid, thread->pid);
binder_node_unlock(ref->node);
binder_proc_unlock(proc);
kfree(freeze);
return -EINVAL;
}
- binder_inner_proc_lock(ref->node->proc);
- is_frozen = ref->node->proc->is_frozen;
- binder_inner_proc_unlock(ref->node->proc);
binder_stats_created(BINDER_STAT_FREEZE);
INIT_LIST_HEAD(&freeze->work.entry);
freeze->cookie = handle_cookie->cookie;
freeze->work.type = BINDER_WORK_FROZEN_BINDER;
- freeze->is_frozen = is_frozen;
-
ref->freeze = freeze;
- binder_inner_proc_lock(proc);
- binder_enqueue_work_ilocked(&ref->freeze->work, &proc->todo);
- binder_wakeup_proc_ilocked(proc);
- binder_inner_proc_unlock(proc);
+ if (ref->node->proc) {
+ binder_inner_proc_lock(ref->node->proc);
+ freeze->is_frozen = ref->node->proc->is_frozen;
+ binder_inner_proc_unlock(ref->node->proc);
+
+ binder_inner_proc_lock(proc);
+ binder_enqueue_work_ilocked(&freeze->work, &proc->todo);
+ binder_wakeup_proc_ilocked(proc);
+ binder_inner_proc_unlock(proc);
+ }
binder_node_unlock(ref->node);
binder_proc_unlock(proc);
--
2.46.1.824.gd892dcdcdd-goog
next prev parent reply other threads:[~2024-09-26 23:36 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 ` Carlos Llamas [this message]
2024-09-27 0:48 ` [PATCH v2 6/8] binder: allow freeze notification for dead nodes 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
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=20240926233632.821189-7-cmllamas@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox