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

[Carlos Llamas <cmllamas@google.com>]

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>