Re: [Bluez PATCH v1] monitor: Fix possible crash of rfcomm packet

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, May 12, 2021 at 2:04 AM Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
>
> Hi Howard,
>
> On Tue, May 11, 2021 at 12:28 AM Howard Chung <howardchung@xxxxxxxxxx> wrote:
> >
> > From: Yun-Hao Chung <howardchung@xxxxxxxxxxxx>
> >
> > When RFCOMM_TEST_EA returns false, btmon assumes packet data has at
> > least 5 bytes long. If that assumption fails, btmon could crash when
> > trying to read the next byte.
> > This patch fix it by checking the remaining size before reading the last
> > byte.
> >
> > Reviewed-by: apusaka@xxxxxxxxxxxx
> > ---
> >
> >  monitor/rfcomm.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c
> > index 9b88a3440e31..76b1123bb23d 100644
> > --- a/monitor/rfcomm.c
> > +++ b/monitor/rfcomm.c
> > @@ -452,6 +452,9 @@ void rfcomm_packet(const struct l2cap_frame *frame)
> >                 hdr.length = GET_LEN16(hdr.length);
> >         }
> >
> > +       if (l2cap_frame->size == 0)
> > +               goto fail;
> > +
>
> if (!l2cap_frame->size)

will do.
>
>
>
> >         l2cap_frame_pull(&tmp_frame, l2cap_frame, l2cap_frame->size-1);
>
> Or perhaps we can make l2cap_frame_pull check if it can really pull
> the frame and return false if it doesn't just as get_*.

IMO, this might not be the best solution. Since |len|l in
2cap_frame_pull is uint16_t, when l2cap_frame->size-1 overflows it
might confuse people.

>
>
>
> >         if (!l2cap_frame_get_u8(&tmp_frame, &hdr.fcs))
> > --
> > 2.31.1.607.g51e8a6a459-goog
> >
>
>
> --
> Luiz Augusto von Dentz



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux