Am 11.10.2011 23:19, schrieb Dan Carpenter:
> The problem here is that max_effects can wrap on 32 bits systems.
> We'd allocate a smaller amount of data than sizeof(struct ff_device).
> The call to kcalloc() on the next line would fail but it would write
> the NULL return outside of the memory we just allocated causing data
> corruption.
>
> The call path is that uinput_setup_device() get ->ff_effects_max from
> the user and sets the value in the ->private_data struct. From there
> it is:
> -> uinput_ioctl_handler()
> -> uinput_create_device()
> -> input_ff_create(dev, udev->ff_effects_max);
>
> I've also changed ff_effects_max so it's an unsigned int instead of
> a signed int as a cleanup.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> ---
> V2: made max_effects unsigned
>
> diff --git a/drivers/input/ff-core.c b/drivers/input/ff-core.c
> index 3367f76..3051c84 100644
> --- a/drivers/input/ff-core.c
> +++ b/drivers/input/ff-core.c
> @@ -309,7 +309,7 @@ EXPORT_SYMBOL_GPL(input_ff_event);
> * Once ff device is created you need to setup its upload, erase,
> * playback and other handlers before registering input device
> */
> -int input_ff_create(struct input_dev *dev, int max_effects)
> +int input_ff_create(struct input_dev *dev, unsigned int max_effects)
> {
> struct ff_device *ff;
> int i;
> @@ -319,6 +319,10 @@ int input_ff_create(struct input_dev *dev, int max_effects)
> return -EINVAL;
> }
>
> + if (sizeof(struct ff_device) + max_effects * sizeof(struct file *) <
> + max_effects)
> + return -EINVAL;
> +
i am not sure if that is the way to go.
the minimum size you need is sizeof(struct ff_device)+sizeof(struct file *)
(assuming that max_effects>=1). If the input can be outside any useful boundaries
i would go for this:
uint64_t tmp= sizeof(struct ff_device) + max_effects * sizeof(struct file *) ;
if (tmp >= UINT_MAX )
......
Clearly it is better to have max_effects in a proper range.
re,
wh
> ff = kzalloc(sizeof(struct ff_device) +
> max_effects * sizeof(struct file *), GFP_KERNEL);
> if (!ff)
> diff --git a/include/linux/input.h b/include/linux/input.h
> index 57add32..6d5eddb 100644
> --- a/include/linux/input.h
> +++ b/include/linux/input.h
> @@ -1610,7 +1610,7 @@ struct ff_device {
> struct file *effect_owners[];
> };
>
> -int input_ff_create(struct input_dev *dev, int max_effects);
> +int input_ff_create(struct input_dev *dev, unsigned int max_effects);
> void input_ff_destroy(struct input_dev *dev);
>
> int input_ff_event(struct input_dev *dev, unsigned int type, unsigned int code, int value);
> diff --git a/include/linux/uinput.h b/include/linux/uinput.h
> index d28c726..2aa2881 100644
> --- a/include/linux/uinput.h
> +++ b/include/linux/uinput.h
> @@ -68,7 +68,7 @@ struct uinput_device {
> unsigned char head;
> unsigned char tail;
> struct input_event buff[UINPUT_BUFFER_SIZE];
> - int ff_effects_max;
> + unsigned int ff_effects_max;
>
> struct uinput_request *requests[UINPUT_NUM_REQUESTS];
> wait_queue_head_t requests_waitq;
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
