* [PATCH RESEND 0/6] Remaining COMPILE_TEST and smatch cleanups
@ 2018-04-19 11:15 Mauro Carvalho Chehab
2018-04-19 11:15 ` [PATCH RESEND 1/6] omap: omap-iommu.h: allow building drivers with COMPILE_TEST Mauro Carvalho Chehab
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: Mauro Carvalho Chehab @ 2018-04-19 11:15 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab, Daniel Mentz, Sakari Ailus, Hans Verkuil,
Hans Verkuil, Laurent Pinchart, Arnd Bergmann, Jacob Chen,
Ramesh Shanmugasundaram, Stanimir Varbanov, Philipp Zabel
I'm re-sending this patch series, because I forgot to add a C/C on the first
patch of this series. Also, the last patch was sent in separate. So, better
to repost this series.
-
There were several interactions at the COMPILE_TEST and smatch
patch series. While I applied most of them, there are 5 patches that
I kept out of it. The omap3 patch that were in my tree was the old
one. So, I'm re-posting it.
The ioctl32 patches are the latest version. Let's repost it to get some
acks, as this patch touches at V4L2 core, so a careful review is
always a good idea.
Arnd Bergmann (1):
media: omap3isp: Allow it to build with COMPILE_TEST
Laurent Pinchart (1):
media: omap3isp: Enable driver compilation on ARM with COMPILE_TEST
Mauro Carvalho Chehab (4):
omap: omap-iommu.h: allow building drivers with COMPILE_TEST
media: v4l2-compat-ioctl32: fix several __user annotations
media: v4l2-compat-ioctl32: better name userspace pointers
media: v4l2-compat-ioctl32: simplify casts
drivers/media/platform/Kconfig | 7 +-
drivers/media/platform/omap3isp/isp.c | 8 +
drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 641 ++++++++++++++------------
include/linux/omap-iommu.h | 5 +
4 files changed, 354 insertions(+), 307 deletions(-)
--
2.14.3
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH RESEND 1/6] omap: omap-iommu.h: allow building drivers with COMPILE_TEST 2018-04-19 11:15 [PATCH RESEND 0/6] Remaining COMPILE_TEST and smatch cleanups Mauro Carvalho Chehab @ 2018-04-19 11:15 ` Mauro Carvalho Chehab 2018-04-19 11:24 ` Hans Verkuil 2018-04-19 11:15 ` [PATCH RESEND 2/6] media: omap3isp: Enable driver compilation on ARM " Mauro Carvalho Chehab ` (4 subsequent siblings) 5 siblings, 1 reply; 13+ messages in thread From: Mauro Carvalho Chehab @ 2018-04-19 11:15 UTC (permalink / raw) Cc: Mauro Carvalho Chehab, Linux Media Mailing List, Mauro Carvalho Chehab, Sakari Ailus, Tony Lindgren, Arnd Bergmann Drivers that depend on omap-iommu.h (currently, just omap3isp) need a stub implementation in order to be built with COMPILE_TEST. Cc: Tony Lindgren <tony@atomide.com> Cc: Arnd Bergmann <arnd@arndb.de> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> --- include/linux/omap-iommu.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/include/linux/omap-iommu.h b/include/linux/omap-iommu.h index c1aede46718b..ce1b7c6283ee 100644 --- a/include/linux/omap-iommu.h +++ b/include/linux/omap-iommu.h @@ -13,7 +13,12 @@ #ifndef _OMAP_IOMMU_H_ #define _OMAP_IOMMU_H_ +#ifdef CONFIG_OMAP_IOMMU extern void omap_iommu_save_ctx(struct device *dev); extern void omap_iommu_restore_ctx(struct device *dev); +#else +static inline void omap_iommu_save_ctx(struct device *dev) {} +static inline void omap_iommu_restore_ctx(struct device *dev) {} +#endif #endif -- 2.14.3 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH RESEND 1/6] omap: omap-iommu.h: allow building drivers with COMPILE_TEST 2018-04-19 11:15 ` [PATCH RESEND 1/6] omap: omap-iommu.h: allow building drivers with COMPILE_TEST Mauro Carvalho Chehab @ 2018-04-19 11:24 ` Hans Verkuil 0 siblings, 0 replies; 13+ messages in thread From: Hans Verkuil @ 2018-04-19 11:24 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Sakari Ailus, Tony Lindgren, Arnd Bergmann On 04/19/18 13:15, Mauro Carvalho Chehab wrote: > Drivers that depend on omap-iommu.h (currently, just omap3isp) > need a stub implementation in order to be built with COMPILE_TEST. > > Cc: Tony Lindgren <tony@atomide.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> Acked-by: Hans Verkuil <hans.verkuil@cisco.com> Regards, Hans > --- > include/linux/omap-iommu.h | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/include/linux/omap-iommu.h b/include/linux/omap-iommu.h > index c1aede46718b..ce1b7c6283ee 100644 > --- a/include/linux/omap-iommu.h > +++ b/include/linux/omap-iommu.h > @@ -13,7 +13,12 @@ > #ifndef _OMAP_IOMMU_H_ > #define _OMAP_IOMMU_H_ > > +#ifdef CONFIG_OMAP_IOMMU > extern void omap_iommu_save_ctx(struct device *dev); > extern void omap_iommu_restore_ctx(struct device *dev); > +#else > +static inline void omap_iommu_save_ctx(struct device *dev) {} > +static inline void omap_iommu_restore_ctx(struct device *dev) {} > +#endif > > #endif > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH RESEND 2/6] media: omap3isp: Enable driver compilation on ARM with COMPILE_TEST 2018-04-19 11:15 [PATCH RESEND 0/6] Remaining COMPILE_TEST and smatch cleanups Mauro Carvalho Chehab 2018-04-19 11:15 ` [PATCH RESEND 1/6] omap: omap-iommu.h: allow building drivers with COMPILE_TEST Mauro Carvalho Chehab @ 2018-04-19 11:15 ` Mauro Carvalho Chehab 2018-04-19 11:25 ` Hans Verkuil 2018-04-19 11:15 ` [PATCH RESEND 3/6] media: omap3isp: Allow it to build " Mauro Carvalho Chehab ` (3 subsequent siblings) 5 siblings, 1 reply; 13+ messages in thread From: Mauro Carvalho Chehab @ 2018-04-19 11:15 UTC (permalink / raw) Cc: Laurent Pinchart, Linux Media Mailing List, Mauro Carvalho Chehab, Hans Verkuil, Arnd Bergmann, Stanimir Varbanov, Sakari Ailus, Philipp Zabel, Ramesh Shanmugasundaram, Jacob Chen, Mauro Carvalho Chehab From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> The omap3isp driver can't be compiled on non-ARM platforms but has no compile-time dependency on OMAP. It however requires common clock framework support, which isn't provided by all ARM platforms. Drop the OMAP dependency when COMPILE_TEST is set and add ARM and COMMON_CLK dependencies. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> --- drivers/media/platform/Kconfig | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index 91b0c7324afb..1ee915b794c0 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -62,7 +62,10 @@ config VIDEO_MUX config VIDEO_OMAP3 tristate "OMAP 3 Camera support" - depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3 + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API + depends on ARCH_OMAP3 || COMPILE_TEST + depends on ARM + depends on COMMON_CLK depends on HAS_DMA && OF depends on OMAP_IOMMU select ARM_DMA_USE_IOMMU -- 2.14.3 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH RESEND 2/6] media: omap3isp: Enable driver compilation on ARM with COMPILE_TEST 2018-04-19 11:15 ` [PATCH RESEND 2/6] media: omap3isp: Enable driver compilation on ARM " Mauro Carvalho Chehab @ 2018-04-19 11:25 ` Hans Verkuil 0 siblings, 0 replies; 13+ messages in thread From: Hans Verkuil @ 2018-04-19 11:25 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Laurent Pinchart, Linux Media Mailing List, Mauro Carvalho Chehab, Hans Verkuil, Arnd Bergmann, Stanimir Varbanov, Sakari Ailus, Philipp Zabel, Ramesh Shanmugasundaram, Jacob Chen On 04/19/18 13:15, Mauro Carvalho Chehab wrote: > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > The omap3isp driver can't be compiled on non-ARM platforms but has no > compile-time dependency on OMAP. It however requires common clock > framework support, which isn't provided by all ARM platforms. > > Drop the OMAP dependency when COMPILE_TEST is set and add ARM and > COMMON_CLK dependencies. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> Acked-by: Hans Verkuil <hans.verkuil@cisco.com> Regards, Hans > --- > drivers/media/platform/Kconfig | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig > index 91b0c7324afb..1ee915b794c0 100644 > --- a/drivers/media/platform/Kconfig > +++ b/drivers/media/platform/Kconfig > @@ -62,7 +62,10 @@ config VIDEO_MUX > > config VIDEO_OMAP3 > tristate "OMAP 3 Camera support" > - depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3 > + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API > + depends on ARCH_OMAP3 || COMPILE_TEST > + depends on ARM > + depends on COMMON_CLK > depends on HAS_DMA && OF > depends on OMAP_IOMMU > select ARM_DMA_USE_IOMMU > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH RESEND 3/6] media: omap3isp: Allow it to build with COMPILE_TEST 2018-04-19 11:15 [PATCH RESEND 0/6] Remaining COMPILE_TEST and smatch cleanups Mauro Carvalho Chehab 2018-04-19 11:15 ` [PATCH RESEND 1/6] omap: omap-iommu.h: allow building drivers with COMPILE_TEST Mauro Carvalho Chehab 2018-04-19 11:15 ` [PATCH RESEND 2/6] media: omap3isp: Enable driver compilation on ARM " Mauro Carvalho Chehab @ 2018-04-19 11:15 ` Mauro Carvalho Chehab 2018-04-19 11:27 ` Hans Verkuil 2018-04-19 11:15 ` [PATCH RESEND 4/6] media: v4l2-compat-ioctl32: fix several __user annotations Mauro Carvalho Chehab ` (2 subsequent siblings) 5 siblings, 1 reply; 13+ messages in thread From: Mauro Carvalho Chehab @ 2018-04-19 11:15 UTC (permalink / raw) Cc: Arnd Bergmann, Linux Media Mailing List, Mauro Carvalho Chehab, Laurent Pinchart, Hans Verkuil, Stanimir Varbanov, Sakari Ailus, Philipp Zabel, Ramesh Shanmugasundaram, Jacob Chen, Mauro Carvalho Chehab From: Arnd Bergmann <arnd@arndb.de> There aren't much things required for it to build with COMPILE_TEST. It just needs to not compile the code that depends on arm-specific iommu implementation. Signed-off-by: Arnd Bergmann <arnd@arndb.de> Co-developed-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> --- drivers/media/platform/Kconfig | 6 ++---- drivers/media/platform/omap3isp/isp.c | 8 ++++++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index 1ee915b794c0..e3229f7baed1 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -63,12 +63,10 @@ config VIDEO_MUX config VIDEO_OMAP3 tristate "OMAP 3 Camera support" depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API - depends on ARCH_OMAP3 || COMPILE_TEST - depends on ARM + depends on (ARCH_OMAP3 && OMAP_IOMMU) || COMPILE_TEST depends on COMMON_CLK depends on HAS_DMA && OF - depends on OMAP_IOMMU - select ARM_DMA_USE_IOMMU + select ARM_DMA_USE_IOMMU if OMAP_IOMMU select VIDEOBUF2_DMA_CONTIG select MFD_SYSCON select V4L2_FWNODE diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c index 16c50099cccd..b8c8761a76b6 100644 --- a/drivers/media/platform/omap3isp/isp.c +++ b/drivers/media/platform/omap3isp/isp.c @@ -61,7 +61,9 @@ #include <linux/sched.h> #include <linux/vmalloc.h> +#ifdef CONFIG_ARM_DMA_USE_IOMMU #include <asm/dma-iommu.h> +#endif #include <media/v4l2-common.h> #include <media/v4l2-fwnode.h> @@ -1938,12 +1940,15 @@ static int isp_initialize_modules(struct isp_device *isp) static void isp_detach_iommu(struct isp_device *isp) { +#ifdef CONFIG_ARM_DMA_USE_IOMMU arm_iommu_release_mapping(isp->mapping); isp->mapping = NULL; +#endif } static int isp_attach_iommu(struct isp_device *isp) { +#ifdef CONFIG_ARM_DMA_USE_IOMMU struct dma_iommu_mapping *mapping; int ret; @@ -1972,6 +1977,9 @@ static int isp_attach_iommu(struct isp_device *isp) error: isp_detach_iommu(isp); return ret; +#else + return -ENODEV; +#endif } /* -- 2.14.3 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH RESEND 3/6] media: omap3isp: Allow it to build with COMPILE_TEST 2018-04-19 11:15 ` [PATCH RESEND 3/6] media: omap3isp: Allow it to build " Mauro Carvalho Chehab @ 2018-04-19 11:27 ` Hans Verkuil 0 siblings, 0 replies; 13+ messages in thread From: Hans Verkuil @ 2018-04-19 11:27 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Arnd Bergmann, Linux Media Mailing List, Mauro Carvalho Chehab, Laurent Pinchart, Hans Verkuil, Stanimir Varbanov, Sakari Ailus, Philipp Zabel, Ramesh Shanmugasundaram, Jacob Chen On 04/19/18 13:15, Mauro Carvalho Chehab wrote: > From: Arnd Bergmann <arnd@arndb.de> > > There aren't much things required for it to build with COMPILE_TEST. > It just needs to not compile the code that depends on arm-specific > iommu implementation. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > Co-developed-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> Acked-by: Hans Verkuil <hans.verkuil@cisco.com> Regards, Hans > --- > drivers/media/platform/Kconfig | 6 ++---- > drivers/media/platform/omap3isp/isp.c | 8 ++++++++ > 2 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig > index 1ee915b794c0..e3229f7baed1 100644 > --- a/drivers/media/platform/Kconfig > +++ b/drivers/media/platform/Kconfig > @@ -63,12 +63,10 @@ config VIDEO_MUX > config VIDEO_OMAP3 > tristate "OMAP 3 Camera support" > depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API > - depends on ARCH_OMAP3 || COMPILE_TEST > - depends on ARM > + depends on (ARCH_OMAP3 && OMAP_IOMMU) || COMPILE_TEST > depends on COMMON_CLK > depends on HAS_DMA && OF > - depends on OMAP_IOMMU > - select ARM_DMA_USE_IOMMU > + select ARM_DMA_USE_IOMMU if OMAP_IOMMU > select VIDEOBUF2_DMA_CONTIG > select MFD_SYSCON > select V4L2_FWNODE > diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c > index 16c50099cccd..b8c8761a76b6 100644 > --- a/drivers/media/platform/omap3isp/isp.c > +++ b/drivers/media/platform/omap3isp/isp.c > @@ -61,7 +61,9 @@ > #include <linux/sched.h> > #include <linux/vmalloc.h> > > +#ifdef CONFIG_ARM_DMA_USE_IOMMU > #include <asm/dma-iommu.h> > +#endif > > #include <media/v4l2-common.h> > #include <media/v4l2-fwnode.h> > @@ -1938,12 +1940,15 @@ static int isp_initialize_modules(struct isp_device *isp) > > static void isp_detach_iommu(struct isp_device *isp) > { > +#ifdef CONFIG_ARM_DMA_USE_IOMMU > arm_iommu_release_mapping(isp->mapping); > isp->mapping = NULL; > +#endif > } > > static int isp_attach_iommu(struct isp_device *isp) > { > +#ifdef CONFIG_ARM_DMA_USE_IOMMU > struct dma_iommu_mapping *mapping; > int ret; > > @@ -1972,6 +1977,9 @@ static int isp_attach_iommu(struct isp_device *isp) > error: > isp_detach_iommu(isp); > return ret; > +#else > + return -ENODEV; > +#endif > } > > /* > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH RESEND 4/6] media: v4l2-compat-ioctl32: fix several __user annotations 2018-04-19 11:15 [PATCH RESEND 0/6] Remaining COMPILE_TEST and smatch cleanups Mauro Carvalho Chehab ` (2 preceding siblings ...) 2018-04-19 11:15 ` [PATCH RESEND 3/6] media: omap3isp: Allow it to build " Mauro Carvalho Chehab @ 2018-04-19 11:15 ` Mauro Carvalho Chehab 2018-04-19 11:32 ` Hans Verkuil 2018-04-19 11:15 ` [PATCH RESEND 5/6] media: v4l2-compat-ioctl32: better name userspace pointers Mauro Carvalho Chehab 2018-04-19 11:15 ` [PATCH RESEND 6/6] media: v4l2-compat-ioctl32: simplify casts Mauro Carvalho Chehab 5 siblings, 1 reply; 13+ messages in thread From: Mauro Carvalho Chehab @ 2018-04-19 11:15 UTC (permalink / raw) Cc: Mauro Carvalho Chehab, Linux Media Mailing List, Mauro Carvalho Chehab, Hans Verkuil, Sakari Ailus, Daniel Mentz Smatch report several issues with bad __user annotations: drivers/media/v4l2-core/v4l2-compat-ioctl32.c:447:21: warning: incorrect type in argument 1 (different address spaces) drivers/media/v4l2-core/v4l2-compat-ioctl32.c:447:21: expected void [noderef] <asn:1>*uptr drivers/media/v4l2-core/v4l2-compat-ioctl32.c:447:21: got void *<noident> drivers/media/v4l2-core/v4l2-compat-ioctl32.c:621:21: warning: incorrect type in argument 1 (different address spaces) drivers/media/v4l2-core/v4l2-compat-ioctl32.c:621:21: expected void const volatile [noderef] <asn:1>*<noident> drivers/media/v4l2-core/v4l2-compat-ioctl32.c:621:21: got struct v4l2_plane [noderef] <asn:1>**<noident> drivers/media/v4l2-core/v4l2-compat-ioctl32.c:693:13: warning: incorrect type in argument 1 (different address spaces) drivers/media/v4l2-core/v4l2-compat-ioctl32.c:693:13: expected void [noderef] <asn:1>*uptr drivers/media/v4l2-core/v4l2-compat-ioctl32.c:693:13: got void *[assigned] base drivers/media/v4l2-core/v4l2-compat-ioctl32.c:871:13: warning: incorrect type in assignment (different address spaces) drivers/media/v4l2-core/v4l2-compat-ioctl32.c:871:13: expected struct v4l2_ext_control [noderef] <asn:1>*kcontrols drivers/media/v4l2-core/v4l2-compat-ioctl32.c:871:13: got struct v4l2_ext_control *<noident> drivers/media/v4l2-core/v4l2-compat-ioctl32.c:957:13: warning: incorrect type in assignment (different address spaces) drivers/media/v4l2-core/v4l2-compat-ioctl32.c:957:13: expected unsigned char [usertype] *__pu_val drivers/media/v4l2-core/v4l2-compat-ioctl32.c:957:13: got void [noderef] <asn:1>* drivers/media/v4l2-core/v4l2-compat-ioctl32.c:973:13: warning: incorrect type in argument 1 (different address spaces) drivers/media/v4l2-core/v4l2-compat-ioctl32.c:973:13: expected void [noderef] <asn:1>*uptr drivers/media/v4l2-core/v4l2-compat-ioctl32.c:973:13: got void *[assigned] edid Fix them. Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> --- drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 51 ++++++++++++++++++--------- 1 file changed, 35 insertions(+), 16 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c index d03a44d89649..c951ac3faf46 100644 --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c @@ -443,8 +443,8 @@ static int put_v4l2_plane32(struct v4l2_plane __user *up, return -EFAULT; break; case V4L2_MEMORY_USERPTR: - if (get_user(p, &up->m.userptr) || - put_user((compat_ulong_t)ptr_to_compat((__force void *)p), + if (get_user(p, &up->m.userptr)|| + put_user((compat_ulong_t)ptr_to_compat((void __user *)p), &up32->m.userptr)) return -EFAULT; break; @@ -587,7 +587,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer __user *kp, u32 length; enum v4l2_memory memory; struct v4l2_plane32 __user *uplane32; - struct v4l2_plane __user *uplane; + struct v4l2_plane *uplane; compat_caddr_t p; int ret; @@ -617,15 +617,22 @@ static int put_v4l2_buffer32(struct v4l2_buffer __user *kp, if (num_planes == 0) return 0; - - if (get_user(uplane, ((__force struct v4l2_plane __user **)&kp->m.planes))) + /* We need to define uplane without __user, even though + * it does point to data in userspace here. The reason is + * that v4l2-ioctl.c copies it from userspace to kernelspace, + * so its definition in videodev2.h doesn't have a + * __user markup. Defining uplane with __user causes + * smatch warnings, so instead declare it without __user + * and cast it as a userspace pointer to put_v4l2_plane32(). + */ + if (get_user(uplane, &kp->m.planes)) return -EFAULT; if (get_user(p, &up->m.planes)) return -EFAULT; uplane32 = compat_ptr(p); while (num_planes--) { - ret = put_v4l2_plane32(uplane, uplane32, memory); + ret = put_v4l2_plane32((void __user *)uplane, uplane32, memory); if (ret) return ret; ++uplane; @@ -675,7 +682,7 @@ static int get_v4l2_framebuffer32(struct v4l2_framebuffer __user *kp, if (!access_ok(VERIFY_READ, up, sizeof(*up)) || get_user(tmp, &up->base) || - put_user((__force void *)compat_ptr(tmp), &kp->base) || + put_user((void __force *)compat_ptr(tmp), &kp->base) || assign_in_user(&kp->capability, &up->capability) || assign_in_user(&kp->flags, &up->flags) || copy_in_user(&kp->fmt, &up->fmt, sizeof(kp->fmt))) @@ -690,7 +697,7 @@ static int put_v4l2_framebuffer32(struct v4l2_framebuffer __user *kp, if (!access_ok(VERIFY_WRITE, up, sizeof(*up)) || get_user(base, &kp->base) || - put_user(ptr_to_compat(base), &up->base) || + put_user(ptr_to_compat((void __user *)base), &up->base) || assign_in_user(&up->capability, &kp->capability) || assign_in_user(&up->flags, &kp->flags) || copy_in_user(&up->fmt, &kp->fmt, sizeof(kp->fmt))) @@ -857,11 +864,19 @@ static int put_v4l2_ext_controls32(struct file *file, struct v4l2_ext_controls32 __user *up) { struct v4l2_ext_control32 __user *ucontrols; - struct v4l2_ext_control __user *kcontrols; + struct v4l2_ext_control *kcontrols; u32 count; u32 n; compat_caddr_t p; + /* + * We need to define kcontrols without __user, even though it does + * point to data in userspace here. The reason is that v4l2-ioctl.c + * copies it from userspace to kernelspace, so its definition in + * videodev2.h doesn't have a __user markup. Defining kcontrols + * with __user causes smatch warnings, so instead declare it + * without __user and cast it as a userspace pointer where needed. + */ if (!access_ok(VERIFY_WRITE, up, sizeof(*up)) || assign_in_user(&up->which, &kp->which) || get_user(count, &kp->count) || @@ -883,10 +898,12 @@ static int put_v4l2_ext_controls32(struct file *file, unsigned int size = sizeof(*ucontrols); u32 id; - if (get_user(id, &kcontrols->id) || + if (get_user(id, (unsigned int __user *)&kcontrols->id) || put_user(id, &ucontrols->id) || - assign_in_user(&ucontrols->size, &kcontrols->size) || - copy_in_user(&ucontrols->reserved2, &kcontrols->reserved2, + assign_in_user(&ucontrols->size, + (unsigned int __user *)&kcontrols->size) || + copy_in_user(&ucontrols->reserved2, + (unsigned int __user *)&kcontrols->reserved2, sizeof(ucontrols->reserved2))) return -EFAULT; @@ -898,7 +915,8 @@ static int put_v4l2_ext_controls32(struct file *file, if (ctrl_is_pointer(file, id)) size -= sizeof(ucontrols->value64); - if (copy_in_user(ucontrols, kcontrols, size)) + if (copy_in_user(ucontrols, + (unsigned int __user *)kcontrols, size)) return -EFAULT; ucontrols++; @@ -952,9 +970,10 @@ static int get_v4l2_edid32(struct v4l2_edid __user *kp, if (!access_ok(VERIFY_READ, up, sizeof(*up)) || assign_in_user(&kp->pad, &up->pad) || assign_in_user(&kp->start_block, &up->start_block) || - assign_in_user(&kp->blocks, &up->blocks) || + assign_in_user(&kp->blocks, + (unsigned char __user *)&up->blocks) || get_user(tmp, &up->edid) || - put_user(compat_ptr(tmp), &kp->edid) || + put_user((void __force *)compat_ptr(tmp), &kp->edid) || copy_in_user(kp->reserved, up->reserved, sizeof(kp->reserved))) return -EFAULT; return 0; @@ -970,7 +989,7 @@ static int put_v4l2_edid32(struct v4l2_edid __user *kp, assign_in_user(&up->start_block, &kp->start_block) || assign_in_user(&up->blocks, &kp->blocks) || get_user(edid, &kp->edid) || - put_user(ptr_to_compat(edid), &up->edid) || + put_user(ptr_to_compat((void __user *)edid), &up->edid) || copy_in_user(up->reserved, kp->reserved, sizeof(up->reserved))) return -EFAULT; return 0; -- 2.14.3 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH RESEND 4/6] media: v4l2-compat-ioctl32: fix several __user annotations 2018-04-19 11:15 ` [PATCH RESEND 4/6] media: v4l2-compat-ioctl32: fix several __user annotations Mauro Carvalho Chehab @ 2018-04-19 11:32 ` Hans Verkuil 0 siblings, 0 replies; 13+ messages in thread From: Hans Verkuil @ 2018-04-19 11:32 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Hans Verkuil, Sakari Ailus, Daniel Mentz On 04/19/18 13:15, Mauro Carvalho Chehab wrote: > Smatch report several issues with bad __user annotations: > > drivers/media/v4l2-core/v4l2-compat-ioctl32.c:447:21: warning: incorrect type in argument 1 (different address spaces) > drivers/media/v4l2-core/v4l2-compat-ioctl32.c:447:21: expected void [noderef] <asn:1>*uptr > drivers/media/v4l2-core/v4l2-compat-ioctl32.c:447:21: got void *<noident> > drivers/media/v4l2-core/v4l2-compat-ioctl32.c:621:21: warning: incorrect type in argument 1 (different address spaces) > drivers/media/v4l2-core/v4l2-compat-ioctl32.c:621:21: expected void const volatile [noderef] <asn:1>*<noident> > drivers/media/v4l2-core/v4l2-compat-ioctl32.c:621:21: got struct v4l2_plane [noderef] <asn:1>**<noident> > drivers/media/v4l2-core/v4l2-compat-ioctl32.c:693:13: warning: incorrect type in argument 1 (different address spaces) > drivers/media/v4l2-core/v4l2-compat-ioctl32.c:693:13: expected void [noderef] <asn:1>*uptr > drivers/media/v4l2-core/v4l2-compat-ioctl32.c:693:13: got void *[assigned] base > drivers/media/v4l2-core/v4l2-compat-ioctl32.c:871:13: warning: incorrect type in assignment (different address spaces) > drivers/media/v4l2-core/v4l2-compat-ioctl32.c:871:13: expected struct v4l2_ext_control [noderef] <asn:1>*kcontrols > drivers/media/v4l2-core/v4l2-compat-ioctl32.c:871:13: got struct v4l2_ext_control *<noident> > drivers/media/v4l2-core/v4l2-compat-ioctl32.c:957:13: warning: incorrect type in assignment (different address spaces) > drivers/media/v4l2-core/v4l2-compat-ioctl32.c:957:13: expected unsigned char [usertype] *__pu_val > drivers/media/v4l2-core/v4l2-compat-ioctl32.c:957:13: got void [noderef] <asn:1>* > drivers/media/v4l2-core/v4l2-compat-ioctl32.c:973:13: warning: incorrect type in argument 1 (different address spaces) > drivers/media/v4l2-core/v4l2-compat-ioctl32.c:973:13: expected void [noderef] <asn:1>*uptr > drivers/media/v4l2-core/v4l2-compat-ioctl32.c:973:13: got void *[assigned] edid > > Fix them. > > Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> > --- > drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 51 ++++++++++++++++++--------- > 1 file changed, 35 insertions(+), 16 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > index d03a44d89649..c951ac3faf46 100644 > --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > @@ -443,8 +443,8 @@ static int put_v4l2_plane32(struct v4l2_plane __user *up, > return -EFAULT; > break; > case V4L2_MEMORY_USERPTR: > - if (get_user(p, &up->m.userptr) || > - put_user((compat_ulong_t)ptr_to_compat((__force void *)p), > + if (get_user(p, &up->m.userptr)|| > + put_user((compat_ulong_t)ptr_to_compat((void __user *)p), > &up32->m.userptr)) > return -EFAULT; > break; > @@ -587,7 +587,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer __user *kp, > u32 length; > enum v4l2_memory memory; > struct v4l2_plane32 __user *uplane32; > - struct v4l2_plane __user *uplane; > + struct v4l2_plane *uplane; > compat_caddr_t p; > int ret; > > @@ -617,15 +617,22 @@ static int put_v4l2_buffer32(struct v4l2_buffer __user *kp, > > if (num_planes == 0) > return 0; > - > - if (get_user(uplane, ((__force struct v4l2_plane __user **)&kp->m.planes))) > + /* We need to define uplane without __user, even though > + * it does point to data in userspace here. The reason is > + * that v4l2-ioctl.c copies it from userspace to kernelspace, > + * so its definition in videodev2.h doesn't have a > + * __user markup. Defining uplane with __user causes > + * smatch warnings, so instead declare it without __user > + * and cast it as a userspace pointer to put_v4l2_plane32(). > + */ > + if (get_user(uplane, &kp->m.planes)) > return -EFAULT; > if (get_user(p, &up->m.planes)) > return -EFAULT; > uplane32 = compat_ptr(p); > > while (num_planes--) { > - ret = put_v4l2_plane32(uplane, uplane32, memory); > + ret = put_v4l2_plane32((void __user *)uplane, uplane32, memory); > if (ret) > return ret; > ++uplane; > @@ -675,7 +682,7 @@ static int get_v4l2_framebuffer32(struct v4l2_framebuffer __user *kp, > > if (!access_ok(VERIFY_READ, up, sizeof(*up)) || > get_user(tmp, &up->base) || > - put_user((__force void *)compat_ptr(tmp), &kp->base) || > + put_user((void __force *)compat_ptr(tmp), &kp->base) || > assign_in_user(&kp->capability, &up->capability) || > assign_in_user(&kp->flags, &up->flags) || > copy_in_user(&kp->fmt, &up->fmt, sizeof(kp->fmt))) > @@ -690,7 +697,7 @@ static int put_v4l2_framebuffer32(struct v4l2_framebuffer __user *kp, > > if (!access_ok(VERIFY_WRITE, up, sizeof(*up)) || > get_user(base, &kp->base) || > - put_user(ptr_to_compat(base), &up->base) || > + put_user(ptr_to_compat((void __user *)base), &up->base) || > assign_in_user(&up->capability, &kp->capability) || > assign_in_user(&up->flags, &kp->flags) || > copy_in_user(&up->fmt, &kp->fmt, sizeof(kp->fmt))) > @@ -857,11 +864,19 @@ static int put_v4l2_ext_controls32(struct file *file, > struct v4l2_ext_controls32 __user *up) > { > struct v4l2_ext_control32 __user *ucontrols; > - struct v4l2_ext_control __user *kcontrols; > + struct v4l2_ext_control *kcontrols; > u32 count; > u32 n; > compat_caddr_t p; > > + /* > + * We need to define kcontrols without __user, even though it does > + * point to data in userspace here. The reason is that v4l2-ioctl.c > + * copies it from userspace to kernelspace, so its definition in > + * videodev2.h doesn't have a __user markup. Defining kcontrols > + * with __user causes smatch warnings, so instead declare it > + * without __user and cast it as a userspace pointer where needed. > + */ > if (!access_ok(VERIFY_WRITE, up, sizeof(*up)) || > assign_in_user(&up->which, &kp->which) || > get_user(count, &kp->count) || > @@ -883,10 +898,12 @@ static int put_v4l2_ext_controls32(struct file *file, > unsigned int size = sizeof(*ucontrols); > u32 id; > > - if (get_user(id, &kcontrols->id) || > + if (get_user(id, (unsigned int __user *)&kcontrols->id) || > put_user(id, &ucontrols->id) || > - assign_in_user(&ucontrols->size, &kcontrols->size) || > - copy_in_user(&ucontrols->reserved2, &kcontrols->reserved2, > + assign_in_user(&ucontrols->size, > + (unsigned int __user *)&kcontrols->size) || > + copy_in_user(&ucontrols->reserved2, > + (unsigned int __user *)&kcontrols->reserved2, > sizeof(ucontrols->reserved2))) > return -EFAULT; > > @@ -898,7 +915,8 @@ static int put_v4l2_ext_controls32(struct file *file, > if (ctrl_is_pointer(file, id)) > size -= sizeof(ucontrols->value64); > > - if (copy_in_user(ucontrols, kcontrols, size)) > + if (copy_in_user(ucontrols, > + (unsigned int __user *)kcontrols, size)) > return -EFAULT; > > ucontrols++; > @@ -952,9 +970,10 @@ static int get_v4l2_edid32(struct v4l2_edid __user *kp, > if (!access_ok(VERIFY_READ, up, sizeof(*up)) || > assign_in_user(&kp->pad, &up->pad) || > assign_in_user(&kp->start_block, &up->start_block) || > - assign_in_user(&kp->blocks, &up->blocks) || > + assign_in_user(&kp->blocks, > + (unsigned char __user *)&up->blocks) || This must be a (u32 __user *) cast, otherwise this patch will break EDID handling on big-endian systems. I know this is corrected in patch 6, but in the meantime this can cause bisect issues. And besides, it's simply a bug :-) After changing this cast you can add my: Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com> Regards, Hans > get_user(tmp, &up->edid) || > - put_user(compat_ptr(tmp), &kp->edid) || > + put_user((void __force *)compat_ptr(tmp), &kp->edid) || > copy_in_user(kp->reserved, up->reserved, sizeof(kp->reserved))) > return -EFAULT; > return 0; > @@ -970,7 +989,7 @@ static int put_v4l2_edid32(struct v4l2_edid __user *kp, > assign_in_user(&up->start_block, &kp->start_block) || > assign_in_user(&up->blocks, &kp->blocks) || > get_user(edid, &kp->edid) || > - put_user(ptr_to_compat(edid), &up->edid) || > + put_user(ptr_to_compat((void __user *)edid), &up->edid) || > copy_in_user(up->reserved, kp->reserved, sizeof(up->reserved))) > return -EFAULT; > return 0; > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH RESEND 5/6] media: v4l2-compat-ioctl32: better name userspace pointers 2018-04-19 11:15 [PATCH RESEND 0/6] Remaining COMPILE_TEST and smatch cleanups Mauro Carvalho Chehab ` (3 preceding siblings ...) 2018-04-19 11:15 ` [PATCH RESEND 4/6] media: v4l2-compat-ioctl32: fix several __user annotations Mauro Carvalho Chehab @ 2018-04-19 11:15 ` Mauro Carvalho Chehab 2018-04-19 11:15 ` [PATCH RESEND 6/6] media: v4l2-compat-ioctl32: simplify casts Mauro Carvalho Chehab 5 siblings, 0 replies; 13+ messages in thread From: Mauro Carvalho Chehab @ 2018-04-19 11:15 UTC (permalink / raw) Cc: Mauro Carvalho Chehab, Linux Media Mailing List, Mauro Carvalho Chehab, Hans Verkuil, Sakari Ailus, Daniel Mentz In the past, "up" were an acronym for "user pointer" and "kp" for "kernel pointer". However, since a1dfb4c48cc1 ("media: v4l2-compat-ioctl32.c: refactor compat ioctl32 logic"), both are now __user pointers. So, the usage of "kp" is really misleading there. So, rename both to just "p32" and "p64" everywhere it occurs, in order to make peace with this file's namespace. There are two exceptions to "up/kp" nomenclature: at alloc_userspace() and at do_video_ioctl(). There, a new userspace pointer were allocated, in order to store the 64 bits version of the ioctl. Those were called as "up_native", with is, IMHO, an even worse name, as "native" could mislead of being the arguments that were filled from userspace. I almost renamed it to just "p64", but, after thinking more about that, it sounded better to call it as "new_p64", as this makes clearer that this is the data structure that was allocated inside this file in order to be used to pass/retrieve data when calling the 64-bit ready file->f_op->unlocked_ioctl() function. Suggested-by: Sakari Ailus <sakari.ailus@linux.intel.com> Suggested-by: Hans Verkuil <hverkuil@xs4all.nl> Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> --- drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 588 +++++++++++++------------- 1 file changed, 294 insertions(+), 294 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c index c951ac3faf46..8c05dd9660d3 100644 --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c @@ -56,8 +56,8 @@ struct v4l2_window32 { __u8 global_alpha; }; -static int get_v4l2_window32(struct v4l2_window __user *kp, - struct v4l2_window32 __user *up, +static int get_v4l2_window32(struct v4l2_window __user *p64, + struct v4l2_window32 __user *p32, void __user *aux_buf, u32 aux_space) { struct v4l2_clip32 __user *uclips; @@ -65,26 +65,26 @@ static int get_v4l2_window32(struct v4l2_window __user *kp, compat_caddr_t p; u32 clipcount; - if (!access_ok(VERIFY_READ, up, sizeof(*up)) || - copy_in_user(&kp->w, &up->w, sizeof(up->w)) || - assign_in_user(&kp->field, &up->field) || - assign_in_user(&kp->chromakey, &up->chromakey) || - assign_in_user(&kp->global_alpha, &up->global_alpha) || - get_user(clipcount, &up->clipcount) || - put_user(clipcount, &kp->clipcount)) + if (!access_ok(VERIFY_READ, p32, sizeof(*p32)) || + copy_in_user(&p64->w, &p32->w, sizeof(p32->w)) || + assign_in_user(&p64->field, &p32->field) || + assign_in_user(&p64->chromakey, &p32->chromakey) || + assign_in_user(&p64->global_alpha, &p32->global_alpha) || + get_user(clipcount, &p32->clipcount) || + put_user(clipcount, &p64->clipcount)) return -EFAULT; if (clipcount > 2048) return -EINVAL; if (!clipcount) - return put_user(NULL, &kp->clips); + return put_user(NULL, &p64->clips); - if (get_user(p, &up->clips)) + if (get_user(p, &p32->clips)) return -EFAULT; uclips = compat_ptr(p); if (aux_space < clipcount * sizeof(*kclips)) return -EFAULT; kclips = aux_buf; - if (put_user(kclips, &kp->clips)) + if (put_user(kclips, &p64->clips)) return -EFAULT; while (clipcount--) { @@ -98,27 +98,27 @@ static int get_v4l2_window32(struct v4l2_window __user *kp, return 0; } -static int put_v4l2_window32(struct v4l2_window __user *kp, - struct v4l2_window32 __user *up) +static int put_v4l2_window32(struct v4l2_window __user *p64, + struct v4l2_window32 __user *p32) { struct v4l2_clip __user *kclips; struct v4l2_clip32 __user *uclips; compat_caddr_t p; u32 clipcount; - if (copy_in_user(&up->w, &kp->w, sizeof(kp->w)) || - assign_in_user(&up->field, &kp->field) || - assign_in_user(&up->chromakey, &kp->chromakey) || - assign_in_user(&up->global_alpha, &kp->global_alpha) || - get_user(clipcount, &kp->clipcount) || - put_user(clipcount, &up->clipcount)) + if (copy_in_user(&p32->w, &p64->w, sizeof(p64->w)) || + assign_in_user(&p32->field, &p64->field) || + assign_in_user(&p32->chromakey, &p64->chromakey) || + assign_in_user(&p32->global_alpha, &p64->global_alpha) || + get_user(clipcount, &p64->clipcount) || + put_user(clipcount, &p32->clipcount)) return -EFAULT; if (!clipcount) return 0; - if (get_user(kclips, &kp->clips)) + if (get_user(kclips, &p64->clips)) return -EFAULT; - if (get_user(p, &up->clips)) + if (get_user(p, &p32->clips)) return -EFAULT; uclips = compat_ptr(p); while (clipcount--) { @@ -161,11 +161,11 @@ struct v4l2_create_buffers32 { __u32 reserved[8]; }; -static int __bufsize_v4l2_format(struct v4l2_format32 __user *up, u32 *size) +static int __bufsize_v4l2_format(struct v4l2_format32 __user *p32, u32 *size) { u32 type; - if (get_user(type, &up->type)) + if (get_user(type, &p32->type)) return -EFAULT; switch (type) { @@ -173,7 +173,7 @@ static int __bufsize_v4l2_format(struct v4l2_format32 __user *up, u32 *size) case V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY: { u32 clipcount; - if (get_user(clipcount, &up->fmt.win.clipcount)) + if (get_user(clipcount, &p32->fmt.win.clipcount)) return -EFAULT; if (clipcount > 2048) return -EINVAL; @@ -186,141 +186,141 @@ static int __bufsize_v4l2_format(struct v4l2_format32 __user *up, u32 *size) } } -static int bufsize_v4l2_format(struct v4l2_format32 __user *up, u32 *size) +static int bufsize_v4l2_format(struct v4l2_format32 __user *p32, u32 *size) { - if (!access_ok(VERIFY_READ, up, sizeof(*up))) + if (!access_ok(VERIFY_READ, p32, sizeof(*p32))) return -EFAULT; - return __bufsize_v4l2_format(up, size); + return __bufsize_v4l2_format(p32, size); } -static int __get_v4l2_format32(struct v4l2_format __user *kp, - struct v4l2_format32 __user *up, +static int __get_v4l2_format32(struct v4l2_format __user *p64, + struct v4l2_format32 __user *p32, void __user *aux_buf, u32 aux_space) { u32 type; - if (get_user(type, &up->type) || put_user(type, &kp->type)) + if (get_user(type, &p32->type) || put_user(type, &p64->type)) return -EFAULT; switch (type) { case V4L2_BUF_TYPE_VIDEO_CAPTURE: case V4L2_BUF_TYPE_VIDEO_OUTPUT: - return copy_in_user(&kp->fmt.pix, &up->fmt.pix, - sizeof(kp->fmt.pix)) ? -EFAULT : 0; + return copy_in_user(&p64->fmt.pix, &p32->fmt.pix, + sizeof(p64->fmt.pix)) ? -EFAULT : 0; case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE: case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE: - return copy_in_user(&kp->fmt.pix_mp, &up->fmt.pix_mp, - sizeof(kp->fmt.pix_mp)) ? -EFAULT : 0; + return copy_in_user(&p64->fmt.pix_mp, &p32->fmt.pix_mp, + sizeof(p64->fmt.pix_mp)) ? -EFAULT : 0; case V4L2_BUF_TYPE_VIDEO_OVERLAY: case V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY: - return get_v4l2_window32(&kp->fmt.win, &up->fmt.win, + return get_v4l2_window32(&p64->fmt.win, &p32->fmt.win, aux_buf, aux_space); case V4L2_BUF_TYPE_VBI_CAPTURE: case V4L2_BUF_TYPE_VBI_OUTPUT: - return copy_in_user(&kp->fmt.vbi, &up->fmt.vbi, - sizeof(kp->fmt.vbi)) ? -EFAULT : 0; + return copy_in_user(&p64->fmt.vbi, &p32->fmt.vbi, + sizeof(p64->fmt.vbi)) ? -EFAULT : 0; case V4L2_BUF_TYPE_SLICED_VBI_CAPTURE: case V4L2_BUF_TYPE_SLICED_VBI_OUTPUT: - return copy_in_user(&kp->fmt.sliced, &up->fmt.sliced, - sizeof(kp->fmt.sliced)) ? -EFAULT : 0; + return copy_in_user(&p64->fmt.sliced, &p32->fmt.sliced, + sizeof(p64->fmt.sliced)) ? -EFAULT : 0; case V4L2_BUF_TYPE_SDR_CAPTURE: case V4L2_BUF_TYPE_SDR_OUTPUT: - return copy_in_user(&kp->fmt.sdr, &up->fmt.sdr, - sizeof(kp->fmt.sdr)) ? -EFAULT : 0; + return copy_in_user(&p64->fmt.sdr, &p32->fmt.sdr, + sizeof(p64->fmt.sdr)) ? -EFAULT : 0; case V4L2_BUF_TYPE_META_CAPTURE: - return copy_in_user(&kp->fmt.meta, &up->fmt.meta, - sizeof(kp->fmt.meta)) ? -EFAULT : 0; + return copy_in_user(&p64->fmt.meta, &p32->fmt.meta, + sizeof(p64->fmt.meta)) ? -EFAULT : 0; default: return -EINVAL; } } -static int get_v4l2_format32(struct v4l2_format __user *kp, - struct v4l2_format32 __user *up, +static int get_v4l2_format32(struct v4l2_format __user *p64, + struct v4l2_format32 __user *p32, void __user *aux_buf, u32 aux_space) { - if (!access_ok(VERIFY_READ, up, sizeof(*up))) + if (!access_ok(VERIFY_READ, p32, sizeof(*p32))) return -EFAULT; - return __get_v4l2_format32(kp, up, aux_buf, aux_space); + return __get_v4l2_format32(p64, p32, aux_buf, aux_space); } -static int bufsize_v4l2_create(struct v4l2_create_buffers32 __user *up, +static int bufsize_v4l2_create(struct v4l2_create_buffers32 __user *p32, u32 *size) { - if (!access_ok(VERIFY_READ, up, sizeof(*up))) + if (!access_ok(VERIFY_READ, p32, sizeof(*p32))) return -EFAULT; - return __bufsize_v4l2_format(&up->format, size); + return __bufsize_v4l2_format(&p32->format, size); } -static int get_v4l2_create32(struct v4l2_create_buffers __user *kp, - struct v4l2_create_buffers32 __user *up, +static int get_v4l2_create32(struct v4l2_create_buffers __user *p64, + struct v4l2_create_buffers32 __user *p32, void __user *aux_buf, u32 aux_space) { - if (!access_ok(VERIFY_READ, up, sizeof(*up)) || - copy_in_user(kp, up, + if (!access_ok(VERIFY_READ, p32, sizeof(*p32)) || + copy_in_user(p64, p32, offsetof(struct v4l2_create_buffers32, format))) return -EFAULT; - return __get_v4l2_format32(&kp->format, &up->format, + return __get_v4l2_format32(&p64->format, &p32->format, aux_buf, aux_space); } -static int __put_v4l2_format32(struct v4l2_format __user *kp, - struct v4l2_format32 __user *up) +static int __put_v4l2_format32(struct v4l2_format __user *p64, + struct v4l2_format32 __user *p32) { u32 type; - if (get_user(type, &kp->type)) + if (get_user(type, &p64->type)) return -EFAULT; switch (type) { case V4L2_BUF_TYPE_VIDEO_CAPTURE: case V4L2_BUF_TYPE_VIDEO_OUTPUT: - return copy_in_user(&up->fmt.pix, &kp->fmt.pix, - sizeof(kp->fmt.pix)) ? -EFAULT : 0; + return copy_in_user(&p32->fmt.pix, &p64->fmt.pix, + sizeof(p64->fmt.pix)) ? -EFAULT : 0; case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE: case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE: - return copy_in_user(&up->fmt.pix_mp, &kp->fmt.pix_mp, - sizeof(kp->fmt.pix_mp)) ? -EFAULT : 0; + return copy_in_user(&p32->fmt.pix_mp, &p64->fmt.pix_mp, + sizeof(p64->fmt.pix_mp)) ? -EFAULT : 0; case V4L2_BUF_TYPE_VIDEO_OVERLAY: case V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY: - return put_v4l2_window32(&kp->fmt.win, &up->fmt.win); + return put_v4l2_window32(&p64->fmt.win, &p32->fmt.win); case V4L2_BUF_TYPE_VBI_CAPTURE: case V4L2_BUF_TYPE_VBI_OUTPUT: - return copy_in_user(&up->fmt.vbi, &kp->fmt.vbi, - sizeof(kp->fmt.vbi)) ? -EFAULT : 0; + return copy_in_user(&p32->fmt.vbi, &p64->fmt.vbi, + sizeof(p64->fmt.vbi)) ? -EFAULT : 0; case V4L2_BUF_TYPE_SLICED_VBI_CAPTURE: case V4L2_BUF_TYPE_SLICED_VBI_OUTPUT: - return copy_in_user(&up->fmt.sliced, &kp->fmt.sliced, - sizeof(kp->fmt.sliced)) ? -EFAULT : 0; + return copy_in_user(&p32->fmt.sliced, &p64->fmt.sliced, + sizeof(p64->fmt.sliced)) ? -EFAULT : 0; case V4L2_BUF_TYPE_SDR_CAPTURE: case V4L2_BUF_TYPE_SDR_OUTPUT: - return copy_in_user(&up->fmt.sdr, &kp->fmt.sdr, - sizeof(kp->fmt.sdr)) ? -EFAULT : 0; + return copy_in_user(&p32->fmt.sdr, &p64->fmt.sdr, + sizeof(p64->fmt.sdr)) ? -EFAULT : 0; case V4L2_BUF_TYPE_META_CAPTURE: - return copy_in_user(&up->fmt.meta, &kp->fmt.meta, - sizeof(kp->fmt.meta)) ? -EFAULT : 0; + return copy_in_user(&p32->fmt.meta, &p64->fmt.meta, + sizeof(p64->fmt.meta)) ? -EFAULT : 0; default: return -EINVAL; } } -static int put_v4l2_format32(struct v4l2_format __user *kp, - struct v4l2_format32 __user *up) +static int put_v4l2_format32(struct v4l2_format __user *p64, + struct v4l2_format32 __user *p32) { - if (!access_ok(VERIFY_WRITE, up, sizeof(*up))) + if (!access_ok(VERIFY_WRITE, p32, sizeof(*p32))) return -EFAULT; - return __put_v4l2_format32(kp, up); + return __put_v4l2_format32(p64, p32); } -static int put_v4l2_create32(struct v4l2_create_buffers __user *kp, - struct v4l2_create_buffers32 __user *up) +static int put_v4l2_create32(struct v4l2_create_buffers __user *p64, + struct v4l2_create_buffers32 __user *p32) { - if (!access_ok(VERIFY_WRITE, up, sizeof(*up)) || - copy_in_user(up, kp, + if (!access_ok(VERIFY_WRITE, p32, sizeof(*p32)) || + copy_in_user(p32, p64, offsetof(struct v4l2_create_buffers32, format)) || - copy_in_user(up->reserved, kp->reserved, sizeof(kp->reserved))) + copy_in_user(p32->reserved, p64->reserved, sizeof(p64->reserved))) return -EFAULT; - return __put_v4l2_format32(&kp->format, &up->format); + return __put_v4l2_format32(&p64->format, &p32->format); } struct v4l2_standard32 { @@ -332,27 +332,27 @@ struct v4l2_standard32 { __u32 reserved[4]; }; -static int get_v4l2_standard32(struct v4l2_standard __user *kp, - struct v4l2_standard32 __user *up) +static int get_v4l2_standard32(struct v4l2_standard __user *p64, + struct v4l2_standard32 __user *p32) { /* other fields are not set by the user, nor used by the driver */ - if (!access_ok(VERIFY_READ, up, sizeof(*up)) || - assign_in_user(&kp->index, &up->index)) + if (!access_ok(VERIFY_READ, p32, sizeof(*p32)) || + assign_in_user(&p64->index, &p32->index)) return -EFAULT; return 0; } -static int put_v4l2_standard32(struct v4l2_standard __user *kp, - struct v4l2_standard32 __user *up) +static int put_v4l2_standard32(struct v4l2_standard __user *p64, + struct v4l2_standard32 __user *p32) { - if (!access_ok(VERIFY_WRITE, up, sizeof(*up)) || - assign_in_user(&up->index, &kp->index) || - assign_in_user(&up->id, &kp->id) || - copy_in_user(up->name, kp->name, sizeof(up->name)) || - copy_in_user(&up->frameperiod, &kp->frameperiod, - sizeof(up->frameperiod)) || - assign_in_user(&up->framelines, &kp->framelines) || - copy_in_user(up->reserved, kp->reserved, sizeof(up->reserved))) + if (!access_ok(VERIFY_WRITE, p32, sizeof(*p32)) || + assign_in_user(&p32->index, &p64->index) || + assign_in_user(&p32->id, &p64->id) || + copy_in_user(p32->name, p64->name, sizeof(p32->name)) || + copy_in_user(&p32->frameperiod, &p64->frameperiod, + sizeof(p32->frameperiod)) || + assign_in_user(&p32->framelines, &p64->framelines) || + copy_in_user(p32->reserved, p64->reserved, sizeof(p32->reserved))) return -EFAULT; return 0; } @@ -392,31 +392,31 @@ struct v4l2_buffer32 { __u32 reserved; }; -static int get_v4l2_plane32(struct v4l2_plane __user *up, - struct v4l2_plane32 __user *up32, +static int get_v4l2_plane32(struct v4l2_plane __user *p64, + struct v4l2_plane32 __user *p32, enum v4l2_memory memory) { compat_ulong_t p; - if (copy_in_user(up, up32, 2 * sizeof(__u32)) || - copy_in_user(&up->data_offset, &up32->data_offset, - sizeof(up->data_offset))) + if (copy_in_user(p64, p32, 2 * sizeof(__u32)) || + copy_in_user(&p64->data_offset, &p32->data_offset, + sizeof(p64->data_offset))) return -EFAULT; switch (memory) { case V4L2_MEMORY_MMAP: case V4L2_MEMORY_OVERLAY: - if (copy_in_user(&up->m.mem_offset, &up32->m.mem_offset, - sizeof(up32->m.mem_offset))) + if (copy_in_user(&p64->m.mem_offset, &p32->m.mem_offset, + sizeof(p32->m.mem_offset))) return -EFAULT; break; case V4L2_MEMORY_USERPTR: - if (get_user(p, &up32->m.userptr) || - put_user((unsigned long)compat_ptr(p), &up->m.userptr)) + if (get_user(p, &p32->m.userptr) || + put_user((unsigned long)compat_ptr(p), &p64->m.userptr)) return -EFAULT; break; case V4L2_MEMORY_DMABUF: - if (copy_in_user(&up->m.fd, &up32->m.fd, sizeof(up32->m.fd))) + if (copy_in_user(&p64->m.fd, &p32->m.fd, sizeof(p32->m.fd))) return -EFAULT; break; } @@ -424,32 +424,32 @@ static int get_v4l2_plane32(struct v4l2_plane __user *up, return 0; } -static int put_v4l2_plane32(struct v4l2_plane __user *up, - struct v4l2_plane32 __user *up32, +static int put_v4l2_plane32(struct v4l2_plane __user *p64, + struct v4l2_plane32 __user *p32, enum v4l2_memory memory) { unsigned long p; - if (copy_in_user(up32, up, 2 * sizeof(__u32)) || - copy_in_user(&up32->data_offset, &up->data_offset, - sizeof(up->data_offset))) + if (copy_in_user(p32, p64, 2 * sizeof(__u32)) || + copy_in_user(&p32->data_offset, &p64->data_offset, + sizeof(p64->data_offset))) return -EFAULT; switch (memory) { case V4L2_MEMORY_MMAP: case V4L2_MEMORY_OVERLAY: - if (copy_in_user(&up32->m.mem_offset, &up->m.mem_offset, - sizeof(up->m.mem_offset))) + if (copy_in_user(&p32->m.mem_offset, &p64->m.mem_offset, + sizeof(p64->m.mem_offset))) return -EFAULT; break; case V4L2_MEMORY_USERPTR: - if (get_user(p, &up->m.userptr)|| + if (get_user(p, &p64->m.userptr)|| put_user((compat_ulong_t)ptr_to_compat((void __user *)p), - &up32->m.userptr)) + &p32->m.userptr)) return -EFAULT; break; case V4L2_MEMORY_DMABUF: - if (copy_in_user(&up32->m.fd, &up->m.fd, sizeof(up->m.fd))) + if (copy_in_user(&p32->m.fd, &p64->m.fd, sizeof(p64->m.fd))) return -EFAULT; break; } @@ -457,14 +457,14 @@ static int put_v4l2_plane32(struct v4l2_plane __user *up, return 0; } -static int bufsize_v4l2_buffer(struct v4l2_buffer32 __user *up, u32 *size) +static int bufsize_v4l2_buffer(struct v4l2_buffer32 __user *p32, u32 *size) { u32 type; u32 length; - if (!access_ok(VERIFY_READ, up, sizeof(*up)) || - get_user(type, &up->type) || - get_user(length, &up->length)) + if (!access_ok(VERIFY_READ, p32, sizeof(*p32)) || + get_user(type, &p32->type) || + get_user(length, &p32->length)) return -EFAULT; if (V4L2_TYPE_IS_MULTIPLANAR(type)) { @@ -482,8 +482,8 @@ static int bufsize_v4l2_buffer(struct v4l2_buffer32 __user *up, u32 *size) return 0; } -static int get_v4l2_buffer32(struct v4l2_buffer __user *kp, - struct v4l2_buffer32 __user *up, +static int get_v4l2_buffer32(struct v4l2_buffer __user *p64, + struct v4l2_buffer32 __user *p32, void __user *aux_buf, u32 aux_space) { u32 type; @@ -494,24 +494,24 @@ static int get_v4l2_buffer32(struct v4l2_buffer __user *kp, compat_caddr_t p; int ret; - if (!access_ok(VERIFY_READ, up, sizeof(*up)) || - assign_in_user(&kp->index, &up->index) || - get_user(type, &up->type) || - put_user(type, &kp->type) || - assign_in_user(&kp->flags, &up->flags) || - get_user(memory, &up->memory) || - put_user(memory, &kp->memory) || - get_user(length, &up->length) || - put_user(length, &kp->length)) + if (!access_ok(VERIFY_READ, p32, sizeof(*p32)) || + assign_in_user(&p64->index, &p32->index) || + get_user(type, &p32->type) || + put_user(type, &p64->type) || + assign_in_user(&p64->flags, &p32->flags) || + get_user(memory, &p32->memory) || + put_user(memory, &p64->memory) || + get_user(length, &p32->length) || + put_user(length, &p64->length)) return -EFAULT; if (V4L2_TYPE_IS_OUTPUT(type)) - if (assign_in_user(&kp->bytesused, &up->bytesused) || - assign_in_user(&kp->field, &up->field) || - assign_in_user(&kp->timestamp.tv_sec, - &up->timestamp.tv_sec) || - assign_in_user(&kp->timestamp.tv_usec, - &up->timestamp.tv_usec)) + if (assign_in_user(&p64->bytesused, &p32->bytesused) || + assign_in_user(&p64->field, &p32->field) || + assign_in_user(&p64->timestamp.tv_sec, + &p32->timestamp.tv_sec) || + assign_in_user(&p64->timestamp.tv_usec, + &p32->timestamp.tv_usec)) return -EFAULT; if (V4L2_TYPE_IS_MULTIPLANAR(type)) { @@ -522,12 +522,12 @@ static int get_v4l2_buffer32(struct v4l2_buffer __user *kp, * num_planes == 0 is legal, e.g. when userspace doesn't * need planes array on DQBUF */ - return put_user(NULL, &kp->m.planes); + return put_user(NULL, &p64->m.planes); } if (num_planes > VIDEO_MAX_PLANES) return -EINVAL; - if (get_user(p, &up->m.planes)) + if (get_user(p, &p32->m.planes)) return -EFAULT; uplane32 = compat_ptr(p); @@ -544,7 +544,7 @@ static int get_v4l2_buffer32(struct v4l2_buffer __user *kp, uplane = aux_buf; if (put_user((__force struct v4l2_plane *)uplane, - &kp->m.planes)) + &p64->m.planes)) return -EFAULT; while (num_planes--) { @@ -558,20 +558,20 @@ static int get_v4l2_buffer32(struct v4l2_buffer __user *kp, switch (memory) { case V4L2_MEMORY_MMAP: case V4L2_MEMORY_OVERLAY: - if (assign_in_user(&kp->m.offset, &up->m.offset)) + if (assign_in_user(&p64->m.offset, &p32->m.offset)) return -EFAULT; break; case V4L2_MEMORY_USERPTR: { compat_ulong_t userptr; - if (get_user(userptr, &up->m.userptr) || + if (get_user(userptr, &p32->m.userptr) || put_user((unsigned long)compat_ptr(userptr), - &kp->m.userptr)) + &p64->m.userptr)) return -EFAULT; break; } case V4L2_MEMORY_DMABUF: - if (assign_in_user(&kp->m.fd, &up->m.fd)) + if (assign_in_user(&p64->m.fd, &p32->m.fd)) return -EFAULT; break; } @@ -580,8 +580,8 @@ static int get_v4l2_buffer32(struct v4l2_buffer __user *kp, return 0; } -static int put_v4l2_buffer32(struct v4l2_buffer __user *kp, - struct v4l2_buffer32 __user *up) +static int put_v4l2_buffer32(struct v4l2_buffer __user *p64, + struct v4l2_buffer32 __user *p32) { u32 type; u32 length; @@ -591,25 +591,25 @@ static int put_v4l2_buffer32(struct v4l2_buffer __user *kp, compat_caddr_t p; int ret; - if (!access_ok(VERIFY_WRITE, up, sizeof(*up)) || - assign_in_user(&up->index, &kp->index) || - get_user(type, &kp->type) || - put_user(type, &up->type) || - assign_in_user(&up->flags, &kp->flags) || - get_user(memory, &kp->memory) || - put_user(memory, &up->memory)) + if (!access_ok(VERIFY_WRITE, p32, sizeof(*p32)) || + assign_in_user(&p32->index, &p64->index) || + get_user(type, &p64->type) || + put_user(type, &p32->type) || + assign_in_user(&p32->flags, &p64->flags) || + get_user(memory, &p64->memory) || + put_user(memory, &p32->memory)) return -EFAULT; - if (assign_in_user(&up->bytesused, &kp->bytesused) || - assign_in_user(&up->field, &kp->field) || - assign_in_user(&up->timestamp.tv_sec, &kp->timestamp.tv_sec) || - assign_in_user(&up->timestamp.tv_usec, &kp->timestamp.tv_usec) || - copy_in_user(&up->timecode, &kp->timecode, sizeof(kp->timecode)) || - assign_in_user(&up->sequence, &kp->sequence) || - assign_in_user(&up->reserved2, &kp->reserved2) || - assign_in_user(&up->reserved, &kp->reserved) || - get_user(length, &kp->length) || - put_user(length, &up->length)) + if (assign_in_user(&p32->bytesused, &p64->bytesused) || + assign_in_user(&p32->field, &p64->field) || + assign_in_user(&p32->timestamp.tv_sec, &p64->timestamp.tv_sec) || + assign_in_user(&p32->timestamp.tv_usec, &p64->timestamp.tv_usec) || + copy_in_user(&p32->timecode, &p64->timecode, sizeof(p64->timecode)) || + assign_in_user(&p32->sequence, &p64->sequence) || + assign_in_user(&p32->reserved2, &p64->reserved2) || + assign_in_user(&p32->reserved, &p64->reserved) || + get_user(length, &p64->length) || + put_user(length, &p32->length)) return -EFAULT; if (V4L2_TYPE_IS_MULTIPLANAR(type)) { @@ -625,9 +625,9 @@ static int put_v4l2_buffer32(struct v4l2_buffer __user *kp, * smatch warnings, so instead declare it without __user * and cast it as a userspace pointer to put_v4l2_plane32(). */ - if (get_user(uplane, &kp->m.planes)) + if (get_user(uplane, &p64->m.planes)) return -EFAULT; - if (get_user(p, &up->m.planes)) + if (get_user(p, &p32->m.planes)) return -EFAULT; uplane32 = compat_ptr(p); @@ -642,15 +642,15 @@ static int put_v4l2_buffer32(struct v4l2_buffer __user *kp, switch (memory) { case V4L2_MEMORY_MMAP: case V4L2_MEMORY_OVERLAY: - if (assign_in_user(&up->m.offset, &kp->m.offset)) + if (assign_in_user(&p32->m.offset, &p64->m.offset)) return -EFAULT; break; case V4L2_MEMORY_USERPTR: - if (assign_in_user(&up->m.userptr, &kp->m.userptr)) + if (assign_in_user(&p32->m.userptr, &p64->m.userptr)) return -EFAULT; break; case V4L2_MEMORY_DMABUF: - if (assign_in_user(&up->m.fd, &kp->m.fd)) + if (assign_in_user(&p32->m.fd, &p64->m.fd)) return -EFAULT; break; } @@ -675,32 +675,32 @@ struct v4l2_framebuffer32 { } fmt; }; -static int get_v4l2_framebuffer32(struct v4l2_framebuffer __user *kp, - struct v4l2_framebuffer32 __user *up) +static int get_v4l2_framebuffer32(struct v4l2_framebuffer __user *p64, + struct v4l2_framebuffer32 __user *p32) { compat_caddr_t tmp; - if (!access_ok(VERIFY_READ, up, sizeof(*up)) || - get_user(tmp, &up->base) || - put_user((void __force *)compat_ptr(tmp), &kp->base) || - assign_in_user(&kp->capability, &up->capability) || - assign_in_user(&kp->flags, &up->flags) || - copy_in_user(&kp->fmt, &up->fmt, sizeof(kp->fmt))) + if (!access_ok(VERIFY_READ, p32, sizeof(*p32)) || + get_user(tmp, &p32->base) || + put_user((void __force *)compat_ptr(tmp), &p64->base) || + assign_in_user(&p64->capability, &p32->capability) || + assign_in_user(&p64->flags, &p32->flags) || + copy_in_user(&p64->fmt, &p32->fmt, sizeof(p64->fmt))) return -EFAULT; return 0; } -static int put_v4l2_framebuffer32(struct v4l2_framebuffer __user *kp, - struct v4l2_framebuffer32 __user *up) +static int put_v4l2_framebuffer32(struct v4l2_framebuffer __user *p64, + struct v4l2_framebuffer32 __user *p32) { void *base; - if (!access_ok(VERIFY_WRITE, up, sizeof(*up)) || - get_user(base, &kp->base) || - put_user(ptr_to_compat((void __user *)base), &up->base) || - assign_in_user(&up->capability, &kp->capability) || - assign_in_user(&up->flags, &kp->flags) || - copy_in_user(&up->fmt, &kp->fmt, sizeof(kp->fmt))) + if (!access_ok(VERIFY_WRITE, p32, sizeof(*p32)) || + get_user(base, &p64->base) || + put_user(ptr_to_compat((void __user *)base), &p32->base) || + assign_in_user(&p32->capability, &p64->capability) || + assign_in_user(&p32->flags, &p64->flags) || + copy_in_user(&p32->fmt, &p64->fmt, sizeof(p64->fmt))) return -EFAULT; return 0; } @@ -721,18 +721,18 @@ struct v4l2_input32 { * The 64-bit v4l2_input struct has extra padding at the end of the struct. * Otherwise it is identical to the 32-bit version. */ -static inline int get_v4l2_input32(struct v4l2_input __user *kp, - struct v4l2_input32 __user *up) +static inline int get_v4l2_input32(struct v4l2_input __user *p64, + struct v4l2_input32 __user *p32) { - if (copy_in_user(kp, up, sizeof(*up))) + if (copy_in_user(p64, p32, sizeof(*p32))) return -EFAULT; return 0; } -static inline int put_v4l2_input32(struct v4l2_input __user *kp, - struct v4l2_input32 __user *up) +static inline int put_v4l2_input32(struct v4l2_input __user *p64, + struct v4l2_input32 __user *p32) { - if (copy_in_user(up, kp, sizeof(*up))) + if (copy_in_user(p32, p64, sizeof(*p32))) return -EFAULT; return 0; } @@ -786,13 +786,13 @@ static inline bool ctrl_is_pointer(struct file *file, u32 id) (qec.flags & V4L2_CTRL_FLAG_HAS_PAYLOAD); } -static int bufsize_v4l2_ext_controls(struct v4l2_ext_controls32 __user *up, +static int bufsize_v4l2_ext_controls(struct v4l2_ext_controls32 __user *p32, u32 *size) { u32 count; - if (!access_ok(VERIFY_READ, up, sizeof(*up)) || - get_user(count, &up->count)) + if (!access_ok(VERIFY_READ, p32, sizeof(*p32)) || + get_user(count, &p32->count)) return -EFAULT; if (count > V4L2_CID_MAX_CTRLS) return -EINVAL; @@ -801,8 +801,8 @@ static int bufsize_v4l2_ext_controls(struct v4l2_ext_controls32 __user *up, } static int get_v4l2_ext_controls32(struct file *file, - struct v4l2_ext_controls __user *kp, - struct v4l2_ext_controls32 __user *up, + struct v4l2_ext_controls __user *p64, + struct v4l2_ext_controls32 __user *p32, void __user *aux_buf, u32 aux_space) { struct v4l2_ext_control32 __user *ucontrols; @@ -811,19 +811,19 @@ static int get_v4l2_ext_controls32(struct file *file, u32 n; compat_caddr_t p; - if (!access_ok(VERIFY_READ, up, sizeof(*up)) || - assign_in_user(&kp->which, &up->which) || - get_user(count, &up->count) || - put_user(count, &kp->count) || - assign_in_user(&kp->error_idx, &up->error_idx) || - copy_in_user(kp->reserved, up->reserved, sizeof(kp->reserved))) + if (!access_ok(VERIFY_READ, p32, sizeof(*p32)) || + assign_in_user(&p64->which, &p32->which) || + get_user(count, &p32->count) || + put_user(count, &p64->count) || + assign_in_user(&p64->error_idx, &p32->error_idx) || + copy_in_user(p64->reserved, p32->reserved, sizeof(p64->reserved))) return -EFAULT; if (count == 0) - return put_user(NULL, &kp->controls); + return put_user(NULL, &p64->controls); if (count > V4L2_CID_MAX_CTRLS) return -EINVAL; - if (get_user(p, &up->controls)) + if (get_user(p, &p32->controls)) return -EFAULT; ucontrols = compat_ptr(p); if (!access_ok(VERIFY_READ, ucontrols, count * sizeof(*ucontrols))) @@ -832,7 +832,7 @@ static int get_v4l2_ext_controls32(struct file *file, return -EFAULT; kcontrols = aux_buf; if (put_user((__force struct v4l2_ext_control *)kcontrols, - &kp->controls)) + &p64->controls)) return -EFAULT; for (n = 0; n < count; n++) { @@ -860,8 +860,8 @@ static int get_v4l2_ext_controls32(struct file *file, } static int put_v4l2_ext_controls32(struct file *file, - struct v4l2_ext_controls __user *kp, - struct v4l2_ext_controls32 __user *up) + struct v4l2_ext_controls __user *p64, + struct v4l2_ext_controls32 __user *p32) { struct v4l2_ext_control32 __user *ucontrols; struct v4l2_ext_control *kcontrols; @@ -877,18 +877,18 @@ static int put_v4l2_ext_controls32(struct file *file, * with __user causes smatch warnings, so instead declare it * without __user and cast it as a userspace pointer where needed. */ - if (!access_ok(VERIFY_WRITE, up, sizeof(*up)) || - assign_in_user(&up->which, &kp->which) || - get_user(count, &kp->count) || - put_user(count, &up->count) || - assign_in_user(&up->error_idx, &kp->error_idx) || - copy_in_user(up->reserved, kp->reserved, sizeof(up->reserved)) || - get_user(kcontrols, &kp->controls)) + if (!access_ok(VERIFY_WRITE, p32, sizeof(*p32)) || + assign_in_user(&p32->which, &p64->which) || + get_user(count, &p64->count) || + put_user(count, &p32->count) || + assign_in_user(&p32->error_idx, &p64->error_idx) || + copy_in_user(p32->reserved, p64->reserved, sizeof(p32->reserved)) || + get_user(kcontrols, &p64->controls)) return -EFAULT; if (!count || count > (U32_MAX/sizeof(*ucontrols))) return 0; - if (get_user(p, &up->controls)) + if (get_user(p, &p32->controls)) return -EFAULT; ucontrols = compat_ptr(p); if (!access_ok(VERIFY_WRITE, ucontrols, count * sizeof(*ucontrols))) @@ -938,18 +938,18 @@ struct v4l2_event32 { __u32 reserved[8]; }; -static int put_v4l2_event32(struct v4l2_event __user *kp, - struct v4l2_event32 __user *up) +static int put_v4l2_event32(struct v4l2_event __user *p64, + struct v4l2_event32 __user *p32) { - if (!access_ok(VERIFY_WRITE, up, sizeof(*up)) || - assign_in_user(&up->type, &kp->type) || - copy_in_user(&up->u, &kp->u, sizeof(kp->u)) || - assign_in_user(&up->pending, &kp->pending) || - assign_in_user(&up->sequence, &kp->sequence) || - assign_in_user(&up->timestamp.tv_sec, &kp->timestamp.tv_sec) || - assign_in_user(&up->timestamp.tv_nsec, &kp->timestamp.tv_nsec) || - assign_in_user(&up->id, &kp->id) || - copy_in_user(up->reserved, kp->reserved, sizeof(up->reserved))) + if (!access_ok(VERIFY_WRITE, p32, sizeof(*p32)) || + assign_in_user(&p32->type, &p64->type) || + copy_in_user(&p32->u, &p64->u, sizeof(p64->u)) || + assign_in_user(&p32->pending, &p64->pending) || + assign_in_user(&p32->sequence, &p64->sequence) || + assign_in_user(&p32->timestamp.tv_sec, &p64->timestamp.tv_sec) || + assign_in_user(&p32->timestamp.tv_nsec, &p64->timestamp.tv_nsec) || + assign_in_user(&p32->id, &p64->id) || + copy_in_user(p32->reserved, p64->reserved, sizeof(p32->reserved))) return -EFAULT; return 0; } @@ -962,35 +962,35 @@ struct v4l2_edid32 { compat_caddr_t edid; }; -static int get_v4l2_edid32(struct v4l2_edid __user *kp, - struct v4l2_edid32 __user *up) +static int get_v4l2_edid32(struct v4l2_edid __user *p64, + struct v4l2_edid32 __user *p32) { compat_uptr_t tmp; - if (!access_ok(VERIFY_READ, up, sizeof(*up)) || - assign_in_user(&kp->pad, &up->pad) || - assign_in_user(&kp->start_block, &up->start_block) || - assign_in_user(&kp->blocks, - (unsigned char __user *)&up->blocks) || - get_user(tmp, &up->edid) || - put_user((void __force *)compat_ptr(tmp), &kp->edid) || - copy_in_user(kp->reserved, up->reserved, sizeof(kp->reserved))) + if (!access_ok(VERIFY_READ, p32, sizeof(*p32)) || + assign_in_user(&p64->pad, &p32->pad) || + assign_in_user(&p64->start_block, &p32->start_block) || + assign_in_user(&p64->blocks, + (unsigned char __user *)&p32->blocks) || + get_user(tmp, &p32->edid) || + put_user((void __force *)compat_ptr(tmp), &p64->edid) || + copy_in_user(p64->reserved, p32->reserved, sizeof(p64->reserved))) return -EFAULT; return 0; } -static int put_v4l2_edid32(struct v4l2_edid __user *kp, - struct v4l2_edid32 __user *up) +static int put_v4l2_edid32(struct v4l2_edid __user *p64, + struct v4l2_edid32 __user *p32) { void *edid; - if (!access_ok(VERIFY_WRITE, up, sizeof(*up)) || - assign_in_user(&up->pad, &kp->pad) || - assign_in_user(&up->start_block, &kp->start_block) || - assign_in_user(&up->blocks, &kp->blocks) || - get_user(edid, &kp->edid) || - put_user(ptr_to_compat((void __user *)edid), &up->edid) || - copy_in_user(up->reserved, kp->reserved, sizeof(up->reserved))) + if (!access_ok(VERIFY_WRITE, p32, sizeof(*p32)) || + assign_in_user(&p32->pad, &p64->pad) || + assign_in_user(&p32->start_block, &p64->start_block) || + assign_in_user(&p32->blocks, &p64->blocks) || + get_user(edid, &p64->edid) || + put_user(ptr_to_compat((void __user *)edid), &p32->edid) || + copy_in_user(p32->reserved, p64->reserved, sizeof(p32->reserved))) return -EFAULT; return 0; } @@ -1024,20 +1024,20 @@ static int put_v4l2_edid32(struct v4l2_edid __user *kp, #define VIDIOC_S_OUTPUT32 _IOWR('V', 47, s32) static int alloc_userspace(unsigned int size, u32 aux_space, - void __user **up_native) + void __user **new_p64) { - *up_native = compat_alloc_user_space(size + aux_space); - if (!*up_native) + *new_p64 = compat_alloc_user_space(size + aux_space); + if (!*new_p64) return -ENOMEM; - if (clear_user(*up_native, size)) + if (clear_user(*new_p64, size)) return -EFAULT; return 0; } static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { - void __user *up = compat_ptr(arg); - void __user *up_native = NULL; + void __user *p32 = compat_ptr(arg); + void __user *new_p64 = NULL; void __user *aux_buf; u32 aux_space; int compatible_arg = 1; @@ -1078,50 +1078,50 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar case VIDIOC_STREAMOFF: case VIDIOC_S_INPUT: case VIDIOC_S_OUTPUT: - err = alloc_userspace(sizeof(unsigned int), 0, &up_native); - if (!err && assign_in_user((unsigned int __user *)up_native, - (compat_uint_t __user *)up)) + err = alloc_userspace(sizeof(unsigned int), 0, &new_p64); + if (!err && assign_in_user((unsigned int __user *)new_p64, + (compat_uint_t __user *)p32)) err = -EFAULT; compatible_arg = 0; break; case VIDIOC_G_INPUT: case VIDIOC_G_OUTPUT: - err = alloc_userspace(sizeof(unsigned int), 0, &up_native); + err = alloc_userspace(sizeof(unsigned int), 0, &new_p64); compatible_arg = 0; break; case VIDIOC_G_EDID: case VIDIOC_S_EDID: - err = alloc_userspace(sizeof(struct v4l2_edid), 0, &up_native); + err = alloc_userspace(sizeof(struct v4l2_edid), 0, &new_p64); if (!err) - err = get_v4l2_edid32(up_native, up); + err = get_v4l2_edid32(new_p64, p32); compatible_arg = 0; break; case VIDIOC_G_FMT: case VIDIOC_S_FMT: case VIDIOC_TRY_FMT: - err = bufsize_v4l2_format(up, &aux_space); + err = bufsize_v4l2_format(p32, &aux_space); if (!err) err = alloc_userspace(sizeof(struct v4l2_format), - aux_space, &up_native); + aux_space, &new_p64); if (!err) { - aux_buf = up_native + sizeof(struct v4l2_format); - err = get_v4l2_format32(up_native, up, + aux_buf = new_p64 + sizeof(struct v4l2_format); + err = get_v4l2_format32(new_p64, p32, aux_buf, aux_space); } compatible_arg = 0; break; case VIDIOC_CREATE_BUFS: - err = bufsize_v4l2_create(up, &aux_space); + err = bufsize_v4l2_create(p32, &aux_space); if (!err) err = alloc_userspace(sizeof(struct v4l2_create_buffers), - aux_space, &up_native); + aux_space, &new_p64); if (!err) { - aux_buf = up_native + sizeof(struct v4l2_create_buffers); - err = get_v4l2_create32(up_native, up, + aux_buf = new_p64 + sizeof(struct v4l2_create_buffers); + err = get_v4l2_create32(new_p64, p32, aux_buf, aux_space); } compatible_arg = 0; @@ -1131,13 +1131,13 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar case VIDIOC_QUERYBUF: case VIDIOC_QBUF: case VIDIOC_DQBUF: - err = bufsize_v4l2_buffer(up, &aux_space); + err = bufsize_v4l2_buffer(p32, &aux_space); if (!err) err = alloc_userspace(sizeof(struct v4l2_buffer), - aux_space, &up_native); + aux_space, &new_p64); if (!err) { - aux_buf = up_native + sizeof(struct v4l2_buffer); - err = get_v4l2_buffer32(up_native, up, + aux_buf = new_p64 + sizeof(struct v4l2_buffer); + err = get_v4l2_buffer32(new_p64, p32, aux_buf, aux_space); } compatible_arg = 0; @@ -1145,49 +1145,49 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar case VIDIOC_S_FBUF: err = alloc_userspace(sizeof(struct v4l2_framebuffer), 0, - &up_native); + &new_p64); if (!err) - err = get_v4l2_framebuffer32(up_native, up); + err = get_v4l2_framebuffer32(new_p64, p32); compatible_arg = 0; break; case VIDIOC_G_FBUF: err = alloc_userspace(sizeof(struct v4l2_framebuffer), 0, - &up_native); + &new_p64); compatible_arg = 0; break; case VIDIOC_ENUMSTD: err = alloc_userspace(sizeof(struct v4l2_standard), 0, - &up_native); + &new_p64); if (!err) - err = get_v4l2_standard32(up_native, up); + err = get_v4l2_standard32(new_p64, p32); compatible_arg = 0; break; case VIDIOC_ENUMINPUT: - err = alloc_userspace(sizeof(struct v4l2_input), 0, &up_native); + err = alloc_userspace(sizeof(struct v4l2_input), 0, &new_p64); if (!err) - err = get_v4l2_input32(up_native, up); + err = get_v4l2_input32(new_p64, p32); compatible_arg = 0; break; case VIDIOC_G_EXT_CTRLS: case VIDIOC_S_EXT_CTRLS: case VIDIOC_TRY_EXT_CTRLS: - err = bufsize_v4l2_ext_controls(up, &aux_space); + err = bufsize_v4l2_ext_controls(p32, &aux_space); if (!err) err = alloc_userspace(sizeof(struct v4l2_ext_controls), - aux_space, &up_native); + aux_space, &new_p64); if (!err) { - aux_buf = up_native + sizeof(struct v4l2_ext_controls); - err = get_v4l2_ext_controls32(file, up_native, up, + aux_buf = new_p64 + sizeof(struct v4l2_ext_controls); + err = get_v4l2_ext_controls32(file, new_p64, p32, aux_buf, aux_space); } compatible_arg = 0; break; case VIDIOC_DQEVENT: - err = alloc_userspace(sizeof(struct v4l2_event), 0, &up_native); + err = alloc_userspace(sizeof(struct v4l2_event), 0, &new_p64); compatible_arg = 0; break; } @@ -1195,9 +1195,9 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar return err; if (compatible_arg) - err = native_ioctl(file, cmd, (unsigned long)up); + err = native_ioctl(file, cmd, (unsigned long)p32); else - err = native_ioctl(file, cmd, (unsigned long)up_native); + err = native_ioctl(file, cmd, (unsigned long)new_p64); if (err == -ENOTTY) return err; @@ -1211,11 +1211,11 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar case VIDIOC_G_EXT_CTRLS: case VIDIOC_S_EXT_CTRLS: case VIDIOC_TRY_EXT_CTRLS: - if (put_v4l2_ext_controls32(file, up_native, up)) + if (put_v4l2_ext_controls32(file, new_p64, p32)) err = -EFAULT; break; case VIDIOC_S_EDID: - if (put_v4l2_edid32(up_native, up)) + if (put_v4l2_edid32(new_p64, p32)) err = -EFAULT; break; } @@ -1227,46 +1227,46 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar case VIDIOC_S_OUTPUT: case VIDIOC_G_INPUT: case VIDIOC_G_OUTPUT: - if (assign_in_user((compat_uint_t __user *)up, - ((unsigned int __user *)up_native))) + if (assign_in_user((compat_uint_t __user *)p32, + ((unsigned int __user *)new_p64))) err = -EFAULT; break; case VIDIOC_G_FBUF: - err = put_v4l2_framebuffer32(up_native, up); + err = put_v4l2_framebuffer32(new_p64, p32); break; case VIDIOC_DQEVENT: - err = put_v4l2_event32(up_native, up); + err = put_v4l2_event32(new_p64, p32); break; case VIDIOC_G_EDID: - err = put_v4l2_edid32(up_native, up); + err = put_v4l2_edid32(new_p64, p32); break; case VIDIOC_G_FMT: case VIDIOC_S_FMT: case VIDIOC_TRY_FMT: - err = put_v4l2_format32(up_native, up); + err = put_v4l2_format32(new_p64, p32); break; case VIDIOC_CREATE_BUFS: - err = put_v4l2_create32(up_native, up); + err = put_v4l2_create32(new_p64, p32); break; case VIDIOC_PREPARE_BUF: case VIDIOC_QUERYBUF: case VIDIOC_QBUF: case VIDIOC_DQBUF: - err = put_v4l2_buffer32(up_native, up); + err = put_v4l2_buffer32(new_p64, p32); break; case VIDIOC_ENUMSTD: - err = put_v4l2_standard32(up_native, up); + err = put_v4l2_standard32(new_p64, p32); break; case VIDIOC_ENUMINPUT: - err = put_v4l2_input32(up_native, up); + err = put_v4l2_input32(new_p64, p32); break; } return err; -- 2.14.3 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH RESEND 6/6] media: v4l2-compat-ioctl32: simplify casts 2018-04-19 11:15 [PATCH RESEND 0/6] Remaining COMPILE_TEST and smatch cleanups Mauro Carvalho Chehab ` (4 preceding siblings ...) 2018-04-19 11:15 ` [PATCH RESEND 5/6] media: v4l2-compat-ioctl32: better name userspace pointers Mauro Carvalho Chehab @ 2018-04-19 11:15 ` Mauro Carvalho Chehab 2018-04-19 11:37 ` Hans Verkuil 5 siblings, 1 reply; 13+ messages in thread From: Mauro Carvalho Chehab @ 2018-04-19 11:15 UTC (permalink / raw) Cc: Mauro Carvalho Chehab, Linux Media Mailing List, Mauro Carvalho Chehab, Hans Verkuil, Sakari Ailus, Daniel Mentz Making the cast right for get_user/put_user is not trivial, as it needs to ensure that the types are the correct ones. Improve it by using macros. Tested with vivid with: $ sudo modprobe vivid no_error_inj=1 $ v4l2-compliance-32bits -a -s10 >32bits && v4l2-compliance-64bits -a -s10 > 64bits && diff -U0 32bits 64bits --- 32bits 2018-04-17 11:18:29.141240772 -0300 +++ 64bits 2018-04-17 11:18:40.635282341 -0300 @@ -1 +1 @@ -v4l2-compliance SHA : bc71e4a67c6fbc5940062843bc41e7c8679634ce, 32 bits +v4l2-compliance SHA : bc71e4a67c6fbc5940062843bc41e7c8679634ce, 64 bits Using the latest version of v4l-utils with this patch applied: https://patchwork.linuxtv.org/patch/48746/ Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> --- drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 40 ++++++++++++++++++--------- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c index 8c05dd9660d3..d2f0268427c2 100644 --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c @@ -30,6 +30,24 @@ get_user(__assign_tmp, from) || put_user(__assign_tmp, to); \ }) +#define get_user_cast(__x, __ptr) \ +({ \ + get_user(__x, (typeof(*__ptr) __user *)(__ptr)); \ +}) + +#define put_user_force(__x, __ptr) \ +({ \ + put_user((typeof(*__x) __force *)(__x), __ptr); \ +}) + +#define assign_in_user_cast(to, from) \ +({ \ + typeof(*from) __assign_tmp; \ + \ + get_user_cast(__assign_tmp, from) || put_user(__assign_tmp, to);\ +}) + + static long native_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { long ret = -ENOIOCTLCMD; @@ -543,8 +561,7 @@ static int get_v4l2_buffer32(struct v4l2_buffer __user *p64, return -EFAULT; uplane = aux_buf; - if (put_user((__force struct v4l2_plane *)uplane, - &p64->m.planes)) + if (put_user_force(uplane, &p64->m.planes)) return -EFAULT; while (num_planes--) { @@ -682,7 +699,7 @@ static int get_v4l2_framebuffer32(struct v4l2_framebuffer __user *p64, if (!access_ok(VERIFY_READ, p32, sizeof(*p32)) || get_user(tmp, &p32->base) || - put_user((void __force *)compat_ptr(tmp), &p64->base) || + put_user_force(compat_ptr(tmp), &p64->base) || assign_in_user(&p64->capability, &p32->capability) || assign_in_user(&p64->flags, &p32->flags) || copy_in_user(&p64->fmt, &p32->fmt, sizeof(p64->fmt))) @@ -831,8 +848,7 @@ static int get_v4l2_ext_controls32(struct file *file, if (aux_space < count * sizeof(*kcontrols)) return -EFAULT; kcontrols = aux_buf; - if (put_user((__force struct v4l2_ext_control *)kcontrols, - &p64->controls)) + if (put_user_force(kcontrols, &p64->controls)) return -EFAULT; for (n = 0; n < count; n++) { @@ -898,12 +914,11 @@ static int put_v4l2_ext_controls32(struct file *file, unsigned int size = sizeof(*ucontrols); u32 id; - if (get_user(id, (unsigned int __user *)&kcontrols->id) || + if (get_user_cast(id, &kcontrols->id) || put_user(id, &ucontrols->id) || - assign_in_user(&ucontrols->size, - (unsigned int __user *)&kcontrols->size) || + assign_in_user_cast(&ucontrols->size, &kcontrols->size) || copy_in_user(&ucontrols->reserved2, - (unsigned int __user *)&kcontrols->reserved2, + (void __user *)&kcontrols->reserved2, sizeof(ucontrols->reserved2))) return -EFAULT; @@ -916,7 +931,7 @@ static int put_v4l2_ext_controls32(struct file *file, size -= sizeof(ucontrols->value64); if (copy_in_user(ucontrols, - (unsigned int __user *)kcontrols, size)) + (void __user *)kcontrols, size)) return -EFAULT; ucontrols++; @@ -970,10 +985,9 @@ static int get_v4l2_edid32(struct v4l2_edid __user *p64, if (!access_ok(VERIFY_READ, p32, sizeof(*p32)) || assign_in_user(&p64->pad, &p32->pad) || assign_in_user(&p64->start_block, &p32->start_block) || - assign_in_user(&p64->blocks, - (unsigned char __user *)&p32->blocks) || + assign_in_user_cast(&p64->blocks, &p32->blocks) || get_user(tmp, &p32->edid) || - put_user((void __force *)compat_ptr(tmp), &p64->edid) || + put_user_force(compat_ptr(tmp), &p64->edid) || copy_in_user(p64->reserved, p32->reserved, sizeof(p64->reserved))) return -EFAULT; return 0; -- 2.14.3 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH RESEND 6/6] media: v4l2-compat-ioctl32: simplify casts 2018-04-19 11:15 ` [PATCH RESEND 6/6] media: v4l2-compat-ioctl32: simplify casts Mauro Carvalho Chehab @ 2018-04-19 11:37 ` Hans Verkuil 2018-04-19 15:27 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 13+ messages in thread From: Hans Verkuil @ 2018-04-19 11:37 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Hans Verkuil, Sakari Ailus, Daniel Mentz On 04/19/18 13:15, Mauro Carvalho Chehab wrote: > Making the cast right for get_user/put_user is not trivial, as > it needs to ensure that the types are the correct ones. > > Improve it by using macros. > > Tested with vivid with: > $ sudo modprobe vivid no_error_inj=1 > $ v4l2-compliance-32bits -a -s10 >32bits && v4l2-compliance-64bits -a -s10 > 64bits && diff -U0 32bits 64bits > --- 32bits 2018-04-17 11:18:29.141240772 -0300 > +++ 64bits 2018-04-17 11:18:40.635282341 -0300 > @@ -1 +1 @@ > -v4l2-compliance SHA : bc71e4a67c6fbc5940062843bc41e7c8679634ce, 32 bits > +v4l2-compliance SHA : bc71e4a67c6fbc5940062843bc41e7c8679634ce, 64 bits > > Using the latest version of v4l-utils with this patch applied: > https://patchwork.linuxtv.org/patch/48746/ > > Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> > --- > drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 40 ++++++++++++++++++--------- > 1 file changed, 27 insertions(+), 13 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > index 8c05dd9660d3..d2f0268427c2 100644 > --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > @@ -30,6 +30,24 @@ > get_user(__assign_tmp, from) || put_user(__assign_tmp, to); \ > }) > > +#define get_user_cast(__x, __ptr) \ > +({ \ > + get_user(__x, (typeof(*__ptr) __user *)(__ptr)); \ > +}) > + > +#define put_user_force(__x, __ptr) \ > +({ \ > + put_user((typeof(*__x) __force *)(__x), __ptr); \ > +}) > + > +#define assign_in_user_cast(to, from) \ > +({ \ > + typeof(*from) __assign_tmp; \ > + \ > + get_user_cast(__assign_tmp, from) || put_user(__assign_tmp, to);\ > +}) Please add comments for these macros. It's not trivially obvious what they do and why they are needed. > + > + > static long native_ioctl(struct file *file, unsigned int cmd, unsigned long arg) > { > long ret = -ENOIOCTLCMD; > @@ -543,8 +561,7 @@ static int get_v4l2_buffer32(struct v4l2_buffer __user *p64, > return -EFAULT; > > uplane = aux_buf; > - if (put_user((__force struct v4l2_plane *)uplane, > - &p64->m.planes)) > + if (put_user_force(uplane, &p64->m.planes)) > return -EFAULT; > > while (num_planes--) { > @@ -682,7 +699,7 @@ static int get_v4l2_framebuffer32(struct v4l2_framebuffer __user *p64, > > if (!access_ok(VERIFY_READ, p32, sizeof(*p32)) || > get_user(tmp, &p32->base) || > - put_user((void __force *)compat_ptr(tmp), &p64->base) || > + put_user_force(compat_ptr(tmp), &p64->base) || > assign_in_user(&p64->capability, &p32->capability) || > assign_in_user(&p64->flags, &p32->flags) || > copy_in_user(&p64->fmt, &p32->fmt, sizeof(p64->fmt))) > @@ -831,8 +848,7 @@ static int get_v4l2_ext_controls32(struct file *file, > if (aux_space < count * sizeof(*kcontrols)) > return -EFAULT; > kcontrols = aux_buf; > - if (put_user((__force struct v4l2_ext_control *)kcontrols, > - &p64->controls)) > + if (put_user_force(kcontrols, &p64->controls)) > return -EFAULT; > > for (n = 0; n < count; n++) { > @@ -898,12 +914,11 @@ static int put_v4l2_ext_controls32(struct file *file, > unsigned int size = sizeof(*ucontrols); > u32 id; > > - if (get_user(id, (unsigned int __user *)&kcontrols->id) || > + if (get_user_cast(id, &kcontrols->id) || > put_user(id, &ucontrols->id) || > - assign_in_user(&ucontrols->size, > - (unsigned int __user *)&kcontrols->size) || > + assign_in_user_cast(&ucontrols->size, &kcontrols->size) || > copy_in_user(&ucontrols->reserved2, > - (unsigned int __user *)&kcontrols->reserved2, > + (void __user *)&kcontrols->reserved2, I would prefer to see this change merged with patch 4/6 instead. There is no reason to correct it here. > sizeof(ucontrols->reserved2))) > return -EFAULT; > > @@ -916,7 +931,7 @@ static int put_v4l2_ext_controls32(struct file *file, > size -= sizeof(ucontrols->value64); > > if (copy_in_user(ucontrols, > - (unsigned int __user *)kcontrols, size)) > + (void __user *)kcontrols, size)) Ditto for this one. > return -EFAULT; > > ucontrols++; > @@ -970,10 +985,9 @@ static int get_v4l2_edid32(struct v4l2_edid __user *p64, > if (!access_ok(VERIFY_READ, p32, sizeof(*p32)) || > assign_in_user(&p64->pad, &p32->pad) || > assign_in_user(&p64->start_block, &p32->start_block) || > - assign_in_user(&p64->blocks, > - (unsigned char __user *)&p32->blocks) || > + assign_in_user_cast(&p64->blocks, &p32->blocks) || > get_user(tmp, &p32->edid) || > - put_user((void __force *)compat_ptr(tmp), &p64->edid) || > + put_user_force(compat_ptr(tmp), &p64->edid) || > copy_in_user(p64->reserved, p32->reserved, sizeof(p64->reserved))) > return -EFAULT; > return 0; > Regards, Hans ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH RESEND 6/6] media: v4l2-compat-ioctl32: simplify casts 2018-04-19 11:37 ` Hans Verkuil @ 2018-04-19 15:27 ` Mauro Carvalho Chehab 0 siblings, 0 replies; 13+ messages in thread From: Mauro Carvalho Chehab @ 2018-04-19 15:27 UTC (permalink / raw) To: Hans Verkuil Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Hans Verkuil, Sakari Ailus, Daniel Mentz Em Thu, 19 Apr 2018 13:37:52 +0200 Hans Verkuil <hverkuil@xs4all.nl> escreveu: > On 04/19/18 13:15, Mauro Carvalho Chehab wrote: > > Making the cast right for get_user/put_user is not trivial, as > > it needs to ensure that the types are the correct ones. > > > > Improve it by using macros. > > > > Tested with vivid with: > > $ sudo modprobe vivid no_error_inj=1 > > $ v4l2-compliance-32bits -a -s10 >32bits && v4l2-compliance-64bits -a -s10 > 64bits && diff -U0 32bits 64bits > > --- 32bits 2018-04-17 11:18:29.141240772 -0300 > > +++ 64bits 2018-04-17 11:18:40.635282341 -0300 > > @@ -1 +1 @@ > > -v4l2-compliance SHA : bc71e4a67c6fbc5940062843bc41e7c8679634ce, 32 bits > > +v4l2-compliance SHA : bc71e4a67c6fbc5940062843bc41e7c8679634ce, 64 bits > > > > Using the latest version of v4l-utils with this patch applied: > > https://patchwork.linuxtv.org/patch/48746/ > > > > Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> > > --- > > drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 40 ++++++++++++++++++--------- > > 1 file changed, 27 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > > index 8c05dd9660d3..d2f0268427c2 100644 > > --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > > +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > > @@ -30,6 +30,24 @@ > > get_user(__assign_tmp, from) || put_user(__assign_tmp, to); \ > > }) > > > > +#define get_user_cast(__x, __ptr) \ > > +({ \ > > + get_user(__x, (typeof(*__ptr) __user *)(__ptr)); \ > > +}) > > + > > +#define put_user_force(__x, __ptr) \ > > +({ \ > > + put_user((typeof(*__x) __force *)(__x), __ptr); \ > > +}) > > + > > +#define assign_in_user_cast(to, from) \ > > +({ \ > > + typeof(*from) __assign_tmp; \ > > + \ > > + get_user_cast(__assign_tmp, from) || put_user(__assign_tmp, to);\ > > +}) > > Please add comments for these macros. It's not trivially obvious what they > do and why they are needed. Ok. Would the comments below be acceptable? I may eventually post it as a separate patch, adding documentation to some other functions (maybe adding it to some .rst file). diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c index d2f0268427c2..9530661d9b43 100644 --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c @@ -22,7 +22,18 @@ #include <media/v4l2-ctrls.h> #include <media/v4l2-ioctl.h> -/* Use the same argument order as copy_in_user */ +/** + * assign_in_user() - Copy from one __user var to another one + * + * @to: __user var where data will be stored + * @from: __user var were data will be retrieved. + * + * As this code very often needs to allocate userspace memory, it is easier + * to have a macro that will do both get_user() and put_user() at once. + * + * This function complements the macros defined at asm-generic/uaccess.h. + * It uses the same argument order as copy_in_user() + */ #define assign_in_user(to, from) \ ({ \ typeof(*from) __assign_tmp; \ @@ -30,16 +41,57 @@ get_user(__assign_tmp, from) || put_user(__assign_tmp, to); \ }) +/** + * get_user_cast() - Stores at a kernelspace local var the contents from a + * pointer with userspace data that is not tagged with __user. + * + * @__x: var where data will be stored + * @ptr: var were data will be retrieved. + * + * Sometimes, we need to declare a pointer without __user, because it + * comes from a pointer struct field that will be retrieved from userspace + * by the 64-bit native ioctl handler. This function ensures that the + * @ptr will be casted to __user before calling get_user(), in order to + * avoid warnings with static code analyzers like smatch. + */ #define get_user_cast(__x, __ptr) \ ({ \ get_user(__x, (typeof(*__ptr) __user *)(__ptr)); \ }) +/** + * put_user_force() - Stores at the contents of a kernelspace local var + * into an userspace pointer, removing any __user cast. + * + * @__x: var where data will be stored + * @ptr: var were data will be retrieved. + * + * As the compat32 code now handles with 32-bits and 64-bits __user + * structs, sometimes we need to remove the __user atributes from some data, + * by passing __force macro. This function ensures that the + * @ptr will be casted with __force before calling put_user(), in order to + * avoid warnings with static code analyzers like smatch. + */ #define put_user_force(__x, __ptr) \ ({ \ put_user((typeof(*__x) __force *)(__x), __ptr); \ }) +/** + * assign_in_user_cast() - Copy from one __user var to another one + * + * @to: __user var where data will be stored + * @from: var were data will be retrieved that needs to be cast to __user. + * + * As this code very often needs to allocate userspace memory, it is easier + * to have a macro that will do both get_user_cast() and put_user() at once. + * + * This function should be used instead of assign_in_user() when the @from + * variable was not declared as __user. See get_user_cast() for more details. + * + * This function complements the macros defined at asm-generic/uaccess.h. + * It uses the same argument order as copy_in_user() + */ #define assign_in_user_cast(to, from) \ ({ \ typeof(*from) __assign_tmp; \ Thanks, Mauro ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-04-19 15:28 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-19 11:15 [PATCH RESEND 0/6] Remaining COMPILE_TEST and smatch cleanups Mauro Carvalho Chehab 2018-04-19 11:15 ` [PATCH RESEND 1/6] omap: omap-iommu.h: allow building drivers with COMPILE_TEST Mauro Carvalho Chehab 2018-04-19 11:24 ` Hans Verkuil 2018-04-19 11:15 ` [PATCH RESEND 2/6] media: omap3isp: Enable driver compilation on ARM " Mauro Carvalho Chehab 2018-04-19 11:25 ` Hans Verkuil 2018-04-19 11:15 ` [PATCH RESEND 3/6] media: omap3isp: Allow it to build " Mauro Carvalho Chehab 2018-04-19 11:27 ` Hans Verkuil 2018-04-19 11:15 ` [PATCH RESEND 4/6] media: v4l2-compat-ioctl32: fix several __user annotations Mauro Carvalho Chehab 2018-04-19 11:32 ` Hans Verkuil 2018-04-19 11:15 ` [PATCH RESEND 5/6] media: v4l2-compat-ioctl32: better name userspace pointers Mauro Carvalho Chehab 2018-04-19 11:15 ` [PATCH RESEND 6/6] media: v4l2-compat-ioctl32: simplify casts Mauro Carvalho Chehab 2018-04-19 11:37 ` Hans Verkuil 2018-04-19 15:27 ` Mauro Carvalho Chehab
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.