All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alice Ryhl <aliceryhl@google.com>
To: Gary Guo <gary@garyguo.net>
Cc: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Carlos Llamas" <cmllamas@google.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	stable@vger.kernel.org, DeepChirp <DeepChirp@outlook.com>
Subject: Re: [PATCH] rust_binder: correctly handle FDA objects of length zero
Date: Mon, 29 Dec 2025 17:04:17 +0000	[thread overview]
Message-ID: <aVK0kS3KqUmpGSDz@google.com> (raw)
In-Reply-To: <20251229164544.1baf659b.gary@garyguo.net>

On Mon, Dec 29, 2025 at 04:45:44PM +0000, Gary Guo wrote:
> On Mon, 29 Dec 2025 15:38:14 +0000
> Alice Ryhl <aliceryhl@google.com> wrote:
> 
> > Fix a bug where an empty FDA (fd array) object with 0 fds would cause an
> > out-of-bounds error. The previous implementation used `skip == 0` to
> > mean "this is a pointer fixup", but 0 is also the correct skip length
> > for an empty FDA. If the FDA is at the end of the buffer, then this
> > results in an attempt to write 8-bytes out of bounds. This is caught and
> > results in an EINVAL error being returned to userspace.
> > 
> > The pattern of using `skip == 0` as a special value originates from the
> > C-implementation of Binder. As part of fixing this bug, this pattern is
> > replaced with a Rust enum.
> 
> I was curious and checked the C binder implementation. Apparently the C
> binder implementation returns early when translating a FD array with
> length 0.
> 
> Would it still make sense to do something similar in the Rust binder? The
> enum change is still good to make, though.

Based on where the early return is, that'd be equivalent in wrapping
this:

	parent_entry
	    .pointer_fixups
	    .push(
	        PointerFixupEntry::Skip {
	            skip: fds_len,
	            target_offset: info.target_offset,
	        },
	        GFP_KERNEL,
	    )
	    .map_err(|_| ENOMEM)?;

in an `if fds_len > 0 {}` block. I don't believe it makes any
difference, but not having a special case may be cleaner?

Alice

  reply	other threads:[~2025-12-29 17:04 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-29 15:38 [PATCH] rust_binder: correctly handle FDA objects of length zero Alice Ryhl
2025-12-29 16:45 ` Gary Guo
2025-12-29 17:04   ` Alice Ryhl [this message]
2026-01-06 18:20 ` Carlos Llamas

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=aVK0kS3KqUmpGSDz@google.com \
    --to=aliceryhl@google.com \
    --cc=DeepChirp@outlook.com \
    --cc=a.hindborg@kernel.org \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=cmllamas@google.com \
    --cc=dakr@kernel.org \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=tmgross@umich.edu \
    /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.