| Message ID | 20220506053740.1113415-1-dzm91@hust.edu.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | HID: bigben: fix slab-out-of-bounds Write in bigben_probe | expand |
On Fri, 6 May 2022, Dongliang Mu wrote: > From: Dongliang Mu <mudongliangabcd@gmail.com> > > There is a slab-out-of-bounds Write bug in hid-bigbenff driver. > The problem is the driver assumes the device must have an input but > some malicious devices violate this assumption. > > Fix this by checking hid_device's input is non-empty before its usage. > > Reported-by: syzkaller <syzkaller@googlegroups.com> > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com> > --- > drivers/hid/hid-bigbenff.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/hid/hid-bigbenff.c b/drivers/hid/hid-bigbenff.c > index 74ad8bf98bfd..c14d1774101d 100644 > --- a/drivers/hid/hid-bigbenff.c > +++ b/drivers/hid/hid-bigbenff.c > @@ -347,6 +347,11 @@ static int bigben_probe(struct hid_device *hid, > bigben->report = list_entry(report_list->next, > struct hid_report, list); > > + if (list_empty(&hid->inputs)) { > + hid_err(hid, "no inputs found\n"); > + return -ENODEV; > + } > + Thanks for the fix. It doesn't seemt o be fully correct though -- as you'd be returning -ENODEV here in the situation when hid_hw_start() has already happened. So I believe better thing to do here is to do error = -ENODEV; goto error_hw_stop; Could you please fix that up and resend? Thanks,
On Fri, May 6, 2022 at 2:53 PM Jiri Kosina <jikos@kernel.org> wrote: > > On Fri, 6 May 2022, Dongliang Mu wrote: > > > From: Dongliang Mu <mudongliangabcd@gmail.com> > > > > There is a slab-out-of-bounds Write bug in hid-bigbenff driver. > > The problem is the driver assumes the device must have an input but > > some malicious devices violate this assumption. > > > > Fix this by checking hid_device's input is non-empty before its usage. > > > > Reported-by: syzkaller <syzkaller@googlegroups.com> > > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com> > > --- > > drivers/hid/hid-bigbenff.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/hid/hid-bigbenff.c b/drivers/hid/hid-bigbenff.c > > index 74ad8bf98bfd..c14d1774101d 100644 > > --- a/drivers/hid/hid-bigbenff.c > > +++ b/drivers/hid/hid-bigbenff.c > > @@ -347,6 +347,11 @@ static int bigben_probe(struct hid_device *hid, > > bigben->report = list_entry(report_list->next, > > struct hid_report, list); > > > > + if (list_empty(&hid->inputs)) { > > + hid_err(hid, "no inputs found\n"); > > + return -ENODEV; > > + } > > + > > Thanks for the fix. It doesn't seemt o be fully correct though -- as you'd > be returning -ENODEV here in the situation when hid_hw_start() has already > happened. So I believe better thing to do here is to do error = -ENODEV; > goto error_hw_stop; > > Could you please fix that up and resend? Thanks, Oh, yes. I will send a v2 version ASAP. > > -- > Jiri Kosina > SUSE Labs >
diff --git a/drivers/hid/hid-bigbenff.c b/drivers/hid/hid-bigbenff.c index 74ad8bf98bfd..c14d1774101d 100644 --- a/drivers/hid/hid-bigbenff.c +++ b/drivers/hid/hid-bigbenff.c @@ -347,6 +347,11 @@ static int bigben_probe(struct hid_device *hid, bigben->report = list_entry(report_list->next, struct hid_report, list); + if (list_empty(&hid->inputs)) { + hid_err(hid, "no inputs found\n"); + return -ENODEV; + } + hidinput = list_first_entry(&hid->inputs, struct hid_input, list); set_bit(FF_RUMBLE, hidinput->input->ffbit);