[PATCH v2 0/3] Fix three issues with freeze listeners

[PATCH v2 0/3] Fix three issues with freeze listeners

[Alice Ryhl]

See each commit message for details.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Changes in v2:
- Add fully frozen fix (patch 3).
- Link to v1: https://lore.kernel.org/r/20251004-binder-freeze-v1-0-fb9fcdcf2b73@google.com

---
Alice Ryhl (3):
      rust_binder: freeze_notif_done should resend if wrong state
      rust_binder: don't delete FreezeListener if there are pending duplicates
      rust_binder: report freeze notification only when fully frozen

 drivers/android/binder/freeze.rs      | 18 +++++++++++---
 drivers/android/binder/process.rs     | 46 ++++++++++++++++++++++++++++-------
 drivers/android/binder/transaction.rs |  6 ++---
 3 files changed, 54 insertions(+), 16 deletions(-)
---
base-commit: eafedbc7c050c44744fbdf80bdf3315e860b7513
change-id: 20251002-binder-freeze-f99e23dc47bf

Best regards,
-- 
Alice Ryhl <aliceryhl@google.com>

[PATCH v2 1/3] rust_binder: freeze_notif_done should resend if wrong state

[Alice Ryhl]

Consider the following scenario:
1. A freeze notification is delivered to thread 1.
2. The process becomes frozen or unfrozen.
3. The message for step 2 is delivered to thread 2 and ignored because
   there is already a pending notification from step 1.
4. Thread 1 acknowledges the notification from step 1.
In this case, step 4 should ensure that the message ignored in step 3 is
resent as it can now be delivered.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 drivers/android/binder/freeze.rs | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/android/binder/freeze.rs b/drivers/android/binder/freeze.rs
index e68c3c8bc55a203c32261c23915d8c427569e3b0..74bebb8d4d9b24860eed34363ce69b1c6df58028 100644
--- a/drivers/android/binder/freeze.rs
+++ b/drivers/android/binder/freeze.rs
@@ -245,8 +245,9 @@ pub(crate) fn freeze_notif_done(self: &Arc<Self>, reader: &mut UserSliceReader)
                 );
                 return Err(EINVAL);
             }
-            if freeze.is_clearing {
-                // Immediately send another FreezeMessage for BR_CLEAR_FREEZE_NOTIFICATION_DONE.
+            let is_frozen = freeze.node.owner.inner.lock().is_frozen;
+            if freeze.is_clearing || freeze.last_is_frozen != Some(is_frozen) {
+                // Immediately send another FreezeMessage.
                 clear_msg = Some(FreezeMessage::init(alloc, cookie));
             }
             freeze.is_pending = false;

-- 
2.51.0.618.g983fd99d29-goog

[PATCH v2 2/3] rust_binder: don't delete FreezeListener if there are pending duplicates

[Alice Ryhl]

When userspace issues commands to a freeze listener, it identifies it
using a cookie. Normally this cookie uniquely identifies a freeze
listener, but when userspace clears a listener with the intent of
deleting it, it's allowed to "regret" clearing it and create a new
freeze listener for the same node using the same cookie. (IMO this was
an API mistake, but userspace relies on it.)

Currently if the active freeze listener gets fully deleted while there
are still pending duplicates, then the code incorrectly deletes the
pending duplicates too. To fix this, do not delete the entry if there
are still pending duplicates.

Since the current data structure requires a main freeze listener, we
convert one pending duplicate into the primary listener in this
scenario.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 drivers/android/binder/freeze.rs | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/android/binder/freeze.rs b/drivers/android/binder/freeze.rs
index 74bebb8d4d9b24860eed34363ce69b1c6df58028..e304aceca7f31c15444cf67bb13488cd144345e6 100644
--- a/drivers/android/binder/freeze.rs
+++ b/drivers/android/binder/freeze.rs
@@ -106,7 +106,16 @@ fn do_work(
             return Ok(true);
         }
         if freeze.is_clearing {
-            _removed_listener = freeze_entry.remove_node();
+            kernel::warn_on!(freeze.num_cleared_duplicates != 0);
+            if freeze.num_pending_duplicates > 0 {
+                // The primary freeze listener was deleted, so convert a pending duplicate back
+                // into the primary one.
+                freeze.num_pending_duplicates -= 1;
+                freeze.is_pending = true;
+                freeze.is_clearing = true;
+            } else {
+                _removed_listener = freeze_entry.remove_node();
+            }
             drop(node_refs);
             writer.write_code(BR_CLEAR_FREEZE_NOTIFICATION_DONE)?;
             writer.write_payload(&self.cookie.0)?;

-- 
2.51.0.618.g983fd99d29-goog

[PATCH v2 3/3] rust_binder: report freeze notification only when fully frozen

[Alice Ryhl]

Binder only sends out freeze notifications when ioctl_freeze() completes
and the process has become fully frozen. However, if a freeze
notification is registered during the freeze operation, then it
registers an initial state of 'frozen'. This is a problem because if
the freeze operation fails, then the listener is not told about that
state change, leading to lost updates.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
This is also an issue in C Binder. A fix for that will be sent
separately.
---
 drivers/android/binder/freeze.rs      |  4 +--
 drivers/android/binder/process.rs     | 46 ++++++++++++++++++++++++++++-------
 drivers/android/binder/transaction.rs |  6 ++---
 3 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/drivers/android/binder/freeze.rs b/drivers/android/binder/freeze.rs
index e304aceca7f31c15444cf67bb13488cd144345e6..220de35ae85ac8de2af717129011e0ace0677b7d 100644
--- a/drivers/android/binder/freeze.rs
+++ b/drivers/android/binder/freeze.rs
@@ -121,7 +121,7 @@ fn do_work(
             writer.write_payload(&self.cookie.0)?;
             Ok(true)
         } else {
-            let is_frozen = freeze.node.owner.inner.lock().is_frozen;
+            let is_frozen = freeze.node.owner.inner.lock().is_frozen.is_fully_frozen();
             if freeze.last_is_frozen == Some(is_frozen) {
                 return Ok(true);
             }
@@ -254,7 +254,7 @@ pub(crate) fn freeze_notif_done(self: &Arc<Self>, reader: &mut UserSliceReader)
                 );
                 return Err(EINVAL);
             }
-            let is_frozen = freeze.node.owner.inner.lock().is_frozen;
+            let is_frozen = freeze.node.owner.inner.lock().is_frozen.is_fully_frozen();
             if freeze.is_clearing || freeze.last_is_frozen != Some(is_frozen) {
                 // Immediately send another FreezeMessage.
                 clear_msg = Some(FreezeMessage::init(alloc, cookie));
diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
index f13a747e784c84a0fb09cbf47442712106eba07c..2da9344397506a3f6d6b13411c45d5ded92d6db1 100644
--- a/drivers/android/binder/process.rs
+++ b/drivers/android/binder/process.rs
@@ -72,6 +72,33 @@ fn new(address: usize, size: usize) -> Self {
 const PROC_DEFER_FLUSH: u8 = 1;
 const PROC_DEFER_RELEASE: u8 = 2;
 
+#[derive(Copy, Clone)]
+pub(crate) enum IsFrozen {
+    Yes,
+    No,
+    InProgress,
+}
+
+impl IsFrozen {
+    /// Whether incoming transactions should be rejected due to freeze.
+    pub(crate) fn is_frozen(self) -> bool {
+        match self {
+            IsFrozen::Yes => true,
+            IsFrozen::No => false,
+            IsFrozen::InProgress => true,
+        }
+    }
+
+    /// Whether freeze notifications consider this process frozen.
+    pub(crate) fn is_fully_frozen(self) -> bool {
+        match self {
+            IsFrozen::Yes => true,
+            IsFrozen::No => false,
+            IsFrozen::InProgress => false,
+        }
+    }
+}
+
 /// The fields of `Process` protected by the spinlock.
 pub(crate) struct ProcessInner {
     is_manager: bool,
@@ -98,7 +125,7 @@ pub(crate) struct ProcessInner {
     /// are woken up.
     outstanding_txns: u32,
     /// Process is frozen and unable to service binder transactions.
-    pub(crate) is_frozen: bool,
+    pub(crate) is_frozen: IsFrozen,
     /// Process received sync transactions since last frozen.
     pub(crate) sync_recv: bool,
     /// Process received async transactions since last frozen.
@@ -124,7 +151,7 @@ fn new() -> Self {
             started_thread_count: 0,
             defer_work: 0,
             outstanding_txns: 0,
-            is_frozen: false,
+            is_frozen: IsFrozen::No,
             sync_recv: false,
             async_recv: false,
             binderfs_file: None,
@@ -1260,7 +1287,7 @@ fn deferred_release(self: Arc<Self>) {
         let is_manager = {
             let mut inner = self.inner.lock();
             inner.is_dead = true;
-            inner.is_frozen = false;
+            inner.is_frozen = IsFrozen::No;
             inner.sync_recv = false;
             inner.async_recv = false;
             inner.is_manager
@@ -1371,7 +1398,7 @@ pub(crate) fn drop_outstanding_txn(&self) {
                 return;
             }
             inner.outstanding_txns -= 1;
-            inner.is_frozen && inner.outstanding_txns == 0
+            inner.is_frozen.is_frozen() && inner.outstanding_txns == 0
         };
 
         if wake {
@@ -1385,7 +1412,7 @@ pub(crate) fn ioctl_freeze(&self, info: &BinderFreezeInfo) -> Result {
             let mut inner = self.inner.lock();
             inner.sync_recv = false;
             inner.async_recv = false;
-            inner.is_frozen = false;
+            inner.is_frozen = IsFrozen::No;
             drop(inner);
             msgs.send_messages();
             return Ok(());
@@ -1394,7 +1421,7 @@ pub(crate) fn ioctl_freeze(&self, info: &BinderFreezeInfo) -> Result {
         let mut inner = self.inner.lock();
         inner.sync_recv = false;
         inner.async_recv = false;
-        inner.is_frozen = true;
+        inner.is_frozen = IsFrozen::InProgress;
 
         if info.timeout_ms > 0 {
             let mut jiffies = kernel::time::msecs_to_jiffies(info.timeout_ms);
@@ -1408,7 +1435,7 @@ pub(crate) fn ioctl_freeze(&self, info: &BinderFreezeInfo) -> Result {
                     .wait_interruptible_timeout(&mut inner, jiffies)
                 {
                     CondVarTimeoutResult::Signal { .. } => {
-                        inner.is_frozen = false;
+                        inner.is_frozen = IsFrozen::No;
                         return Err(ERESTARTSYS);
                     }
                     CondVarTimeoutResult::Woken { jiffies: remaining } => {
@@ -1422,17 +1449,18 @@ pub(crate) fn ioctl_freeze(&self, info: &BinderFreezeInfo) -> Result {
         }
 
         if inner.txns_pending_locked() {
-            inner.is_frozen = false;
+            inner.is_frozen = IsFrozen::No;
             Err(EAGAIN)
         } else {
             drop(inner);
             match self.prepare_freeze_messages() {
                 Ok(batch) => {
+                    self.inner.lock().is_frozen = IsFrozen::Yes;
                     batch.send_messages();
                     Ok(())
                 }
                 Err(kernel::alloc::AllocError) => {
-                    self.inner.lock().is_frozen = false;
+                    self.inner.lock().is_frozen = IsFrozen::No;
                     Err(ENOMEM)
                 }
             }
diff --git a/drivers/android/binder/transaction.rs b/drivers/android/binder/transaction.rs
index 02512175d6229535373f2d3e543ba8c91ecd72f0..4bd3c0e417eb93d5d62d9c20daadde1fb0e4990f 100644
--- a/drivers/android/binder/transaction.rs
+++ b/drivers/android/binder/transaction.rs
@@ -249,7 +249,7 @@ pub(crate) fn submit(self: DLArc<Self>) -> BinderResult {
 
         if oneway {
             if let Some(target_node) = self.target_node.clone() {
-                if process_inner.is_frozen {
+                if process_inner.is_frozen.is_frozen() {
                     process_inner.async_recv = true;
                     if self.flags & TF_UPDATE_TXN != 0 {
                         if let Some(t_outdated) =
@@ -270,7 +270,7 @@ pub(crate) fn submit(self: DLArc<Self>) -> BinderResult {
                     }
                 }
 
-                if process_inner.is_frozen {
+                if process_inner.is_frozen.is_frozen() {
                     return Err(BinderError::new_frozen_oneway());
                 } else {
                     return Ok(());
@@ -280,7 +280,7 @@ pub(crate) fn submit(self: DLArc<Self>) -> BinderResult {
             }
         }
 
-        if process_inner.is_frozen {
+        if process_inner.is_frozen.is_frozen() {
             process_inner.sync_recv = true;
             return Err(BinderError::new_frozen());
         }

-- 
2.51.0.618.g983fd99d29-goog

Re: [PATCH v2 1/3] rust_binder: freeze_notif_done should resend if wrong state

[Carlos Llamas]

On Tue, Oct 07, 2025 at 09:39:51AM +0000, Alice Ryhl wrote:
> Consider the following scenario:
> 1. A freeze notification is delivered to thread 1.
> 2. The process becomes frozen or unfrozen.
> 3. The message for step 2 is delivered to thread 2 and ignored because
>    there is already a pending notification from step 1.
> 4. Thread 1 acknowledges the notification from step 1.
> In this case, step 4 should ensure that the message ignored in step 3 is
> resent as it can now be delivered.

hmmm, I wonder what happens with 3 threads involved where the state goes
back to the (unconsumed) initial freeze notification. Userspace will
probably see two separate notifications of the same state?

> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  drivers/android/binder/freeze.rs | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/android/binder/freeze.rs b/drivers/android/binder/freeze.rs
> index e68c3c8bc55a203c32261c23915d8c427569e3b0..74bebb8d4d9b24860eed34363ce69b1c6df58028 100644
> --- a/drivers/android/binder/freeze.rs
> +++ b/drivers/android/binder/freeze.rs
> @@ -245,8 +245,9 @@ pub(crate) fn freeze_notif_done(self: &Arc<Self>, reader: &mut UserSliceReader)
>                  );
>                  return Err(EINVAL);
>              }
> -            if freeze.is_clearing {
> -                // Immediately send another FreezeMessage for BR_CLEAR_FREEZE_NOTIFICATION_DONE.
> +            let is_frozen = freeze.node.owner.inner.lock().is_frozen;
> +            if freeze.is_clearing || freeze.last_is_frozen != Some(is_frozen) {
> +                // Immediately send another FreezeMessage.
>                  clear_msg = Some(FreezeMessage::init(alloc, cookie));
>              }
>              freeze.is_pending = false;
> 
> -- 
> 2.51.0.618.g983fd99d29-goog
> 

Acked-by: Carlos Llamas <cmllamas@google.com>

Re: [PATCH v2 1/3] rust_binder: freeze_notif_done should resend if wrong state

[Alice Ryhl]

On Wed, Oct 8, 2025 at 6:32 PM Carlos Llamas <cmllamas@google.com> wrote:
>
> On Tue, Oct 07, 2025 at 09:39:51AM +0000, Alice Ryhl wrote:
> > Consider the following scenario:
> > 1. A freeze notification is delivered to thread 1.
> > 2. The process becomes frozen or unfrozen.
> > 3. The message for step 2 is delivered to thread 2 and ignored because
> >    there is already a pending notification from step 1.
> > 4. Thread 1 acknowledges the notification from step 1.
> > In this case, step 4 should ensure that the message ignored in step 3 is
> > resent as it can now be delivered.
>
> hmmm, I wonder what happens with 3 threads involved where the state goes
> back to the (unconsumed) initial freeze notification. Userspace will
> probably see two separate notifications of the same state?

The way I implemented it, the work items report the current state when
the work item is *executed*, and they do nothing if there's no change
since last notification.

Alice

Re: [PATCH v2 1/3] rust_binder: freeze_notif_done should resend if wrong state

[Carlos Llamas]

On Wed, Oct 08, 2025 at 06:34:54PM +0200, Alice Ryhl wrote:
> On Wed, Oct 8, 2025 at 6:32 PM Carlos Llamas <cmllamas@google.com> wrote:
> >
> > On Tue, Oct 07, 2025 at 09:39:51AM +0000, Alice Ryhl wrote:
> > > Consider the following scenario:
> > > 1. A freeze notification is delivered to thread 1.
> > > 2. The process becomes frozen or unfrozen.
> > > 3. The message for step 2 is delivered to thread 2 and ignored because
> > >    there is already a pending notification from step 1.
> > > 4. Thread 1 acknowledges the notification from step 1.
> > > In this case, step 4 should ensure that the message ignored in step 3 is
> > > resent as it can now be delivered.
> >
> > hmmm, I wonder what happens with 3 threads involved where the state goes
> > back to the (unconsumed) initial freeze notification. Userspace will
> > probably see two separate notifications of the same state?
> 
> The way I implemented it, the work items report the current state when
> the work item is *executed*, and they do nothing if there's no change
> since last notification.

Oh I see, then that means the 2nd and 3rd notifications would do nothing
as the state went back to the last notification, correct?

Re: [PATCH v2 1/3] rust_binder: freeze_notif_done should resend if wrong state

[Alice Ryhl]

On Wed, Oct 8, 2025 at 6:38 PM Carlos Llamas <cmllamas@google.com> wrote:
>
> On Wed, Oct 08, 2025 at 06:34:54PM +0200, Alice Ryhl wrote:
> > On Wed, Oct 8, 2025 at 6:32 PM Carlos Llamas <cmllamas@google.com> wrote:
> > >
> > > On Tue, Oct 07, 2025 at 09:39:51AM +0000, Alice Ryhl wrote:
> > > > Consider the following scenario:
> > > > 1. A freeze notification is delivered to thread 1.
> > > > 2. The process becomes frozen or unfrozen.
> > > > 3. The message for step 2 is delivered to thread 2 and ignored because
> > > >    there is already a pending notification from step 1.
> > > > 4. Thread 1 acknowledges the notification from step 1.
> > > > In this case, step 4 should ensure that the message ignored in step 3 is
> > > > resent as it can now be delivered.
> > >
> > > hmmm, I wonder what happens with 3 threads involved where the state goes
> > > back to the (unconsumed) initial freeze notification. Userspace will
> > > probably see two separate notifications of the same state?
> >
> > The way I implemented it, the work items report the current state when
> > the work item is *executed*, and they do nothing if there's no change
> > since last notification.
>
> Oh I see, then that means the 2nd and 3rd notifications would do nothing
> as the state went back to the last notification, correct?

Yeah.

If the state flips quickly, userspace might not get told about that if
it's too slow to receive the update, but that's no different from C
Binder.

Alice

Re: [PATCH v2 1/3] rust_binder: freeze_notif_done should resend if wrong state

[Carlos Llamas]

On Wed, Oct 08, 2025 at 06:41:20PM +0200, Alice Ryhl wrote:
> On Wed, Oct 8, 2025 at 6:38 PM Carlos Llamas <cmllamas@google.com> wrote:
> >
> > On Wed, Oct 08, 2025 at 06:34:54PM +0200, Alice Ryhl wrote:
> > > On Wed, Oct 8, 2025 at 6:32 PM Carlos Llamas <cmllamas@google.com> wrote:
> > > >
> > > > On Tue, Oct 07, 2025 at 09:39:51AM +0000, Alice Ryhl wrote:
> > > > > Consider the following scenario:
> > > > > 1. A freeze notification is delivered to thread 1.
> > > > > 2. The process becomes frozen or unfrozen.
> > > > > 3. The message for step 2 is delivered to thread 2 and ignored because
> > > > >    there is already a pending notification from step 1.
> > > > > 4. Thread 1 acknowledges the notification from step 1.
> > > > > In this case, step 4 should ensure that the message ignored in step 3 is
> > > > > resent as it can now be delivered.
> > > >
> > > > hmmm, I wonder what happens with 3 threads involved where the state goes
> > > > back to the (unconsumed) initial freeze notification. Userspace will
> > > > probably see two separate notifications of the same state?
> > >
> > > The way I implemented it, the work items report the current state when
> > > the work item is *executed*, and they do nothing if there's no change
> > > since last notification.
> >
> > Oh I see, then that means the 2nd and 3rd notifications would do nothing
> > as the state went back to the last notification, correct?
> 
> Yeah.
> 
> If the state flips quickly, userspace might not get told about that if
> it's too slow to receive the update, but that's no different from C
> Binder.

I believe the difference is C binder doesn't report the current state at
the time of consuming the notification. So I'm thinking that it would
report two notifications regardless of the state, even if they are both
the same. Oh well.

Thanks,
Carlos Llamas

Re: [PATCH v2 2/3] rust_binder: don't delete FreezeListener if there are pending duplicates

[Carlos Llamas]

On Tue, Oct 07, 2025 at 09:39:52AM +0000, Alice Ryhl wrote:
> When userspace issues commands to a freeze listener, it identifies it
> using a cookie. Normally this cookie uniquely identifies a freeze
> listener, but when userspace clears a listener with the intent of
> deleting it, it's allowed to "regret" clearing it and create a new
> freeze listener for the same node using the same cookie. (IMO this was
> an API mistake, but userspace relies on it.)
> 
> Currently if the active freeze listener gets fully deleted while there
> are still pending duplicates, then the code incorrectly deletes the
> pending duplicates too. To fix this, do not delete the entry if there
> are still pending duplicates.
> 
> Since the current data structure requires a main freeze listener, we
> convert one pending duplicate into the primary listener in this
> scenario.
> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  drivers/android/binder/freeze.rs | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/android/binder/freeze.rs b/drivers/android/binder/freeze.rs
> index 74bebb8d4d9b24860eed34363ce69b1c6df58028..e304aceca7f31c15444cf67bb13488cd144345e6 100644
> --- a/drivers/android/binder/freeze.rs
> +++ b/drivers/android/binder/freeze.rs
> @@ -106,7 +106,16 @@ fn do_work(
>              return Ok(true);
>          }
>          if freeze.is_clearing {
> -            _removed_listener = freeze_entry.remove_node();
> +            kernel::warn_on!(freeze.num_cleared_duplicates != 0);
> +            if freeze.num_pending_duplicates > 0 {
> +                // The primary freeze listener was deleted, so convert a pending duplicate back
> +                // into the primary one.
> +                freeze.num_pending_duplicates -= 1;
> +                freeze.is_pending = true;
> +                freeze.is_clearing = true;
> +            } else {
> +                _removed_listener = freeze_entry.remove_node();
> +            }
>              drop(node_refs);
>              writer.write_code(BR_CLEAR_FREEZE_NOTIFICATION_DONE)?;
>              writer.write_payload(&self.cookie.0)?;
> 
> -- 
> 2.51.0.618.g983fd99d29-goog
> 

Acked-by: Carlos Llamas <cmllamas@google.com>

Re: [PATCH v2 3/3] rust_binder: report freeze notification only when fully frozen

[Carlos Llamas]

On Tue, Oct 07, 2025 at 09:39:53AM +0000, Alice Ryhl wrote:
> Binder only sends out freeze notifications when ioctl_freeze() completes
> and the process has become fully frozen. However, if a freeze
> notification is registered during the freeze operation, then it
> registers an initial state of 'frozen'. This is a problem because if
> the freeze operation fails, then the listener is not told about that
> state change, leading to lost updates.
> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> This is also an issue in C Binder. A fix for that will be sent
> separately.
> ---
>  drivers/android/binder/freeze.rs      |  4 +--
>  drivers/android/binder/process.rs     | 46 ++++++++++++++++++++++++++++-------
>  drivers/android/binder/transaction.rs |  6 ++---
>  3 files changed, 42 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/android/binder/freeze.rs b/drivers/android/binder/freeze.rs
> index e304aceca7f31c15444cf67bb13488cd144345e6..220de35ae85ac8de2af717129011e0ace0677b7d 100644
> --- a/drivers/android/binder/freeze.rs
> +++ b/drivers/android/binder/freeze.rs
> @@ -121,7 +121,7 @@ fn do_work(
>              writer.write_payload(&self.cookie.0)?;
>              Ok(true)
>          } else {
> -            let is_frozen = freeze.node.owner.inner.lock().is_frozen;
> +            let is_frozen = freeze.node.owner.inner.lock().is_frozen.is_fully_frozen();
>              if freeze.last_is_frozen == Some(is_frozen) {
>                  return Ok(true);
>              }
> @@ -254,7 +254,7 @@ pub(crate) fn freeze_notif_done(self: &Arc<Self>, reader: &mut UserSliceReader)
>                  );
>                  return Err(EINVAL);
>              }
> -            let is_frozen = freeze.node.owner.inner.lock().is_frozen;
> +            let is_frozen = freeze.node.owner.inner.lock().is_frozen.is_fully_frozen();
>              if freeze.is_clearing || freeze.last_is_frozen != Some(is_frozen) {
>                  // Immediately send another FreezeMessage.
>                  clear_msg = Some(FreezeMessage::init(alloc, cookie));
> diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
> index f13a747e784c84a0fb09cbf47442712106eba07c..2da9344397506a3f6d6b13411c45d5ded92d6db1 100644
> --- a/drivers/android/binder/process.rs
> +++ b/drivers/android/binder/process.rs
> @@ -72,6 +72,33 @@ fn new(address: usize, size: usize) -> Self {
>  const PROC_DEFER_FLUSH: u8 = 1;
>  const PROC_DEFER_RELEASE: u8 = 2;
>  
> +#[derive(Copy, Clone)]
> +pub(crate) enum IsFrozen {
> +    Yes,
> +    No,
> +    InProgress,
> +}
> +
> +impl IsFrozen {
> +    /// Whether incoming transactions should be rejected due to freeze.
> +    pub(crate) fn is_frozen(self) -> bool {
> +        match self {
> +            IsFrozen::Yes => true,
> +            IsFrozen::No => false,
> +            IsFrozen::InProgress => true,
> +        }
> +    }
> +
> +    /// Whether freeze notifications consider this process frozen.
> +    pub(crate) fn is_fully_frozen(self) -> bool {
> +        match self {
> +            IsFrozen::Yes => true,
> +            IsFrozen::No => false,
> +            IsFrozen::InProgress => false,
> +        }
> +    }
> +}
> +
>  /// The fields of `Process` protected by the spinlock.
>  pub(crate) struct ProcessInner {
>      is_manager: bool,
> @@ -98,7 +125,7 @@ pub(crate) struct ProcessInner {
>      /// are woken up.
>      outstanding_txns: u32,
>      /// Process is frozen and unable to service binder transactions.
> -    pub(crate) is_frozen: bool,
> +    pub(crate) is_frozen: IsFrozen,
>      /// Process received sync transactions since last frozen.
>      pub(crate) sync_recv: bool,
>      /// Process received async transactions since last frozen.
> @@ -124,7 +151,7 @@ fn new() -> Self {
>              started_thread_count: 0,
>              defer_work: 0,
>              outstanding_txns: 0,
> -            is_frozen: false,
> +            is_frozen: IsFrozen::No,
>              sync_recv: false,
>              async_recv: false,
>              binderfs_file: None,
> @@ -1260,7 +1287,7 @@ fn deferred_release(self: Arc<Self>) {
>          let is_manager = {
>              let mut inner = self.inner.lock();
>              inner.is_dead = true;
> -            inner.is_frozen = false;
> +            inner.is_frozen = IsFrozen::No;
>              inner.sync_recv = false;
>              inner.async_recv = false;
>              inner.is_manager
> @@ -1371,7 +1398,7 @@ pub(crate) fn drop_outstanding_txn(&self) {
>                  return;
>              }
>              inner.outstanding_txns -= 1;
> -            inner.is_frozen && inner.outstanding_txns == 0
> +            inner.is_frozen.is_frozen() && inner.outstanding_txns == 0
>          };
>  
>          if wake {
> @@ -1385,7 +1412,7 @@ pub(crate) fn ioctl_freeze(&self, info: &BinderFreezeInfo) -> Result {
>              let mut inner = self.inner.lock();
>              inner.sync_recv = false;
>              inner.async_recv = false;
> -            inner.is_frozen = false;
> +            inner.is_frozen = IsFrozen::No;
>              drop(inner);
>              msgs.send_messages();
>              return Ok(());
> @@ -1394,7 +1421,7 @@ pub(crate) fn ioctl_freeze(&self, info: &BinderFreezeInfo) -> Result {
>          let mut inner = self.inner.lock();
>          inner.sync_recv = false;
>          inner.async_recv = false;
> -        inner.is_frozen = true;
> +        inner.is_frozen = IsFrozen::InProgress;
>  
>          if info.timeout_ms > 0 {
>              let mut jiffies = kernel::time::msecs_to_jiffies(info.timeout_ms);
> @@ -1408,7 +1435,7 @@ pub(crate) fn ioctl_freeze(&self, info: &BinderFreezeInfo) -> Result {
>                      .wait_interruptible_timeout(&mut inner, jiffies)
>                  {
>                      CondVarTimeoutResult::Signal { .. } => {
> -                        inner.is_frozen = false;
> +                        inner.is_frozen = IsFrozen::No;
>                          return Err(ERESTARTSYS);
>                      }
>                      CondVarTimeoutResult::Woken { jiffies: remaining } => {
> @@ -1422,17 +1449,18 @@ pub(crate) fn ioctl_freeze(&self, info: &BinderFreezeInfo) -> Result {
>          }
>  
>          if inner.txns_pending_locked() {
> -            inner.is_frozen = false;
> +            inner.is_frozen = IsFrozen::No;
>              Err(EAGAIN)
>          } else {
>              drop(inner);
>              match self.prepare_freeze_messages() {
>                  Ok(batch) => {
> +                    self.inner.lock().is_frozen = IsFrozen::Yes;
>                      batch.send_messages();
>                      Ok(())
>                  }
>                  Err(kernel::alloc::AllocError) => {
> -                    self.inner.lock().is_frozen = false;
> +                    self.inner.lock().is_frozen = IsFrozen::No;
>                      Err(ENOMEM)
>                  }
>              }
> diff --git a/drivers/android/binder/transaction.rs b/drivers/android/binder/transaction.rs
> index 02512175d6229535373f2d3e543ba8c91ecd72f0..4bd3c0e417eb93d5d62d9c20daadde1fb0e4990f 100644
> --- a/drivers/android/binder/transaction.rs
> +++ b/drivers/android/binder/transaction.rs
> @@ -249,7 +249,7 @@ pub(crate) fn submit(self: DLArc<Self>) -> BinderResult {
>  
>          if oneway {
>              if let Some(target_node) = self.target_node.clone() {
> -                if process_inner.is_frozen {
> +                if process_inner.is_frozen.is_frozen() {
>                      process_inner.async_recv = true;
>                      if self.flags & TF_UPDATE_TXN != 0 {
>                          if let Some(t_outdated) =
> @@ -270,7 +270,7 @@ pub(crate) fn submit(self: DLArc<Self>) -> BinderResult {
>                      }
>                  }
>  
> -                if process_inner.is_frozen {
> +                if process_inner.is_frozen.is_frozen() {
>                      return Err(BinderError::new_frozen_oneway());
>                  } else {
>                      return Ok(());
> @@ -280,7 +280,7 @@ pub(crate) fn submit(self: DLArc<Self>) -> BinderResult {
>              }
>          }
>  
> -        if process_inner.is_frozen {
> +        if process_inner.is_frozen.is_frozen() {
>              process_inner.sync_recv = true;
>              return Err(BinderError::new_frozen());
>          }
> 
> -- 
> 2.51.0.618.g983fd99d29-goog
> 

Acked-by: Carlos Llamas <cmllamas@google.com>

Re: [PATCH v2 1/3] rust_binder: freeze_notif_done should resend if wrong state

[Alice Ryhl]

On Wed, Oct 08, 2025 at 04:52:54PM +0000, Carlos Llamas wrote:
> On Wed, Oct 08, 2025 at 06:41:20PM +0200, Alice Ryhl wrote:
> > On Wed, Oct 8, 2025 at 6:38 PM Carlos Llamas <cmllamas@google.com> wrote:
> > >
> > > On Wed, Oct 08, 2025 at 06:34:54PM +0200, Alice Ryhl wrote:
> > > > On Wed, Oct 8, 2025 at 6:32 PM Carlos Llamas <cmllamas@google.com> wrote:
> > > > >
> > > > > On Tue, Oct 07, 2025 at 09:39:51AM +0000, Alice Ryhl wrote:
> > > > > > Consider the following scenario:
> > > > > > 1. A freeze notification is delivered to thread 1.
> > > > > > 2. The process becomes frozen or unfrozen.
> > > > > > 3. The message for step 2 is delivered to thread 2 and ignored because
> > > > > >    there is already a pending notification from step 1.
> > > > > > 4. Thread 1 acknowledges the notification from step 1.
> > > > > > In this case, step 4 should ensure that the message ignored in step 3 is
> > > > > > resent as it can now be delivered.
> > > > >
> > > > > hmmm, I wonder what happens with 3 threads involved where the state goes
> > > > > back to the (unconsumed) initial freeze notification. Userspace will
> > > > > probably see two separate notifications of the same state?
> > > >
> > > > The way I implemented it, the work items report the current state when
> > > > the work item is *executed*, and they do nothing if there's no change
> > > > since last notification.
> > >
> > > Oh I see, then that means the 2nd and 3rd notifications would do nothing
> > > as the state went back to the last notification, correct?
> > 
> > Yeah.
> > 
> > If the state flips quickly, userspace might not get told about that if
> > it's too slow to receive the update, but that's no different from C
> > Binder.
> 
> I believe the difference is C binder doesn't report the current state at
> the time of consuming the notification. So I'm thinking that it would
> report two notifications regardless of the state, even if they are both
> the same. Oh well.

Yeah, I guess if it only toggles once, then C Binder will report that.
On the other hand, if it toggles multiple times, then C Binder might not
report it that many times.

We could make Rust Binder faithfully report the callback the right
number of times in the face of toggling, but it seems not worth it.

Alice