All of lore.kernel.org
 help / color / mirror / Atom feed
From: Breno Leitao <leitao@debian.org>
To: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc: Marcel Holtmann <marcel@holtmann.org>,
	 "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	 Jakub Kicinski <kuba@kernel.org>,
	Paolo Abeni <pabeni@redhat.com>, Simon Horman <horms@kernel.org>,
	 Shuah Khan <shuah@kernel.org>,
	linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org,
	 netdev@vger.kernel.org, linux-kselftest@vger.kernel.org
Subject: Re: [PATCH net-next v2 0/6] Bluetooth: convert remaining bluetooth socket families to getsockopt_iter
Date: Mon, 1 Jun 2026 07:21:29 -0700	[thread overview]
Message-ID: <ah2GaClhUehy5ys9@gmail.com> (raw)
In-Reply-To: <CABBYNZ+nvtMHcU9but5VEG891vyO=K0MF3OAGofANxzdLVmw-g@mail.gmail.com>

Hello Luiz,

On Thu, May 14, 2026 at 01:51:16PM -0400, Luiz Augusto von Dentz wrote:
> On Tue, May 12, 2026 at 7:12 AM Breno Leitao <leitao@debian.org> wrote:
> >
> > Continue the conversion to .getsockopt_iter for the Bluetooth socket
> > families: hci_sock, ISO, RFCOMM, SCO and L2CAP. The first patch is a
> > small precursor that fixes a long-standing 1-byte put_user write in
> > hci_sock_getsockopt_old() so the subsequent conversion stays mechanical.
> >
> > The riskiest change in this series is the SCO BT_CODEC conversion: it
> > is the only one that drops an open-coded ptr cursor in favour of
> > relying on iter_out advancing naturally on every copy_to_iter() call.
> > Every other socket option is a near-mechanical s/copy_to_user/
> > copy_to_iter/ rewrite, but BT_CODEC walks a variable-length list of
> > codecs + capabilities and previously tracked its own write offset by
> > hand. Getting the cursor semantics wrong here would silently truncate
> > or misalign user-visible codec data.
> >
> > For more context about the motivation for this change, please check
> > commit 67fab22a7ad ("net: add getsockopt_iter callback to proto_ops")
> >
> > Signed-off-by: Breno Leitao <leitao@debian.org>
> > ---
> > Changes in v2:
> > - rebase the tree on top of bluetooth-next.
> > - Remove the selftest, which was mixing network and bluetooth together.
> > - Link to v1: https://patch.msgid.link/20260511-getsock_three-v1-0-1461fa8786ab@debian.org
> >
> > To: Marcel Holtmann <marcel@holtmann.org>
> > To: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
> > Cc: linux-bluetooth@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> >
> > ---
> > Breno Leitao (6):
> >       Bluetooth: hci_sock: write the full optval for getsockopt
> >       Bluetooth: hci_sock: convert to getsockopt_iter
> >       Bluetooth: ISO: convert to getsockopt_iter
> >       Bluetooth: RFCOMM: convert to getsockopt_iter
> >       Bluetooth: L2CAP: convert to getsockopt_iter
> >       Bluetooth: SCO: convert to getsockopt_iter
> >
> >  net/bluetooth/hci_sock.c    | 26 +++++++++++--------
> >  net/bluetooth/iso.c         | 27 ++++++++++----------
> >  net/bluetooth/l2cap_sock.c  | 61 +++++++++++++++++++++++++++------------------
> >  net/bluetooth/rfcomm/sock.c | 30 ++++++++++++----------
> >  net/bluetooth/sco.c         | 59 ++++++++++++++++++++++---------------------
> >  5 files changed, 114 insertions(+), 89 deletions(-)
> > ---
> > base-commit: c2f0079e8c42fd6814c8d6b1491e3ce0a0e3b3fa
> > change-id: 20260511-getsock_three-d0d7f1b2629e
> >
> > Best regards,
> > --
> > Breno Leitao <leitao@debian.org>
>
> There are some comments from sashiko on this:
>
> https://sashiko.dev/#/patchset/20260512-getsock_three-v2-0-30b7b22ef14c%40debian.org
>
> Now Im not sure the truncating is actually used by our tools, but that
> sounds like it could break userspace if we don't check properly, or
> have you already done this for other socket families and it was
> considered to be ok?

You're right — to clarify, the patch changes behavior when the user
provides a short optlen, preventing the overflow that existed before:

 - Old code: put_user(opt, (u32 __user *)optval) unconditionally
   writes sizeof(*ptr) bytes regardless of optlen, so optlen < 4
   would overflow the user buffer and return 0.

 - New code: copy_to_iter() respects the optlen boundary, eliminating
   the overflow — but now a short buffer fails the strict length check
   and returns -EFAULT instead of 0.

I don't believe any legitimate userspace relies on the old overflow
behavior, though there's a theoretical risk of breakage. I'm not deeply
familiar with the Bluetooth ecosystem specifically, but, I would prefer to
avoid the buffer overflow in such case.

I've addressed a similar issue in another subsystem recently:

https://lore.kernel.org/all/20260521-fix_llc-v2-1-ab44cc09179c@debian.org/

Anyway, let me know if you need me to change it.
--breno


      reply	other threads:[~2026-06-01 14:21 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-12 11:12 [PATCH net-next v2 0/6] Bluetooth: convert remaining bluetooth socket families to getsockopt_iter Breno Leitao
2026-05-12 11:12 ` [PATCH net-next v2 1/6] Bluetooth: hci_sock: write the full optval for getsockopt Breno Leitao
2026-05-12 13:44   ` Bluetooth: convert remaining bluetooth socket families to getsockopt_iter bluez.test.bot
2026-05-12 11:12 ` [PATCH net-next v2 2/6] Bluetooth: hci_sock: convert " Breno Leitao
2026-05-12 11:12 ` [PATCH net-next v2 3/6] Bluetooth: ISO: " Breno Leitao
2026-05-12 11:12 ` [PATCH net-next v2 4/6] Bluetooth: RFCOMM: " Breno Leitao
2026-05-12 11:12 ` [PATCH net-next v2 5/6] Bluetooth: L2CAP: " Breno Leitao
2026-05-12 11:12 ` [PATCH net-next v2 6/6] Bluetooth: SCO: " Breno Leitao
2026-05-12 17:50 ` [PATCH net-next v2 0/6] Bluetooth: convert remaining bluetooth socket families " patchwork-bot+bluetooth
2026-05-14 17:51 ` Luiz Augusto von Dentz
2026-06-01 14:21   ` Breno Leitao [this message]

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=ah2GaClhUehy5ys9@gmail.com \
    --to=leitao@debian.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=luiz.dentz@gmail.com \
    --cc=marcel@holtmann.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=shuah@kernel.org \
    /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.