| Message ID | 1391182129-5234-1-git-send-email-sakari.ailus@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Sakari, Sorry, this isn't right. It should go through v4l2_compat_ioctl32, otherwise ioctls for e.g. extended controls won't be converted correctly. In addition, v4l2_compat_ioctl32 needs to list all the subdev-specific ioctls. I'd have sworn I did that once, but I've no idea what happened to that patch... Regards, Hans On 01/31/2014 04:28 PM, Sakari Ailus wrote: > I thought this was already working but apparently not. Allow 32-bit compat > IOCTLs on 64-bit systems. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > drivers/media/v4l2-core/v4l2-subdev.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > index 996c248..99c54f4 100644 > --- a/drivers/media/v4l2-core/v4l2-subdev.c > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > @@ -389,6 +389,9 @@ const struct v4l2_file_operations v4l2_subdev_fops = { > .owner = THIS_MODULE, > .open = subdev_open, > .unlocked_ioctl = subdev_ioctl, > +#ifdef CONFIG_COMPAT > + .compat_ioctl32 = subdev_ioctl, > +#endif /* CONFIG_COMPAT */ > .release = subdev_close, > .poll = subdev_poll, > }; > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Hans, Thanks for the comments. Hans Verkuil wrote: > Hi Sakari, > > Sorry, this isn't right. > > It should go through v4l2_compat_ioctl32, otherwise ioctls for e.g. extended controls > won't be converted correctly. Now that you mention it, indeed the state back when I thought this was already implemented, the IOCTLs were exactly the same. Now that struct v4l2_subdev_edid is used on VIDIOC_SUBDEV_G_EDID and VIDIOC_SUBDEV_S_EDID32, this no longer holds. The two IOCTLs are already handled by v4l2_compat_ioctl32 explicitly. Perhaps that's what you remember? :-)
On 01/31/2014 04:51 PM, Sakari Ailus wrote: > Hi Hans, > > Thanks for the comments. > > Hans Verkuil wrote: >> Hi Sakari, >> >> Sorry, this isn't right. >> >> It should go through v4l2_compat_ioctl32, otherwise ioctls for e.g. extended controls >> won't be converted correctly. > > Now that you mention it, indeed the state back when I thought this was already implemented, the IOCTLs were exactly the same. Now that struct v4l2_subdev_edid is used on VIDIOC_SUBDEV_G_EDID and VIDIOC_SUBDEV_S_EDID32, this no longer holds. > > The two IOCTLs are already handled by v4l2_compat_ioctl32 explicitly. Perhaps that's what you remember? :-) > No, someone recently mentioned similar problems with compat32 and subdev nodes. I did some work on it then, but I've no idea where it is :-( I did add support for subdev ioctl32 tests to v4l-utils.git recently, so I know I worked on it... It could be on one other test server that I can't access from here, I'll check on Tuesday. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Sakari Ailus wrote: > Hi Hans, > > Thanks for the comments. > > Hans Verkuil wrote: >> Hi Sakari, >> >> Sorry, this isn't right. >> >> It should go through v4l2_compat_ioctl32, otherwise ioctls for e.g. >> extended controls >> won't be converted correctly. > > Now that you mention it, indeed the state back when I thought this was > already implemented, the IOCTLs were exactly the same. Now that struct > v4l2_subdev_edid is used on VIDIOC_SUBDEV_G_EDID and > VIDIOC_SUBDEV_S_EDID32, this no longer holds. Well, indeed, with the patch, the compat_ioctl32 handler wrongly would handle the non-compat IOCTL as well. To fix this properly, the sub-device IOCTL numbers that require no conversion should be added to v4l2_compat_ioctl32() list of IOCTLs. Currently they're not there. Is this what you meant?
On 01/31/2014 05:06 PM, Sakari Ailus wrote: > Sakari Ailus wrote: >> Hi Hans, >> >> Thanks for the comments. >> >> Hans Verkuil wrote: >>> Hi Sakari, >>> >>> Sorry, this isn't right. >>> >>> It should go through v4l2_compat_ioctl32, otherwise ioctls for e.g. >>> extended controls >>> won't be converted correctly. >> >> Now that you mention it, indeed the state back when I thought this was >> already implemented, the IOCTLs were exactly the same. Now that struct >> v4l2_subdev_edid is used on VIDIOC_SUBDEV_G_EDID and >> VIDIOC_SUBDEV_S_EDID32, this no longer holds. > > Well, indeed, with the patch, the compat_ioctl32 handler wrongly would handle the non-compat IOCTL as well. > > To fix this properly, the sub-device IOCTL numbers that require no conversion should be added to v4l2_compat_ioctl32() list of IOCTLs. Currently they're not there. Is this what you meant? > Yes. Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 996c248..99c54f4 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -389,6 +389,9 @@ const struct v4l2_file_operations v4l2_subdev_fops = { .owner = THIS_MODULE, .open = subdev_open, .unlocked_ioctl = subdev_ioctl, +#ifdef CONFIG_COMPAT + .compat_ioctl32 = subdev_ioctl, +#endif /* CONFIG_COMPAT */ .release = subdev_close, .poll = subdev_poll, };
I thought this was already working but apparently not. Allow 32-bit compat IOCTLs on 64-bit systems. Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> --- drivers/media/v4l2-core/v4l2-subdev.c | 3 +++ 1 file changed, 3 insertions(+)