Hi Sungwoo, On Wed, May 1, 2024 at 6:23 PM Sungwoo Kim <iam@xxxxxxxxxxxx> wrote: > > Dear Luiz, > > On Mon, Apr 29, 2024 at 11:15 AM Luiz Augusto von Dentz > <luiz.dentz@xxxxxxxxx> wrote: > > > > Hi Sungwoo, > > > > On Sun, Apr 28, 2024 at 1:43 AM Sungwoo Kim <iam@xxxxxxxxxxxx> 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
