binder: allow freeze notification for dead nodes

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/
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Acked-by: Todd Kjos <tkjos@google.com>
Link: https://lore.kernel.org/r/20240926233632.821189-7-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 73dc6cbc168154fdfa71cb5f08770271c5ef6af5..415fc97592497b5139e9f1ba3886ecfdff5ef8df 100644 (file)
--- 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);