| Message ID | 20210511152807.Bluez.v1.1.I6d2ab6907d9a84fa62ac8a39daef5bef7ff545d5@changeid (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [Bluez,v1] monitor: Fix possible crash of rfcomm packet | expand |
This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=480043 ---Test result--- Test Summary: CheckPatch PASS 0.27 seconds GitLint PASS 0.10 seconds Prep - Setup ELL PASS 39.28 seconds Build - Prep PASS 0.10 seconds Build - Configure PASS 7.20 seconds Build - Make PASS 169.18 seconds Make Check PASS 8.61 seconds Make Dist PASS 11.36 seconds Make Dist - Configure PASS 4.52 seconds Make Dist - Make PASS 70.51 seconds Build w/ext ELL - Configure PASS 7.17 seconds Build w/ext ELL - Make PASS 156.78 seconds Details ############################## Test: CheckPatch - PASS Desc: Run checkpatch.pl script with rule in .checkpatch.conf ############################## Test: GitLint - PASS Desc: Run gitlint with rule in .gitlint ############################## Test: Prep - Setup ELL - PASS Desc: Clone, build, and install ELL ############################## Test: Build - Prep - PASS Desc: Prepare environment for build ############################## Test: Build - Configure - PASS Desc: Configure the BlueZ source tree ############################## Test: Build - Make - PASS Desc: Build the BlueZ source tree ############################## Test: Make Check - PASS Desc: Run 'make check' ############################## Test: Make Dist - PASS Desc: Run 'make dist' and build the distribution tarball ############################## Test: Make Dist - Configure - PASS Desc: Configure the source from distribution tarball ############################## Test: Make Dist - Make - PASS Desc: Build the source from distribution tarball ############################## Test: Build w/ext ELL - Configure - PASS Desc: Configure BlueZ source with '--enable-external-ell' configuration ############################## Test: Build w/ext ELL - Make - PASS Desc: Build BlueZ source with '--enable-external-ell' configuration --- Regards, Linux Bluetooth
Hi Howard, On Tue, May 11, 2021 at 12:28 AM Howard Chung <howardchung@google.com> wrote: > > From: Yun-Hao Chung <howardchung@chromium.org> > > 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@chromium.org > --- > > 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) > 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_*. > if (!l2cap_frame_get_u8(&tmp_frame, &hdr.fcs)) > -- > 2.31.1.607.g51e8a6a459-goog >
On Wed, May 12, 2021 at 2:04 AM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Howard, > > On Tue, May 11, 2021 at 12:28 AM Howard Chung <howardchung@google.com> wrote: > > > > From: Yun-Hao Chung <howardchung@chromium.org> > > > > 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@chromium.org > > --- > > > > 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
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; + l2cap_frame_pull(&tmp_frame, l2cap_frame, l2cap_frame->size-1); if (!l2cap_frame_get_u8(&tmp_frame, &hdr.fcs))