Hi Elias, On Fri, Jul 17, 2015 at 12:09:37AM +0200, Elias Vanderstuyft wrote: > Hi everyone, > > Making observations based on the following headers: > > uinput.h: "(struct uinput_device).ff_effects_max" is defined as "unsigned int". > uapi/uinput.h: "(struct uinput_user_dev).ff_effects_max" is defined as "__u32". > > vs > > input.h: "(struct ff_device).max_effects" is defined as "int", > however, signature of input_ff_create() in input.h is: > int input_ff_create(struct input_dev *dev, unsigned int max_effects) > > Why is "(struct ff_device).max_effects" defined as a signed integer, > instead of an unsigned integer, as defined in the majority of the headers? The effect->id is signed (because we treat -1 as special value when given to us by userspace) and so it made sense to have max_effects the same type... > > If that question is cleared out, > I would like to point to a potential integer overflow in the assignment in > ff-core.c::input_ff_create(): > ff->max_effects = max_effects; > (http://lxr.free-electrons.com/source/drivers/input/ff-core.c#L337) > Although physically unlikely that max_effects would exceed 0x7FFFFFFF, > shouldn't there be a check to prevent "ff->max_effects" becoming negative? > Note that using UInput, the user can define its own value of max_effects, > so adding a check might be justified. Yes, we should compare against INT_MAX and fail with -EINVAL I guess. Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
