On Fri, Jul 09, 2004 at 04:34:24PM +0400, Sergey Vlasov wrote:
> On Thu, Jul 08, 2004 at 10:39:24PM -0700, Pete Zaitcev wrote:
> > On Wed, 7 Jul 2004 17:52:01 -0400
> > Adam Kropelin <[EMAIL PROTECTED]> wrote:
> >
> > > /* hiddev_usage_ref_multi is used for sending multiple bytes to a control.
> > > * It really manifests itself as setting the value of consecutive usages */
> > > +#define HID_MAX_MULTI_USAGES 1024
> > > struct hiddev_usage_ref_multi {
> > > struct hiddev_usage_ref uref;
> > > __u32 num_values;
> > > - __s32 values[HID_MAX_USAGES];
> > > + __s32 values[HID_MAX_MULTI_USAGES];
> > > };
> >
> > Woa dude, this is some enormous struct here. I sure hope we do not
> > leave it included into some other struct accidentially.
>
> It is even worse - this monster thing is allocated on the kernel stack in
> hiddev_ioctl() :(
Ewwww. Somebody botched that up but good.
Here is a patch to fix up hiddev_ioctl(), slightly improved over the way
it was done for 2.6. This patch avoids allocating a uref_multi when a
standard uref will do. It also fixes copy_{to,from}_user() return value
handling as was done in 2.6.
--Adam
--- linux-2.4.26/drivers/usb/hiddev.c.orig Fri Jul 9 14:54:17 2004
+++ linux-2.4.26/drivers/usb/hiddev.c Fri Jul 9 15:06:01 2004
@@ -396,8 +396,8 @@
struct hiddev_collection_info cinfo;
struct hiddev_report_info rinfo;
struct hiddev_field_info finfo;
- struct hiddev_usage_ref_multi uref_multi;
- struct hiddev_usage_ref *uref = &uref_multi.uref;
+ struct hiddev_usage_ref_multi *uref_multi;
+ struct hiddev_usage_ref *uref;
struct hiddev_devinfo dinfo;
struct hid_report *report;
struct hid_field *field;
@@ -555,96 +555,126 @@
return copy_to_user((void *) arg, &finfo, sizeof(finfo));
case HIDIOCGUCODE:
+ uref = kmalloc(sizeof(*uref), GFP_KERNEL);
+ if (!uref)
+ return -ENOMEM;
if (copy_from_user(uref, (void *) arg, sizeof(*uref)))
- return -EFAULT;
+ goto uref_fault;
rinfo.report_type = uref->report_type;
rinfo.report_id = uref->report_id;
if ((report = hiddev_lookup_report(hid, &rinfo)) == NULL)
- return -EINVAL;
+ goto uref_inval;
if (uref->field_index >= report->maxfield)
- return -EINVAL;
+ goto uref_inval;
field = report->field[uref->field_index];
if (uref->usage_index >= field->maxusage)
- return -EINVAL;
+ goto uref_inval;
uref->usage_code = field->usage[uref->usage_index].hid;
- return copy_to_user((void *) arg, uref, sizeof(*uref));
+ if (copy_to_user((void *) arg, uref, sizeof(*uref)))
+ goto uref_fault;
+
+ kfree(uref);
+ return 0;
+uref_fault:
+ kfree(uref);
+ return -EFAULT;
+uref_inval:
+ kfree(uref);
+ return -EINVAL;
case HIDIOCGUSAGE:
case HIDIOCSUSAGE:
case HIDIOCGUSAGES:
case HIDIOCSUSAGES:
case HIDIOCGCOLLECTIONINDEX:
+ uref_multi = kmalloc(sizeof(*uref_multi), GFP_KERNEL);
+ if (!uref_multi)
+ return -ENOMEM;
+ uref = &uref_multi->uref;
if (cmd == HIDIOCGUSAGES || cmd == HIDIOCSUSAGES) {
- if (copy_from_user(&uref_multi, (void *) arg,
- sizeof(uref_multi)))
- return -EFAULT;
+ if (copy_from_user(uref_multi, (void *) arg,
+ sizeof(*uref_multi)))
+ goto fault;
} else {
if (copy_from_user(uref, (void *) arg, sizeof(*uref)))
- return -EFAULT;
+ goto fault;
}
if (cmd != HIDIOCGUSAGE &&
cmd != HIDIOCGUSAGES &&
uref->report_type == HID_REPORT_TYPE_INPUT)
- return -EINVAL;
+ goto inval;
if (uref->report_id == HID_REPORT_ID_UNKNOWN) {
field = hiddev_lookup_usage(hid, uref);
if (field == NULL)
- return -EINVAL;
+ goto inval;
} else {
rinfo.report_type = uref->report_type;
rinfo.report_id = uref->report_id;
if ((report = hiddev_lookup_report(hid, &rinfo)) == NULL)
- return -EINVAL;
+ goto inval;
if (uref->field_index >= report->maxfield)
- return -EINVAL;
+ goto inval;
field = report->field[uref->field_index];
if (uref->usage_index >= field->maxusage)
- return -EINVAL;
+ goto inval;
if (cmd == HIDIOCGUSAGES || cmd == HIDIOCSUSAGES) {
- if (uref_multi.num_values >= HID_MAX_USAGES ||
+ if (uref_multi->num_values >= HID_MAX_USAGES ||
uref->usage_index >= field->maxusage ||
- (uref->usage_index + uref_multi.num_values) >=
field->maxusage)
- return -EINVAL;
+ (uref->usage_index + uref_multi->num_values) >=
field->maxusage)
+ goto inval;
}
}
switch (cmd) {
case HIDIOCGUSAGE:
uref->value = field->value[uref->usage_index];
- return copy_to_user((void *) arg, uref, sizeof(*uref));
+ if (copy_to_user((void *) arg, uref, sizeof(*uref)))
+ goto fault;
+ goto goodreturn;
case HIDIOCSUSAGE:
field->value[uref->usage_index] = uref->value;
- return 0;
+ goto goodreturn;
case HIDIOCGCOLLECTIONINDEX:
+ kfree(uref_multi);
return field->usage[uref->usage_index].collection_index;
case HIDIOCGUSAGES:
- for (i = 0; i < uref_multi.num_values; i++)
- uref_multi.values[i] =
+ for (i = 0; i < uref_multi->num_values; i++)
+ uref_multi->values[i] =
field->value[uref->usage_index + i];
- if (copy_to_user((void *) arg, &uref_multi,
- sizeof(uref_multi)))
- return -EFAULT;
- return 0;
+ if (copy_to_user((void *) arg, uref_multi,
+ sizeof(*uref_multi)))
+ goto fault;
+ goto goodreturn;
case HIDIOCSUSAGES:
- for (i = 0; i < uref_multi.num_values; i++)
+ for (i = 0; i < uref_multi->num_values; i++)
field->value[uref->usage_index + i] =
- uref_multi.values[i];
- return 0;
+ uref_multi->values[i];
+ goto goodreturn;
}
break;
+goodreturn:
+ kfree(uref_multi);
+ return 0;
+fault:
+ kfree(uref_multi);
+ return -EFAULT;
+inval:
+ kfree(uref_multi);
+ return -EINVAL;
+
case HIDIOCGCOLLECTIONINFO:
if (copy_from_user(&cinfo, (void *) arg, sizeof(cinfo)))
return -EFAULT;
-------------------------------------------------------
This SF.Net email sponsored by Black Hat Briefings & Training.
Attend Black Hat Briefings & Training, Las Vegas July 24-29 -
digital self defense, top technical experts, no vendor pitches,
unmatched networking opportunities. Visit www.blackhat.com
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
