All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gavin Li <gav@thegavinli.com>
To: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc: Marcel Holtmann <marcel@holtmann.org>,
	Johan Hedberg <johan.hedberg@gmail.com>,
	"linux-bluetooth@vger.kernel.org"
	<linux-bluetooth@vger.kernel.org>, Gavin Li <git@thegavinli.com>
Subject: Re: [PATCH] Bluetooth: ensure valid channel mode when creating l2cap conn on LE
Date: Sun, 16 Jan 2022 23:48:23 -0800	[thread overview]
Message-ID: <CAHxvarA8-FTmRX9FHC0GXGE7o_iW3SpxiaCkzOzbnd15_4x9Bg@mail.gmail.com> (raw)
In-Reply-To: <CABBYNZ+b4MehqjjXt7gZYi=b0yRrNRvYgTAJqjmy8Z+fM3SNjw@mail.gmail.com>

This looks good; thanks for catching that.

Gavin


On Fri, Jan 14, 2022 at 3:00 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi,
>
> On Fri, Jan 14, 2022 at 2:52 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi,
> >
> > On Wed, Jan 12, 2022 at 2:17 AM <gav@thegavinli.com> wrote:
> > >
> > > From: Gavin Li <git@thegavinli.com>
> > >
> > > After creating a socket(AF_INET, SOCK_STREAM, BTPROTO_L2CAP) socket and
> > > connect()'ing to a LE device with default settings (no setsockopt), upon
> > > the first sendmsg, the following BUG occurs because chan->mode==L2CAP_MODE_ERTM,
> > > causing l2cap_ertm_send() -> __set_retrans_timer() -> schedule_delayed_work()
> > > on l2cap_chan.retrans_timer, which was never initialized because
> > > l2cap_ertm_init() was never called to initialize it.
> > >
> > >   Call Trace:
> > >    queue_delayed_work_on+0x36/0x40
> > >    l2cap_ertm_send.isra.0+0x14d/0x2d0 [bluetooth]
> > >    l2cap_tx+0x361/0x510 [bluetooth]
> > >    l2cap_chan_send+0xb26/0xb50 [bluetooth]
> > >    l2cap_sock_sendmsg+0xc9/0x100 [bluetooth]
> > >    sock_sendmsg+0x5e/0x60
> > >    sock_write_iter+0x97/0x100
> > >    new_sync_write+0x1d3/0x1f0
> > >    vfs_write+0x1b4/0x270
> > >    ksys_write+0xaf/0xe0
> > >    do_syscall_64+0x33/0x40
> > >    entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > >
> > > This patch ensures that when connecting to a LE device, chan->mode will
> > > always be corrected to L2CAP_MODE_LE_FLOWCTL if it is invalid for LE.
> > >
> > > Signed-off-by: Gavin Li <git@thegavinli.com>
> > > ---
> > >  net/bluetooth/l2cap_sock.c | 15 +++++++++++++--
> > >  1 file changed, 13 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> > > index 160c016a5dfb9..58c06ef32656c 100644
> > > --- a/net/bluetooth/l2cap_sock.c
> > > +++ b/net/bluetooth/l2cap_sock.c
> > > @@ -78,6 +78,17 @@ static int l2cap_validate_le_psm(u16 psm)
> > >         return 0;
> > >  }
> > >
> > > +static bool l2cap_mode_supports_le(u8 mode)
> > > +{
> > > +       switch (mode) {
> > > +               case L2CAP_MODE_LE_FLOWCTL:
> > > +               case L2CAP_MODE_EXT_FLOWCTL:
> > > +                       return true;
> > > +               default:
> > > +                       return false;
> > > +       }
> > > +}
> > > +
> > >  static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
> > >  {
> > >         struct sock *sk = sock->sk;
> > > @@ -161,7 +172,7 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
> > >                 break;
> > >         }
> > >
> > > -       if (chan->psm && bdaddr_type_is_le(chan->src_type))
> > > +       if (chan->psm && bdaddr_type_is_le(la.l2_bdaddr_type) && !l2cap_mode_supports_le(chan->mode))
> > >                 chan->mode = L2CAP_MODE_LE_FLOWCTL;
> > >
> > >         chan->state = BT_BOUND;
> > > @@ -240,7 +251,7 @@ static int l2cap_sock_connect(struct socket *sock, struct sockaddr *addr,
> > >                         return -EINVAL;
> > >         }
> > >
> > > -       if (chan->psm && bdaddr_type_is_le(chan->src_type) && !chan->mode)
> > > +       if (chan->psm && bdaddr_type_is_le(la.l2_bdaddr_type) && !l2cap_mode_supports_le(chan->mode))
> > >                 chan->mode = L2CAP_MODE_LE_FLOWCTL;
> > >
> > >         err = l2cap_chan_connect(chan, la.l2_psm, __le16_to_cpu(la.l2_cid),
> > > --
> > > 2.34.1
> >
> > Doesn't apply to bluetooth-next:
> >
> > https://github.com/bluez/bluez/issues/250
>
> Please disregard the link above, Ive meant to paste:
>
> Applying: Bluetooth: ensure valid channel mode when creating l2cap conn on LE
> error: patch failed: net/bluetooth/l2cap_sock.c:161
> error: net/bluetooth/l2cap_sock.c: patch does not apply
>
> I did fix something similar:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=30d57722732d9736554f85f75f9d7ad5402d192e
>
> --
> Luiz Augusto von Dentz

  reply	other threads:[~2022-01-17  7:48 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-12 10:17 [PATCH] Bluetooth: ensure valid channel mode when creating l2cap conn on LE gav
2022-01-14 22:52 ` Luiz Augusto von Dentz
2022-01-14 23:00   ` Luiz Augusto von Dentz
2022-01-17  7:48     ` Gavin Li [this message]
2022-01-19 17:06 ` Marcel Holtmann

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=CAHxvarA8-FTmRX9FHC0GXGE7o_iW3SpxiaCkzOzbnd15_4x9Bg@mail.gmail.com \
    --to=gav@thegavinli.com \
    --cc=git@thegavinli.com \
    --cc=johan.hedberg@gmail.com \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=luiz.dentz@gmail.com \
    --cc=marcel@holtmann.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.