From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
To: Sungwoo Kim <iam@sung-woo.kim>
Cc: daveti@purdue.edu, Marcel Holtmann <marcel@holtmann.org>,
Johan Hedberg <johan.hedberg@gmail.com>,
linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Bluetooth: L2CAP: Fix div-by-zero in l2cap_le_flowctl_init()
Date: Wed, 1 May 2024 18:36:26 -0400 [thread overview]
Message-ID: <CABBYNZ+Mr1c7UKa4Vey7Wmh-+WQaFU0P8AifBf7HDEb7G8PTJQ@mail.gmail.com> (raw)
In-Reply-To: <CAJNyHpKzhFBJ3N0eF1x0icd7O1VkWbTA9k0Gkm8aCMonsagYwg@mail.gmail.com>
Hi Sungwoo,
On Wed, May 1, 2024 at 6:23 PM Sungwoo Kim <iam@sung-woo.kim> wrote:
>
> Dear Luiz,
>
> On Mon, Apr 29, 2024 at 11:15 AM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Sungwoo,
> >
> > On Sun, Apr 28, 2024 at 1:43 AM Sungwoo Kim <iam@sung-woo.kim> wrote:
> > >
> > > Hello, could you review this bug and its patch?
> > >
> > > l2cap_le_flowctl_init() can cause both div-by-zero and an integer overflow.
> > >
> > > l2cap_le_flowctl_init()
> > > chan->mps = min_t(u16, chan->imtu, chan->conn->mtu - L2CAP_HDR_SIZE);
> > > chan->rx_credits = (chan->imtu / chan->mps) + 1; <- div-by-zero
> > >
> > > Here, mtu could be less than or equal to L2CAP_HDR_SIZE (4). If mtu is 4, it
> > > causes div-by-zero. If mtu is less than 4, it causes an integer overflow.
> >
> > That is because it is not valid to have hdev->le_mtu < 0x001b (the
> > range is 0x001b to 0xffff), so we should really look into checking
> > that conn->mtu is actually valid.
> >
> > > How mtu could have such low value:
> > >
> > > hci_cc_le_read_buffer_size()
> > > hdev->le_mtu = __le16_to_cpu(rp->le_mtu);
> > >
> > > l2cap_conn_add()
> > > conn->mtu = hcon->hdev->le_mtu;
> >
> > Yeah this assignment is incorrect and in fact we don't do that if
> > le_mtu is zero so we probably should do some checks e.g. le_mtu >
> > 0x001a, or perhaps we need to move the MTU directly to hci_conn so it
> > can check there are enough buffers to serve the link so we stop the
> > connection procedure earlier.
>
> Let's say we moved MTU directly to hci_conn and already checked enough
> buffers at the creation of hcon.
> Then, what should happen if hdev->le_mtu is updated? (by a new
> le_read_buffer_size cmd)
> Should hcon->mtu be synced with hdev->le_mtu? Or hcon->mtu can keep
> its old value?
What now, why would we read it again? These commands are only suppose
to be send during init phase and if you do this sort of thing as an
event without a command then it is also against the spec and we shall
probably just ignore it if there is no command pending.
> Best,
> Sungwoo.
--
Luiz Augusto von Dentz
prev parent reply other threads:[~2024-05-01 22:36 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-28 5:43 [PATCH] Bluetooth: L2CAP: Fix div-by-zero in l2cap_le_flowctl_init() Sungwoo Kim
2024-04-29 15:15 ` Luiz Augusto von Dentz
2024-05-01 22:23 ` Sungwoo Kim
2024-05-01 22:36 ` Luiz Augusto von Dentz [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=CABBYNZ+Mr1c7UKa4Vey7Wmh-+WQaFU0P8AifBf7HDEb7G8PTJQ@mail.gmail.com \
--to=luiz.dentz@gmail.com \
--cc=daveti@purdue.edu \
--cc=iam@sung-woo.kim \
--cc=johan.hedberg@gmail.com \
--cc=linux-bluetooth@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).