Hi Sakari,
Thanks for working on this!
I've got one comment:
On 05/04/2016 01:20 PM, Sakari Ailus wrote:
> Refactor copying the IOCTL argument structs from the user space and back,
> in order to reduce code copied around and make the implementation more
> robust.
>
> As a result, the copying is done while not holding the graph mutex.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> ---
> drivers/media/media-device.c | 214 ++++++++++++++++++++++---------------------
> 1 file changed, 110 insertions(+), 104 deletions(-)
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 9b5a88d..39fe07f 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
<snip>
> static long __media_device_ioctl(
> struct file *filp, unsigned int cmd, void __user *arg,
> - const struct media_ioctl_info *info_array, unsigned int info_array_len)
> + const struct media_ioctl_info *info_array, unsigned int info_array_len,
> + unsigned int *max_arg_size)
> {
> struct media_devnode *devnode = media_devnode_data(filp);
> struct media_device *dev = to_media_device(devnode);
> const struct media_ioctl_info *info;
> + char karg[media_ioctl_max_arg_size(info_array, info_array_len,
> + max_arg_size)];
This isn't going to work. Sparse (and/or smatch) will complain about this. I recommend
doing the same as videodev does: have a fixed array on the stack, and use kmalloc if
more is needed.
I don't like the max_arg_size anyway :-)
> long ret;
>
> ret = is_valid_ioctl(info_array, info_array_len, cmd);
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
