Re: [PATCH] Bluetooth: SCO: check for codecs->num_codecs == 1 before assigning to sco_pi(sk)->codec

[Stefan Metzmacher]

Hi Luiz,

> On Tue, Apr 7, 2026 at 11:13 AM Stefan Metzmacher <metze@xxxxxxxxx> wrote:
> > copy_struct_from_sockptr() fill 'buffer' in
> > sco_sock_setsockopt() with zeros, so there's no
> > real problem.
> >
> > But it actually looks strange to do this,
> > without checking all of codecs->codecs[0]
> > really comes from userspace:
> >
> >    sco_pi(sk)->codec = codecs->codecs[0];
> >
> > As only optlen < sizeof(struct bt_codecs) is checked
> > and codecs->num_codecs is not checked against != 1,
> > but only <= 1, and the space for the additional struct bt_codec
> > is not checked.
> >
> > Note I don't understand bluetooth and I didn't do any runtime
> > tests with this! I just found it when debugging a problem
> > in copy_struct_from_sockptr().
> >
> > I just added this to check the size is as expected:
> >
> >    BUILD_BUG_ON(struct_size(codecs, codecs, 0) != 1);
> >    BUILD_BUG_ON(struct_size(codecs, codecs, 1) != 8);
> >
> > And made sure it still compiles using this:
> >
> >    make CF=-D__CHECK_ENDIAN__ W=1ce C=1 net/bluetooth/sco.o
> >
> > Fixes: 3e643e4efa1e ("Bluetooth: Improve setsockopt() handling of malformed user input")
> > Cc: Michal Luczaj <mhal@xxxxxxx>
> > Cc: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
> > Cc: Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx>
> > Cc: Marcel Holtmann <marcel@xxxxxxxxxxxx>
> > Cc: David Wei <dw@xxxxxxxxxxx>
> > Cc: linux-bluetooth@xxxxxxxxxxxxxxx
> > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > Signed-off-by: Stefan Metzmacher <metze@xxxxxxxxx>
> > ---
> >   net/bluetooth/sco.c | 8 +++++++-
> >   1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> > index b84587811ef4..359eabf7dddb 100644
> > --- a/net/bluetooth/sco.c
> > +++ b/net/bluetooth/sco.c
> > @@ -1045,7 +1045,13 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
> >
> >                  codecs = (void *)buffer;
> >
> > -               if (codecs->num_codecs > 1) {
> > +               if (codecs->num_codecs != 1) {
> > +                       hci_dev_put(hdev);
> > +                       err = -EINVAL;
> > +                       break;
> > +               }
> > +
> > +               if (optlen < struct_size(codecs, codecs, codecs->num_codecs)) {
>
> I guess this is odd because there was no need for this code to exist,
> BT_CODEC should have used struct bt_codec rather than struct bt_codecs
> since we can only set exactly one codec, that said I don't think we
> can change this now thus why we need to perform these checks, anyway
> back to your changes why don't we have both checks for num_codecs != 1
> && optlen < struct_size(codecs, codecs, codecs->num_codecs)) since the
> handling is the same?

Should I submit a v2 using this:
if (codecs->num_codecs != 1 || optlen < struct_size(codecs, codecs, codecs->num_codecs))

Or can you to take it from here?
As I don't really care and the patch is not strictly needed.

Thanks!
metze