On 01/29/2018 10:56 AM, Sakari Ailus wrote:
> Hi Hans,
>
> On Fri, Jan 26, 2018 at 01:43:26PM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verk...@cisco.com>
>>
>> Some ioctls need to copy back the result even if the ioctl returned
>> an error. However, don't do this for the error codes -ENOTTY, -EFAULT
>> and -ENOIOCTLCMD. It makes no sense in those cases.
>>
>> Signed-off-by: Hans Verkuil <hans.verk...@cisco.com>
>
> Acked-by: Sakari Ailus <sakari.ai...@linux.intel.com>
>
> Shouldn't such a change be made to video_usercopy() as well? Doesn't need
> to be in this set though.
Good point. I'll add that for v2. This is not actually a bug as such, but
it's just weird to copy back results if the ioctl wasn't implemented at all.
I realize that I need to drop the -EFAULT check: if you call VIDIOC_G_EXT_CTRLS
with an incorrect userspace buffer for the payload, then the control framework
will set error_idx to the index of the control with the wrong buffer. So you do
need to copy back the data in case of -EFAULT.
I can also drop -ENOIOCTLCMD since video_usercopy() converts that to -ENOTTY.
Regards,
Hans
>
>> ---
>> drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
>> b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
>> index 790473b45a21..2aa9b43daf60 100644
>> --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
>> +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
>> @@ -966,6 +966,9 @@ static long do_video_ioctl(struct file *file, unsigned
>> int cmd, unsigned long ar
>> set_fs(old_fs);
>> }
>>
>> + if (err == -ENOTTY || err == -EFAULT || err == -ENOIOCTLCMD)
>> + return err;
>> +
>> /* Special case: even after an error we need to put the
>> results back for these ioctls since the error_idx will
>> contain information on which control failed. */
>
