On Wed, May 4, 2016 at 2:44 AM, Peter Hutterer <peter.hutte...@who-t.net> wrote: > On Tue, May 03, 2016 at 03:33:43PM -0700, Jason Gerecke wrote: >> If a device with a malformed USB interface descriptor that indicates >> zero enpoints are available is probed by our driver, the system will > > typo, endpoints > >> crash due to NULL dereferences. This patch implements the fix suggested >> in the mentioned Red Hat bugs. >> >> Ref: CVE-2016-3139 >> Fixes: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1556883 >> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1283375 >> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1283377 >> >> Signed-off-by: Jason Gerecke <jason.gere...@wacom.com> >> --- >> Open question: RHEL has no plans to fix this in their tree -- do we want >> to fix it in input-wacom?
It's a bug fix. We need the patch at least for input-wacom. Since the patch was initially created by Vladis Dronov (https://bugzilla.redhat.com/attachment.cgi?id=1099056&action=diff), shouldn't we get Vladis' s-o-b? > yes. RHEL has its own schedule, don't take it as a reference unless you > *specifically* work on something you intend to get into RHEL only. There are > a variety of other ways how this patch could end up in a future RHEL > version. This patch only affects kernels older than 3.17. If RHEL only backports patches from kernel.org, the fix won't be in kernel 3.10 unless we submit a patch for those longterm stable versions. >> 2.6.30/wacom_sys.c | 6 ++++++ >> 2.6.36/wacom_sys.c | 6 ++++++ >> 2.6.38/wacom_sys.c | 6 ++++++ >> 3.7/wacom_sys.c | 6 ++++++ >> 4 files changed, 24 insertions(+) >> >> diff --git a/2.6.30/wacom_sys.c b/2.6.30/wacom_sys.c >> index c12cb02..473292c 100644 >> --- a/2.6.30/wacom_sys.c >> +++ b/2.6.30/wacom_sys.c >> @@ -798,6 +798,12 @@ static int wacom_probe(struct usb_interface *intf, >> const struct usb_device_id *i >> >> wacom_wac->input = input_dev; >> >> + /* Verify that a device really has an endpoint */ >> + if (intf->cur_altsetting->desc.bNumEndpoints < 1) { >> + error = -EINVAL; >> + goto fail3; >> + } >> + I think we want to validate bNumEndpoints before creating other objects. Something like below would get the job done: diff --git a/3.7/wacom_sys.c b/3.7/wacom_sys.c index 3047773..8afeeb2 100644 --- a/3.7/wacom_sys.c +++ b/3.7/wacom_sys.c @@ -1630,6 +1630,10 @@ static int wacom_probe(struct usb_interface *intf, const struct usb_device_id *i if (!id->driver_info) return -EINVAL; + /* Verify that a device really has an endpoint */ + if (intf->cur_altsetting->desc.bNumEndpoints < 1) + return -EINVAL; + wacom = kzalloc(sizeof(struct wacom), GFP_KERNEL); if (!wacom) return -ENOMEM; Either way, you get my Reviewed-by: Ping Cheng <pi...@wacom.com> Cheers, Ping >> endpoint = &intf->cur_altsetting->endpoint[0].desc; >> >> /* Retrieve the physical and logical size for OEM devices */ >> diff --git a/2.6.36/wacom_sys.c b/2.6.36/wacom_sys.c >> index eda6175..1d90a91 100644 >> --- a/2.6.36/wacom_sys.c >> +++ b/2.6.36/wacom_sys.c >> @@ -885,6 +885,12 @@ static int wacom_probe(struct usb_interface *intf, >> const struct usb_device_id *i >> >> wacom_wac->input = input_dev; >> >> + /* Verify that a device really has an endpoint */ >> + if (intf->cur_altsetting->desc.bNumEndpoints < 1) { >> + error = -EINVAL; >> + goto fail3; >> + } >> + >> endpoint = &intf->cur_altsetting->endpoint[0].desc; >> >> /* Retrieve the physical and logical size for touch devices */ >> diff --git a/2.6.38/wacom_sys.c b/2.6.38/wacom_sys.c >> index 7902e21..190e5bc 100644 >> --- a/2.6.38/wacom_sys.c >> +++ b/2.6.38/wacom_sys.c >> @@ -1664,6 +1664,12 @@ static int wacom_probe(struct usb_interface *intf, >> const struct usb_device_id *i >> usb_make_path(dev, wacom->phys, sizeof(wacom->phys)); >> strlcat(wacom->phys, "/input0", sizeof(wacom->phys)); >> >> + /* Verify that a device really has an endpoint */ >> + if (intf->cur_altsetting->desc.bNumEndpoints < 1) { >> + error = -EINVAL; >> + goto fail3; >> + } >> + >> endpoint = &intf->cur_altsetting->endpoint[0].desc; >> >> /* set the default size in case we do not get them from hid */ >> diff --git a/3.7/wacom_sys.c b/3.7/wacom_sys.c >> index 3047773..da086ed 100644 >> --- a/3.7/wacom_sys.c >> +++ b/3.7/wacom_sys.c >> @@ -1662,6 +1662,12 @@ static int wacom_probe(struct usb_interface *intf, >> const struct usb_device_id *i >> usb_make_path(dev, wacom->phys, sizeof(wacom->phys)); >> strlcat(wacom->phys, "/input0", sizeof(wacom->phys)); >> >> + /* Verify that a device really has an endpoint */ >> + if (intf->cur_altsetting->desc.bNumEndpoints < 1) { >> + error = -EINVAL; >> + goto fail3; >> + } >> + >> endpoint = &intf->cur_altsetting->endpoint[0].desc; >> >> /* set the default size in case we do not get them from hid */ >> -- >> 2.8.2 >> >> >> ------------------------------------------------------------------------------ >> Find and fix application performance issues faster with Applications Manager >> Applications Manager provides deep performance insights into multiple tiers >> of >> your business applications. It resolves application problems quickly and >> reduces your MTTR. Get your free trial! >> https://ad.doubleclick.net/ddm/clk/302982198;130105516;z >> _______________________________________________ >> Linuxwacom-devel mailing list >> Linuxwacom-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel >> ------------------------------------------------------------------------------ Find and fix application performance issues faster with Applications Manager Applications Manager provides deep performance insights into multiple tiers of your business applications. It resolves application problems quickly and reduces your MTTR. Get your free trial! https://ad.doubleclick.net/ddm/clk/302982198;130105516;z _______________________________________________ Linuxwacom-devel mailing list Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
