On 08/22/2016 02:55 PM, Hans Verkuil wrote:
> On 08/11/2016 10:29 PM, Sakari Ailus wrote:
>> Maintain a list of supported IOCTL argument sizes and allow only those in
>> the list.
>>
>> As an additional bonus, IOCTL handlers will be able to check whether the
>> caller actually set (using the argument size) the field vs. assigning it
>> to zero. Separate macro can be provided for that.
>>
>> This will be easier for applications as well since there is no longer the
>> problem of setting the reserved fields zero, or at least it is a lesser
>> problem.
>>
>> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
>
> Acked-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
As discussed in v3, I will probably park this patch for now since we don't need
this functionality today.
Regards,
Hans
>
>> ---
>> drivers/media/media-device.c | 56 ++++++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 51 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
>> index 6f565a2..aa37520 100644
>> --- a/drivers/media/media-device.c
>> +++ b/drivers/media/media-device.c
>> @@ -384,22 +384,36 @@ static long copy_arg_to_user(void __user *uarg, void *karg, unsigned int cmd)
>> /* Do acquire the graph mutex */
>> #define MEDIA_IOC_FL_GRAPH_MUTEX BIT(0)
>>
>> -#define MEDIA_IOC_ARG(__cmd, func, fl, from_user, to_user) \
>> +#define MEDIA_IOC_SZ_ARG(__cmd, func, fl, alt_sz, from_user, to_user) \
>> [_IOC_NR(MEDIA_IOC_##__cmd)] = { \
>> .cmd = MEDIA_IOC_##__cmd, \
>> .fn = (long (*)(struct media_device *, void *))func, \
>> .flags = fl, \
>> + .alt_arg_sizes = alt_sz, \
>> .arg_from_user = from_user, \
>> .arg_to_user = to_user, \
>> }
>>
>> -#define MEDIA_IOC(__cmd, func, fl) \
>> - MEDIA_IOC_ARG(__cmd, func, fl, copy_arg_from_user, copy_arg_to_user)
>> +#define MEDIA_IOC_ARG(__cmd, func, fl, from_user, to_user) \
>> + MEDIA_IOC_SZ_ARG(__cmd, func, fl, NULL, from_user, to_user)
>> +
>> +#define MEDIA_IOC_SZ(__cmd, func, fl, alt_sz) \
>> + MEDIA_IOC_SZ_ARG(__cmd, func, fl, alt_sz, \
>> + copy_arg_from_user, copy_arg_to_user)
>> +
>> +#define MEDIA_IOC(__cmd, func, fl) \
>> + MEDIA_IOC_ARG(__cmd, func, fl, \
>> + copy_arg_from_user, copy_arg_to_user)
>>
>> /* the table is indexed by _IOC_NR(cmd) */
>> struct media_ioctl_info {
>> unsigned int cmd;
>> unsigned short flags;
>> + /*
>> + * Sizes of the alternative arguments. If there are none, this
>> + * pointer is NULL.
>> + */
>> + const unsigned short *alt_arg_sizes;
>> long (*fn)(struct media_device *dev, void *arg);
>> long (*arg_from_user)(void *karg, void __user *uarg, unsigned int cmd);
>> long (*arg_to_user)(void __user *uarg, void *karg, unsigned int cmd);
>> @@ -413,11 +427,40 @@ static const struct media_ioctl_info ioctl_info[] = {
>> MEDIA_IOC(G_TOPOLOGY, media_device_get_topology, MEDIA_IOC_FL_GRAPH_MUTEX),
>> };
>>
>> +#define MASK_IOC_SIZE(cmd) \
>> + ((cmd) & ~(_IOC_SIZEMASK << _IOC_SIZESHIFT))
>> +
>> static inline long is_valid_ioctl(const struct media_ioctl_info *info,
>> unsigned int cmd)
>> {
>> - return (_IOC_NR(cmd) >= ARRAY_SIZE(ioctl_info)
>> - || info[_IOC_NR(cmd)].cmd != cmd) ? -ENOIOCTLCMD : 0;
>> + const unsigned short *alt_arg_sizes;
>> +
>> + if (_IOC_NR(cmd) >= ARRAY_SIZE(ioctl_info))
>> + return -ENOIOCTLCMD;
>> +
>> + info += _IOC_NR(cmd);
>> +
>> + if (info->cmd == cmd)
>> + return 0;
>> +
>> + /*
>> + * Verify that the size-dependent patch of the IOCTL command
>> + * matches and that the size does not exceed the principal
>> + * argument size.
>> + */
>> + if (MASK_IOC_SIZE(info->cmd) != MASK_IOC_SIZE(cmd)
>> + || _IOC_SIZE(info->cmd) < _IOC_SIZE(cmd))
>> + return -ENOIOCTLCMD;
>> +
>> + alt_arg_sizes = info->alt_arg_sizes;
>> + if (!alt_arg_sizes)
>> + return -ENOIOCTLCMD;
>> +
>> + for (; *alt_arg_sizes; alt_arg_sizes++)
>> + if (_IOC_SIZE(cmd) == *alt_arg_sizes)
>> + return 0;
>> +
>> + return -ENOIOCTLCMD;
>> }
>>
>> static long __media_device_ioctl(
>> @@ -448,6 +491,9 @@ static long __media_device_ioctl(
>> goto out_free;
>> }
>>
>> + /* Set the rest of the argument struct to zero */
>> + memset(karg + _IOC_SIZE(cmd), 0, _IOC_SIZE(info->cmd) - _IOC_SIZE(cmd));
>> +
>> if (info->flags & MEDIA_IOC_FL_GRAPH_MUTEX)
>> mutex_lock(&dev->graph_mutex);
>>
>>
> --
> 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
>
--
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
