linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] dma-buf: support 32bit DMA_BUF_SET_NAME ioctl
@ 2020-01-14 13:41 Martin Liu
  2020-02-24  3:39 ` Martin Liu
  0 siblings, 1 reply; 4+ messages in thread
From: Martin Liu @ 2020-01-14 13:41 UTC (permalink / raw)
  To: sumit.semwal; +Cc: linux-media, linux-kernel, liumartin, jenhaochen

This commit adds SET_NAME ioctl coversion to
support 32 bit ioctl.

Signed-off-by: Martin Liu <liumartin@google.com>
---
 drivers/dma-buf/dma-buf.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index ce41cd9b758a..a73048b34843 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -25,6 +25,7 @@
 #include <linux/mm.h>
 #include <linux/mount.h>
 #include <linux/pseudo_fs.h>
+#include <linux/compat.h>
 
 #include <uapi/linux/dma-buf.h>
 #include <uapi/linux/magic.h>
@@ -409,13 +410,32 @@ static void dma_buf_show_fdinfo(struct seq_file *m, struct file *file)
 	dma_resv_unlock(dmabuf->resv);
 }
 
+#ifdef CONFIG_COMPAT
+static long dma_buf_ioctl_compat(struct file *file, unsigned int cmd,
+				 unsigned long arg)
+{
+	switch (_IOC_NR(cmd)) {
+	case _IOC_NR(DMA_BUF_SET_NAME):
+		/* Fix up pointer size*/
+		if (_IOC_SIZE(cmd) == sizeof(compat_uptr_t)) {
+			cmd &= ~IOCSIZE_MASK;
+			cmd |= sizeof(void *) << IOCSIZE_SHIFT;
+		}
+		break;
+	}
+	return dma_buf_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
+}
+#endif
+
 static const struct file_operations dma_buf_fops = {
 	.release	= dma_buf_release,
 	.mmap		= dma_buf_mmap_internal,
 	.llseek		= dma_buf_llseek,
 	.poll		= dma_buf_poll,
 	.unlocked_ioctl	= dma_buf_ioctl,
-	.compat_ioctl	= compat_ptr_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl	= dma_buf_ioctl_compat,
+#endif
 	.show_fdinfo	= dma_buf_show_fdinfo,
 };
 
-- 
2.25.0.rc1.283.g88dfdc4193-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] dma-buf: support 32bit DMA_BUF_SET_NAME ioctl
  2020-01-14 13:41 [PATCH] dma-buf: support 32bit DMA_BUF_SET_NAME ioctl Martin Liu
@ 2020-02-24  3:39 ` Martin Liu
  2020-04-07 13:26   ` Sumit Semwal
  0 siblings, 1 reply; 4+ messages in thread
From: Martin Liu @ 2020-02-24  3:39 UTC (permalink / raw)
  To: sumit.semwal, minchan, surenb, wvw
  Cc: linux-media, dri-devel, linaro-mm-sig, linux-kernel, liumartin,
	jenhaochen

On Tue, Jan 14, 2020 at 09:41:01PM +0800, Martin Liu wrote:

CC more MLs for winder review.

> This commit adds SET_NAME ioctl coversion to
> support 32 bit ioctl.
> 
> Signed-off-by: Martin Liu <liumartin@google.com>
> ---
>  drivers/dma-buf/dma-buf.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index ce41cd9b758a..a73048b34843 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -25,6 +25,7 @@
>  #include <linux/mm.h>
>  #include <linux/mount.h>
>  #include <linux/pseudo_fs.h>
> +#include <linux/compat.h>
>  
>  #include <uapi/linux/dma-buf.h>
>  #include <uapi/linux/magic.h>
> @@ -409,13 +410,32 @@ static void dma_buf_show_fdinfo(struct seq_file *m, struct file *file)
>  	dma_resv_unlock(dmabuf->resv);
>  }
>  
> +#ifdef CONFIG_COMPAT
> +static long dma_buf_ioctl_compat(struct file *file, unsigned int cmd,
> +				 unsigned long arg)
> +{
> +	switch (_IOC_NR(cmd)) {
> +	case _IOC_NR(DMA_BUF_SET_NAME):
> +		/* Fix up pointer size*/
> +		if (_IOC_SIZE(cmd) == sizeof(compat_uptr_t)) {
> +			cmd &= ~IOCSIZE_MASK;
> +			cmd |= sizeof(void *) << IOCSIZE_SHIFT;
> +		}
> +		break;
> +	}
> +	return dma_buf_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
> +}
> +#endif
> +
>  static const struct file_operations dma_buf_fops = {
>  	.release	= dma_buf_release,
>  	.mmap		= dma_buf_mmap_internal,
>  	.llseek		= dma_buf_llseek,
>  	.poll		= dma_buf_poll,
>  	.unlocked_ioctl	= dma_buf_ioctl,
> -	.compat_ioctl	= compat_ptr_ioctl,
> +#ifdef CONFIG_COMPAT
> +	.compat_ioctl	= dma_buf_ioctl_compat,
> +#endif
>  	.show_fdinfo	= dma_buf_show_fdinfo,
>  };
>  
> -- 
> 2.25.0.rc1.283.g88dfdc4193-goog
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] dma-buf: support 32bit DMA_BUF_SET_NAME ioctl
  2020-02-24  3:39 ` Martin Liu
@ 2020-04-07 13:26   ` Sumit Semwal
  2020-04-07 14:05     ` Daniel Vetter
  0 siblings, 1 reply; 4+ messages in thread
From: Sumit Semwal @ 2020-04-07 13:26 UTC (permalink / raw)
  To: Martin Liu, Daniel Vetter
  Cc: minchan, surenb, wvw, open list:DMA BUFFER SHARING FRAMEWORK,
	DRI mailing list, Linaro MM SIG, LKML, jenhaochen

Hello,

Daniel, your comments here, please?

On Mon, 24 Feb 2020 at 09:09, Martin Liu <liumartin@google.com> wrote:
>
> On Tue, Jan 14, 2020 at 09:41:01PM +0800, Martin Liu wrote:
>
> CC more MLs for winder review.
>
> > This commit adds SET_NAME ioctl coversion to
> > support 32 bit ioctl.
> >
> > Signed-off-by: Martin Liu <liumartin@google.com>
> > ---
> >  drivers/dma-buf/dma-buf.c | 22 +++++++++++++++++++++-
> >  1 file changed, 21 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > index ce41cd9b758a..a73048b34843 100644
> > --- a/drivers/dma-buf/dma-buf.c
> > +++ b/drivers/dma-buf/dma-buf.c
> > @@ -25,6 +25,7 @@
> >  #include <linux/mm.h>
> >  #include <linux/mount.h>
> >  #include <linux/pseudo_fs.h>
> > +#include <linux/compat.h>
> >
> >  #include <uapi/linux/dma-buf.h>
> >  #include <uapi/linux/magic.h>
> > @@ -409,13 +410,32 @@ static void dma_buf_show_fdinfo(struct seq_file *m, struct file *file)
> >       dma_resv_unlock(dmabuf->resv);
> >  }
> >
> > +#ifdef CONFIG_COMPAT
> > +static long dma_buf_ioctl_compat(struct file *file, unsigned int cmd,
> > +                              unsigned long arg)
> > +{
> > +     switch (_IOC_NR(cmd)) {
> > +     case _IOC_NR(DMA_BUF_SET_NAME):
> > +             /* Fix up pointer size*/
> > +             if (_IOC_SIZE(cmd) == sizeof(compat_uptr_t)) {
> > +                     cmd &= ~IOCSIZE_MASK;
> > +                     cmd |= sizeof(void *) << IOCSIZE_SHIFT;
> > +             }
> > +             break;
> > +     }
> > +     return dma_buf_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
> > +}
> > +#endif
> > +
> >  static const struct file_operations dma_buf_fops = {
> >       .release        = dma_buf_release,
> >       .mmap           = dma_buf_mmap_internal,
> >       .llseek         = dma_buf_llseek,
> >       .poll           = dma_buf_poll,
> >       .unlocked_ioctl = dma_buf_ioctl,
> > -     .compat_ioctl   = compat_ptr_ioctl,
> > +#ifdef CONFIG_COMPAT
> > +     .compat_ioctl   = dma_buf_ioctl_compat,
> > +#endif
> >       .show_fdinfo    = dma_buf_show_fdinfo,
> >  };
> >
> > --
> > 2.25.0.rc1.283.g88dfdc4193-goog
> >



-- 
Thanks and regards,

Sumit Semwal
Linaro Consumer Group - Kernel Team Lead
Linaro.org │ Open source software for ARM SoCs

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] dma-buf: support 32bit DMA_BUF_SET_NAME ioctl
  2020-04-07 13:26   ` Sumit Semwal
@ 2020-04-07 14:05     ` Daniel Vetter
  0 siblings, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2020-04-07 14:05 UTC (permalink / raw)
  To: Sumit Semwal
  Cc: Martin Liu, Minchan Kim, surenb, Wei Wang,
	open list:DMA BUFFER SHARING FRAMEWORK, DRI mailing list,
	Linaro MM SIG, LKML, jenhaochen

On Tue, Apr 7, 2020 at 3:26 PM Sumit Semwal <sumit.semwal@linaro.org> wrote:
>
> Hello,
>
> Daniel, your comments here, please?

Don't :-)

I mean rule of thumb you should never need a compat_ioctl for a new
ioctl, that's just failure to read
https://www.kernel.org/doc/html/v5.4-preprc-cpu/ioctl/botching-up-ioctls.html

Specifically the char * pointer in the ioctl definition is the
problem. Now we're somewhat lucky here, since the actual layout of the
data isn't different between 32 and 64 bit, it's only the ioctl. Which
is the 2nd issue, the type in there should be the type of the data in
userspace, not the type of the _pointer_ to the stuff in userspace. So
here actually variable-sized char[] array, which also doesn't work
really.

Anyway I've created a quick patch to have distinct ioctl number
defines and handle both in the main handler, that should work. Cc'ed
everyone from this thread, please test.
-Daniel

>
> On Mon, 24 Feb 2020 at 09:09, Martin Liu <liumartin@google.com> wrote:
> >
> > On Tue, Jan 14, 2020 at 09:41:01PM +0800, Martin Liu wrote:
> >
> > CC more MLs for winder review.
> >
> > > This commit adds SET_NAME ioctl coversion to
> > > support 32 bit ioctl.
> > >
> > > Signed-off-by: Martin Liu <liumartin@google.com>
> > > ---
> > >  drivers/dma-buf/dma-buf.c | 22 +++++++++++++++++++++-
> > >  1 file changed, 21 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > > index ce41cd9b758a..a73048b34843 100644
> > > --- a/drivers/dma-buf/dma-buf.c
> > > +++ b/drivers/dma-buf/dma-buf.c
> > > @@ -25,6 +25,7 @@
> > >  #include <linux/mm.h>
> > >  #include <linux/mount.h>
> > >  #include <linux/pseudo_fs.h>
> > > +#include <linux/compat.h>
> > >
> > >  #include <uapi/linux/dma-buf.h>
> > >  #include <uapi/linux/magic.h>
> > > @@ -409,13 +410,32 @@ static void dma_buf_show_fdinfo(struct seq_file *m, struct file *file)
> > >       dma_resv_unlock(dmabuf->resv);
> > >  }
> > >
> > > +#ifdef CONFIG_COMPAT
> > > +static long dma_buf_ioctl_compat(struct file *file, unsigned int cmd,
> > > +                              unsigned long arg)
> > > +{
> > > +     switch (_IOC_NR(cmd)) {
> > > +     case _IOC_NR(DMA_BUF_SET_NAME):
> > > +             /* Fix up pointer size*/
> > > +             if (_IOC_SIZE(cmd) == sizeof(compat_uptr_t)) {
> > > +                     cmd &= ~IOCSIZE_MASK;
> > > +                     cmd |= sizeof(void *) << IOCSIZE_SHIFT;
> > > +             }
> > > +             break;
> > > +     }
> > > +     return dma_buf_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
> > > +}
> > > +#endif
> > > +
> > >  static const struct file_operations dma_buf_fops = {
> > >       .release        = dma_buf_release,
> > >       .mmap           = dma_buf_mmap_internal,
> > >       .llseek         = dma_buf_llseek,
> > >       .poll           = dma_buf_poll,
> > >       .unlocked_ioctl = dma_buf_ioctl,
> > > -     .compat_ioctl   = compat_ptr_ioctl,
> > > +#ifdef CONFIG_COMPAT
> > > +     .compat_ioctl   = dma_buf_ioctl_compat,
> > > +#endif
> > >       .show_fdinfo    = dma_buf_show_fdinfo,
> > >  };
> > >
> > > --
> > > 2.25.0.rc1.283.g88dfdc4193-goog
> > >
>
>
>
> --
> Thanks and regards,
>
> Sumit Semwal
> Linaro Consumer Group - Kernel Team Lead
> Linaro.org │ Open source software for ARM SoCs



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-04-07 14:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-14 13:41 [PATCH] dma-buf: support 32bit DMA_BUF_SET_NAME ioctl Martin Liu
2020-02-24  3:39 ` Martin Liu
2020-04-07 13:26   ` Sumit Semwal
2020-04-07 14:05     ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).