fbdev sbuslib cleanups

fbdev sbuslib cleanups

[Christoph Hellwig]

Hi all,

this series cleans up the fbdev sbuslib support, most importantly by
removing usage of the deprecated compat_alloc_user_space API.

fbdev sbuslib cleanups

[Christoph Hellwig]

Hi all,

this series cleans up the fbdev sbuslib support, most importantly by
removing usage of the deprecated compat_alloc_user_space API.

[PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers

[Christoph Hellwig]

There are no actual implementations of FBIOSCURSOR/FBIOGCURSOR left, so
don't bother with a compat handler for it, and remove the remaining
definitions as well.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/m68k/include/asm/fbio.h       | 29 -----------------------------
 arch/sparc/include/asm/fbio.h      | 13 -------------
 arch/sparc/include/uapi/asm/fbio.h | 15 ---------------
 drivers/video/fbdev/sbuslib.c      | 30 ------------------------------
 4 files changed, 87 deletions(-)

diff --git a/arch/m68k/include/asm/fbio.h b/arch/m68k/include/asm/fbio.h
index 590b923c470d3e..90ae409c6bdb4e 100644
--- a/arch/m68k/include/asm/fbio.h
+++ b/arch/m68k/include/asm/fbio.h
@@ -97,21 +97,6 @@ struct fbgattr {
 #define FBIOSVIDEO _IOW('F', 7, int)
 #define FBIOGVIDEO _IOR('F', 8, int)
 
-struct fbcursor {
-        short set;              /* what to set, choose from the list above */
-        short enable;           /* cursor on/off */
-        struct fbcurpos pos;    /* cursor position */
-        struct fbcurpos hot;    /* cursor hot spot */
-        struct fbcmap cmap;     /* color map info */
-        struct fbcurpos size;   /* cursor bit map size */
-        char __user *image;     /* cursor image bits */
-        char __user *mask;      /* cursor mask bits */
-};
-
-/* set/get cursor attributes/shape */
-#define FBIOSCURSOR     _IOW('F', 24, struct fbcursor)
-#define FBIOGCURSOR     _IOWR('F', 25, struct fbcursor)
- 
 /* set/get cursor position */
 #define FBIOSCURPOS     _IOW('F', 26, struct fbcurpos)
 #define FBIOGCURPOS     _IOW('F', 27, struct fbcurpos)
@@ -312,20 +297,6 @@ struct  fbcmap32 {
 
 #define FBIOPUTCMAP32	_IOW('F', 3, struct fbcmap32)
 #define FBIOGETCMAP32	_IOW('F', 4, struct fbcmap32)
-
-struct fbcursor32 {
-	short set;		/* what to set, choose from the list above */
-	short enable;		/* cursor on/off */
-	struct fbcurpos pos;	/* cursor position */
-	struct fbcurpos hot;	/* cursor hot spot */
-	struct fbcmap32 cmap;	/* color map info */
-	struct fbcurpos size;	/* cursor bit map size */
-	u32	image;		/* cursor image bits */
-	u32	mask;		/* cursor mask bits */
-};
-
-#define FBIOSCURSOR32	_IOW('F', 24, struct fbcursor32)
-#define FBIOGCURSOR32	_IOW('F', 25, struct fbcursor32)
 #endif
 
 #endif /* __LINUX_FBIO_H */
diff --git a/arch/sparc/include/asm/fbio.h b/arch/sparc/include/asm/fbio.h
index 02654cb95dec57..348994cc142973 100644
--- a/arch/sparc/include/asm/fbio.h
+++ b/arch/sparc/include/asm/fbio.h
@@ -57,17 +57,4 @@ struct  fbcmap32 {
 #define FBIOPUTCMAP32	_IOW('F', 3, struct fbcmap32)
 #define FBIOGETCMAP32	_IOW('F', 4, struct fbcmap32)
 
-struct fbcursor32 {
-	short set;		/* what to set, choose from the list above */
-	short enable;		/* cursor on/off */
-	struct fbcurpos pos;	/* cursor position */
-	struct fbcurpos hot;	/* cursor hot spot */
-	struct fbcmap32 cmap;	/* color map info */
-	struct fbcurpos size;	/* cursor bit map size */
-	u32	image;		/* cursor image bits */
-	u32	mask;		/* cursor mask bits */
-};
-
-#define FBIOSCURSOR32	_IOW('F', 24, struct fbcursor32)
-#define FBIOGCURSOR32	_IOW('F', 25, struct fbcursor32)
 #endif /* __LINUX_FBIO_H */
diff --git a/arch/sparc/include/uapi/asm/fbio.h b/arch/sparc/include/uapi/asm/fbio.h
index 0dafe2c1eab740..bda278c2a7d091 100644
--- a/arch/sparc/include/uapi/asm/fbio.h
+++ b/arch/sparc/include/uapi/asm/fbio.h
@@ -94,21 +94,6 @@ struct fbgattr {
 #define FBIOSVIDEO _IOW('F', 7, int)
 #define FBIOGVIDEO _IOR('F', 8, int)
 
-struct fbcursor {
-        short set;              /* what to set, choose from the list above */
-        short enable;           /* cursor on/off */
-        struct fbcurpos pos;    /* cursor position */
-        struct fbcurpos hot;    /* cursor hot spot */
-        struct fbcmap cmap;     /* color map info */
-        struct fbcurpos size;   /* cursor bit map size */
-        char __user *image;     /* cursor image bits */
-        char __user *mask;      /* cursor mask bits */
-};
-
-/* set/get cursor attributes/shape */
-#define FBIOSCURSOR     _IOW('F', 24, struct fbcursor)
-#define FBIOGCURSOR     _IOWR('F', 25, struct fbcursor)
- 
 /* set/get cursor position */
 #define FBIOSCURPOS     _IOW('F', 26, struct fbcurpos)
 #define FBIOGCURPOS     _IOW('F', 27, struct fbcurpos)
diff --git a/drivers/video/fbdev/sbuslib.c b/drivers/video/fbdev/sbuslib.c
index 01a7110e61a76a..176dbfb5d3efca 100644
--- a/drivers/video/fbdev/sbuslib.c
+++ b/drivers/video/fbdev/sbuslib.c
@@ -214,32 +214,6 @@ static int fbiogetputcmap(struct fb_info *info, unsigned int cmd, unsigned long
 			(unsigned long)p);
 }
 
-static int fbiogscursor(struct fb_info *info, unsigned long arg)
-{
-	struct fbcursor __user *p = compat_alloc_user_space(sizeof(*p));
-	struct fbcursor32 __user *argp =  (void __user *)arg;
-	compat_uptr_t addr;
-	int ret;
-
-	ret = copy_in_user(p, argp,
-			      2 * sizeof (short) + 2 * sizeof(struct fbcurpos));
-	ret |= copy_in_user(&p->size, &argp->size, sizeof(struct fbcurpos));
-	ret |= copy_in_user(&p->cmap, &argp->cmap, 2 * sizeof(int));
-	ret |= get_user(addr, &argp->cmap.red);
-	ret |= put_user(compat_ptr(addr), &p->cmap.red);
-	ret |= get_user(addr, &argp->cmap.green);
-	ret |= put_user(compat_ptr(addr), &p->cmap.green);
-	ret |= get_user(addr, &argp->cmap.blue);
-	ret |= put_user(compat_ptr(addr), &p->cmap.blue);
-	ret |= get_user(addr, &argp->mask);
-	ret |= put_user(compat_ptr(addr), &p->mask);
-	ret |= get_user(addr, &argp->image);
-	ret |= put_user(compat_ptr(addr), &p->image);
-	if (ret)
-		return -EFAULT;
-	return info->fbops->fb_ioctl(info, FBIOSCURSOR, (unsigned long)p);
-}
-
 int sbusfb_compat_ioctl(struct fb_info *info, unsigned int cmd, unsigned long arg)
 {
 	switch (cmd) {
@@ -248,8 +222,6 @@ int sbusfb_compat_ioctl(struct fb_info *info, unsigned int cmd, unsigned long ar
 	case FBIOGATTR:
 	case FBIOSVIDEO:
 	case FBIOGVIDEO:
-	case FBIOGCURSOR32:	/* This is not implemented yet.
-				   Later it should be converted... */
 	case FBIOSCURPOS:
 	case FBIOGCURPOS:
 	case FBIOGCURMAX:
@@ -258,8 +230,6 @@ int sbusfb_compat_ioctl(struct fb_info *info, unsigned int cmd, unsigned long ar
 		return fbiogetputcmap(info, cmd, arg);
 	case FBIOGETCMAP32:
 		return fbiogetputcmap(info, cmd, arg);
-	case FBIOSCURSOR32:
-		return fbiogscursor(info, arg);
 	default:
 		return -ENOIOCTLCMD;
 	}
-- 
2.28.0

[PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers

[Christoph Hellwig]

There are no actual implementations of FBIOSCURSOR/FBIOGCURSOR left, so
don't bother with a compat handler for it, and remove the remaining
definitions as well.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/m68k/include/asm/fbio.h       | 29 -----------------------------
 arch/sparc/include/asm/fbio.h      | 13 -------------
 arch/sparc/include/uapi/asm/fbio.h | 15 ---------------
 drivers/video/fbdev/sbuslib.c      | 30 ------------------------------
 4 files changed, 87 deletions(-)

diff --git a/arch/m68k/include/asm/fbio.h b/arch/m68k/include/asm/fbio.h
index 590b923c470d3e..90ae409c6bdb4e 100644
--- a/arch/m68k/include/asm/fbio.h
+++ b/arch/m68k/include/asm/fbio.h
@@ -97,21 +97,6 @@ struct fbgattr {
 #define FBIOSVIDEO _IOW('F', 7, int)
 #define FBIOGVIDEO _IOR('F', 8, int)
 
-struct fbcursor {
-        short set;              /* what to set, choose from the list above */
-        short enable;           /* cursor on/off */
-        struct fbcurpos pos;    /* cursor position */
-        struct fbcurpos hot;    /* cursor hot spot */
-        struct fbcmap cmap;     /* color map info */
-        struct fbcurpos size;   /* cursor bit map size */
-        char __user *image;     /* cursor image bits */
-        char __user *mask;      /* cursor mask bits */
-};
-
-/* set/get cursor attributes/shape */
-#define FBIOSCURSOR     _IOW('F', 24, struct fbcursor)
-#define FBIOGCURSOR     _IOWR('F', 25, struct fbcursor)
- 
 /* set/get cursor position */
 #define FBIOSCURPOS     _IOW('F', 26, struct fbcurpos)
 #define FBIOGCURPOS     _IOW('F', 27, struct fbcurpos)
@@ -312,20 +297,6 @@ struct  fbcmap32 {
 
 #define FBIOPUTCMAP32	_IOW('F', 3, struct fbcmap32)
 #define FBIOGETCMAP32	_IOW('F', 4, struct fbcmap32)
-
-struct fbcursor32 {
-	short set;		/* what to set, choose from the list above */
-	short enable;		/* cursor on/off */
-	struct fbcurpos pos;	/* cursor position */
-	struct fbcurpos hot;	/* cursor hot spot */
-	struct fbcmap32 cmap;	/* color map info */
-	struct fbcurpos size;	/* cursor bit map size */
-	u32	image;		/* cursor image bits */
-	u32	mask;		/* cursor mask bits */
-};
-
-#define FBIOSCURSOR32	_IOW('F', 24, struct fbcursor32)
-#define FBIOGCURSOR32	_IOW('F', 25, struct fbcursor32)
 #endif
 
 #endif /* __LINUX_FBIO_H */
diff --git a/arch/sparc/include/asm/fbio.h b/arch/sparc/include/asm/fbio.h
index 02654cb95dec57..348994cc142973 100644
--- a/arch/sparc/include/asm/fbio.h
+++ b/arch/sparc/include/asm/fbio.h
@@ -57,17 +57,4 @@ struct  fbcmap32 {
 #define FBIOPUTCMAP32	_IOW('F', 3, struct fbcmap32)
 #define FBIOGETCMAP32	_IOW('F', 4, struct fbcmap32)
 
-struct fbcursor32 {
-	short set;		/* what to set, choose from the list above */
-	short enable;		/* cursor on/off */
-	struct fbcurpos pos;	/* cursor position */
-	struct fbcurpos hot;	/* cursor hot spot */
-	struct fbcmap32 cmap;	/* color map info */
-	struct fbcurpos size;	/* cursor bit map size */
-	u32	image;		/* cursor image bits */
-	u32	mask;		/* cursor mask bits */
-};
-
-#define FBIOSCURSOR32	_IOW('F', 24, struct fbcursor32)
-#define FBIOGCURSOR32	_IOW('F', 25, struct fbcursor32)
 #endif /* __LINUX_FBIO_H */
diff --git a/arch/sparc/include/uapi/asm/fbio.h b/arch/sparc/include/uapi/asm/fbio.h
index 0dafe2c1eab740..bda278c2a7d091 100644
--- a/arch/sparc/include/uapi/asm/fbio.h
+++ b/arch/sparc/include/uapi/asm/fbio.h
@@ -94,21 +94,6 @@ struct fbgattr {
 #define FBIOSVIDEO _IOW('F', 7, int)
 #define FBIOGVIDEO _IOR('F', 8, int)
 
-struct fbcursor {
-        short set;              /* what to set, choose from the list above */
-        short enable;           /* cursor on/off */
-        struct fbcurpos pos;    /* cursor position */
-        struct fbcurpos hot;    /* cursor hot spot */
-        struct fbcmap cmap;     /* color map info */
-        struct fbcurpos size;   /* cursor bit map size */
-        char __user *image;     /* cursor image bits */
-        char __user *mask;      /* cursor mask bits */
-};
-
-/* set/get cursor attributes/shape */
-#define FBIOSCURSOR     _IOW('F', 24, struct fbcursor)
-#define FBIOGCURSOR     _IOWR('F', 25, struct fbcursor)
- 
 /* set/get cursor position */
 #define FBIOSCURPOS     _IOW('F', 26, struct fbcurpos)
 #define FBIOGCURPOS     _IOW('F', 27, struct fbcurpos)
diff --git a/drivers/video/fbdev/sbuslib.c b/drivers/video/fbdev/sbuslib.c
index 01a7110e61a76a..176dbfb5d3efca 100644
--- a/drivers/video/fbdev/sbuslib.c
+++ b/drivers/video/fbdev/sbuslib.c
@@ -214,32 +214,6 @@ static int fbiogetputcmap(struct fb_info *info, unsigned int cmd, unsigned long
 			(unsigned long)p);
 }
 
-static int fbiogscursor(struct fb_info *info, unsigned long arg)
-{
-	struct fbcursor __user *p = compat_alloc_user_space(sizeof(*p));
-	struct fbcursor32 __user *argp =  (void __user *)arg;
-	compat_uptr_t addr;
-	int ret;
-
-	ret = copy_in_user(p, argp,
-			      2 * sizeof (short) + 2 * sizeof(struct fbcurpos));
-	ret |= copy_in_user(&p->size, &argp->size, sizeof(struct fbcurpos));
-	ret |= copy_in_user(&p->cmap, &argp->cmap, 2 * sizeof(int));
-	ret |= get_user(addr, &argp->cmap.red);
-	ret |= put_user(compat_ptr(addr), &p->cmap.red);
-	ret |= get_user(addr, &argp->cmap.green);
-	ret |= put_user(compat_ptr(addr), &p->cmap.green);
-	ret |= get_user(addr, &argp->cmap.blue);
-	ret |= put_user(compat_ptr(addr), &p->cmap.blue);
-	ret |= get_user(addr, &argp->mask);
-	ret |= put_user(compat_ptr(addr), &p->mask);
-	ret |= get_user(addr, &argp->image);
-	ret |= put_user(compat_ptr(addr), &p->image);
-	if (ret)
-		return -EFAULT;
-	return info->fbops->fb_ioctl(info, FBIOSCURSOR, (unsigned long)p);
-}
-
 int sbusfb_compat_ioctl(struct fb_info *info, unsigned int cmd, unsigned long arg)
 {
 	switch (cmd) {
@@ -248,8 +222,6 @@ int sbusfb_compat_ioctl(struct fb_info *info, unsigned int cmd, unsigned long ar
 	case FBIOGATTR:
 	case FBIOSVIDEO:
 	case FBIOGVIDEO:
-	case FBIOGCURSOR32:	/* This is not implemented yet.
-				   Later it should be converted... */
 	case FBIOSCURPOS:
 	case FBIOGCURPOS:
 	case FBIOGCURMAX:
@@ -258,8 +230,6 @@ int sbusfb_compat_ioctl(struct fb_info *info, unsigned int cmd, unsigned long ar
 		return fbiogetputcmap(info, cmd, arg);
 	case FBIOGETCMAP32:
 		return fbiogetputcmap(info, cmd, arg);
-	case FBIOSCURSOR32:
-		return fbiogscursor(info, arg);
 	default:
 		return -ENOIOCTLCMD;
 	}
-- 
2.28.0

Re: [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers

[Geert Uytterhoeven]

On Wed, Oct 7, 2020 at 9:44 AM Christoph Hellwig <hch@lst.de> wrote:
> There are no actual implementations of FBIOSCURSOR/FBIOGCURSOR left, so
> don't bother with a compat handler for it, and remove the remaining
> definitions as well.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers

[Geert Uytterhoeven]

On Wed, Oct 7, 2020 at 9:44 AM Christoph Hellwig <hch@lst.de> wrote:
> There are no actual implementations of FBIOSCURSOR/FBIOGCURSOR left, so
> don't bother with a compat handler for it, and remove the remaining
> definitions as well.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers

[Arnd Bergmann]

On Wed, Oct 7, 2020 at 9:44 AM Christoph Hellwig <hch@lst.de> wrote:
>
> There are no actual implementations of FBIOSCURSOR/FBIOGCURSOR left, so
> don't bother with a compat handler for it, and remove the remaining
> definitions as well.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

I had submitted a similar patch earlier, and Sam Ravnborg applied it to the
drm-misc tree, but it doesn't seem to be in linux-next, so I don't know
what the state is.

My version only removed the compat handling, not the data structures,
so I'm happy to see your version used instead if mine got lost.

Acked-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers

[Arnd Bergmann]

On Wed, Oct 7, 2020 at 9:44 AM Christoph Hellwig <hch@lst.de> wrote:
>
> There are no actual implementations of FBIOSCURSOR/FBIOGCURSOR left, so
> don't bother with a compat handler for it, and remove the remaining
> definitions as well.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

I had submitted a similar patch earlier, and Sam Ravnborg applied it to the
drm-misc tree, but it doesn't seem to be in linux-next, so I don't know
what the state is.

My version only removed the compat handling, not the data structures,
so I'm happy to see your version used instead if mine got lost.

Acked-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers

[Christoph Hellwig]

On Wed, Oct 07, 2020 at 10:54:19AM +0200, Arnd Bergmann wrote:
> On Wed, Oct 7, 2020 at 9:44 AM Christoph Hellwig <hch@lst.de> wrote:
> >
> > There are no actual implementations of FBIOSCURSOR/FBIOGCURSOR left, so
> > don't bother with a compat handler for it, and remove the remaining
> > definitions as well.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> I had submitted a similar patch earlier, and Sam Ravnborg applied it to the
> drm-misc tree, but it doesn't seem to be in linux-next, so I don't know
> what the state is.
> 
> My version only removed the compat handling, not the data structures,
> so I'm happy to see your version used instead if mine got lost.

Oh, sorry.  I thought in your summary you decided to give up on
the sbuslib ones.

Re: [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers

[Christoph Hellwig]

On Wed, Oct 07, 2020 at 10:54:19AM +0200, Arnd Bergmann wrote:
> On Wed, Oct 7, 2020 at 9:44 AM Christoph Hellwig <hch@lst.de> wrote:
> >
> > There are no actual implementations of FBIOSCURSOR/FBIOGCURSOR left, so
> > don't bother with a compat handler for it, and remove the remaining
> > definitions as well.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> I had submitted a similar patch earlier, and Sam Ravnborg applied it to the
> drm-misc tree, but it doesn't seem to be in linux-next, so I don't know
> what the state is.
> 
> My version only removed the compat handling, not the data structures,
> so I'm happy to see your version used instead if mine got lost.

Oh, sorry.  I thought in your summary you decided to give up on
the sbuslib ones.

Re: [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers

[Arnd Bergmann]

On Wed, Oct 7, 2020 at 10:59 AM Christoph Hellwig <hch@lst.de> wrote:
> On Wed, Oct 07, 2020 at 10:54:19AM +0200, Arnd Bergmann wrote:
> > On Wed, Oct 7, 2020 at 9:44 AM Christoph Hellwig <hch@lst.de> wrote:
> > >
> > > There are no actual implementations of FBIOSCURSOR/FBIOGCURSOR left, so
> > > don't bother with a compat handler for it, and remove the remaining
> > > definitions as well.
> > >
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> >
> > I had submitted a similar patch earlier, and Sam Ravnborg applied it to the
> > drm-misc tree, but it doesn't seem to be in linux-next, so I don't know
> > what the state is.
> >
> > My version only removed the compat handling, not the data structures,
> > so I'm happy to see your version used instead if mine got lost.
>
> Oh, sorry.  I thought in your summary you decided to give up on
> the sbuslib ones.

Here is what I have pending at the moment for set_fs() and
compat_alloc_user_space():

https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git/log/?h=compat-alloc-user-space-2

The only one I have actually given up on is the atomisp staging driver,
which uses an awful hack, copying the x86 implementation of
copy_in_user()/compat_alloc_user_space() into the driver.

This is based on last week's linux-next, as I plan to resubmit after the
merge window.

     Arnd

Re: [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers

[Arnd Bergmann]

On Wed, Oct 7, 2020 at 10:59 AM Christoph Hellwig <hch@lst.de> wrote:
> On Wed, Oct 07, 2020 at 10:54:19AM +0200, Arnd Bergmann wrote:
> > On Wed, Oct 7, 2020 at 9:44 AM Christoph Hellwig <hch@lst.de> wrote:
> > >
> > > There are no actual implementations of FBIOSCURSOR/FBIOGCURSOR left, so
> > > don't bother with a compat handler for it, and remove the remaining
> > > definitions as well.
> > >
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> >
> > I had submitted a similar patch earlier, and Sam Ravnborg applied it to the
> > drm-misc tree, but it doesn't seem to be in linux-next, so I don't know
> > what the state is.
> >
> > My version only removed the compat handling, not the data structures,
> > so I'm happy to see your version used instead if mine got lost.
>
> Oh, sorry.  I thought in your summary you decided to give up on
> the sbuslib ones.

Here is what I have pending at the moment for set_fs() and
compat_alloc_user_space():

https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git/log/?h=compat-alloc-user-space-2

The only one I have actually given up on is the atomisp staging driver,
which uses an awful hack, copying the x86 implementation of
copy_in_user()/compat_alloc_user_space() into the driver.

This is based on last week's linux-next, as I plan to resubmit after the
merge window.

     Arnd

Re: [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers

[Christoph Hellwig]

On Wed, Oct 07, 2020 at 11:28:58AM +0200, Arnd Bergmann wrote:
> The only one I have actually given up on is the atomisp staging driver,
> which uses an awful hack, copying the x86 implementation of
> copy_in_user()/compat_alloc_user_space() into the driver.

That is gross.  Just mark the driver as broken for now.  Linus agreed
years ago that we don't need to work around staging drivers.  But
it would still be nice to ping the mainainers, as they often have
better ideas how to solve the problem.

Re: [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers

[Christoph Hellwig]

On Wed, Oct 07, 2020 at 11:28:58AM +0200, Arnd Bergmann wrote:
> The only one I have actually given up on is the atomisp staging driver,
> which uses an awful hack, copying the x86 implementation of
> copy_in_user()/compat_alloc_user_space() into the driver.

That is gross.  Just mark the driver as broken for now.  Linus agreed
years ago that we don't need to work around staging drivers.  But
it would still be nice to ping the mainainers, as they often have
better ideas how to solve the problem.

Re: [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers

[Arnd Bergmann]

On Wed, Oct 7, 2020 at 12:40 PM Christoph Hellwig <hch@lst.de> wrote:
>
> On Wed, Oct 07, 2020 at 11:28:58AM +0200, Arnd Bergmann wrote:
> > The only one I have actually given up on is the atomisp staging driver,
> > which uses an awful hack, copying the x86 implementation of
> > copy_in_user()/compat_alloc_user_space() into the driver.
>
> That is gross.  Just mark the driver as broken for now.

Ah, it seems that Hans has already done that in commit 57e6b6f2303e
("media: atomisp_fops.c: disable atomisp_compat_ioctl32"), which
removes the only reference to this code. In this case, I think a one-line
change to stop building that file would be best, then if anyone ever
wants to fix the bug that Hans and Sakari found, they get to do also
deal with replacing compat_alloc_user_space().

I'll send a patch for that right away, the patch I have in my tree was
so evil that I hadn't dared submit that, but it was useful for
compile-testing the compat_alloc_user_space() removal patch.

   Arnd

Re: [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers

[Arnd Bergmann]

On Wed, Oct 7, 2020 at 12:40 PM Christoph Hellwig <hch@lst.de> wrote:
>
> On Wed, Oct 07, 2020 at 11:28:58AM +0200, Arnd Bergmann wrote:
> > The only one I have actually given up on is the atomisp staging driver,
> > which uses an awful hack, copying the x86 implementation of
> > copy_in_user()/compat_alloc_user_space() into the driver.
>
> That is gross.  Just mark the driver as broken for now.

Ah, it seems that Hans has already done that in commit 57e6b6f2303e
("media: atomisp_fops.c: disable atomisp_compat_ioctl32"), which
removes the only reference to this code. In this case, I think a one-line
change to stop building that file would be best, then if anyone ever
wants to fix the bug that Hans and Sakari found, they get to do also
deal with replacing compat_alloc_user_space().

I'll send a patch for that right away, the patch I have in my tree was
so evil that I hadn't dared submit that, but it was useful for
compile-testing the compat_alloc_user_space() removal patch.

   Arnd

Re: [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers

[Sam Ravnborg]

Hi Arnd.

On Wed, Oct 07, 2020 at 10:54:19AM +0200, Arnd Bergmann wrote:
> On Wed, Oct 7, 2020 at 9:44 AM Christoph Hellwig <hch@lst.de> wrote:
> >
> > There are no actual implementations of FBIOSCURSOR/FBIOGCURSOR left, so
> > don't bother with a compat handler for it, and remove the remaining
> > definitions as well.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> I had submitted a similar patch earlier, and Sam Ravnborg applied it to the
> drm-misc tree, but it doesn't seem to be in linux-next, so I don't know
> what the state is.
> 
> My version only removed the compat handling, not the data structures,
> so I'm happy to see your version used instead if mine got lost.
> 
> Acked-by: Arnd Bergmann <arnd@arndb.de>

I think the patches got applied to drm-misc-next *after* the final push
to -next for the current merge window.
So the patches are lingering in drm-misc-next waiting for the upcoming
merge window to comple before they are pushed to linux-next.
So for now the patches are only visible if one pulls drm-misc and checks
out drm-misc-next branch.

Pulling a fresh drm-misc tree just to be 100% sure .......

Yep, patches are there when I pull a fresh tree and mripard just
confirmed on irc that he sees them in his drm-misc-next repo.

I am working with git.freedesktop.org/git/drm/drm-misc.

	Sam

Re: [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers

[Sam Ravnborg]

Hi Arnd.

On Wed, Oct 07, 2020 at 10:54:19AM +0200, Arnd Bergmann wrote:
> On Wed, Oct 7, 2020 at 9:44 AM Christoph Hellwig <hch@lst.de> wrote:
> >
> > There are no actual implementations of FBIOSCURSOR/FBIOGCURSOR left, so
> > don't bother with a compat handler for it, and remove the remaining
> > definitions as well.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> I had submitted a similar patch earlier, and Sam Ravnborg applied it to the
> drm-misc tree, but it doesn't seem to be in linux-next, so I don't know
> what the state is.
> 
> My version only removed the compat handling, not the data structures,
> so I'm happy to see your version used instead if mine got lost.
> 
> Acked-by: Arnd Bergmann <arnd@arndb.de>

I think the patches got applied to drm-misc-next *after* the final push
to -next for the current merge window.
So the patches are lingering in drm-misc-next waiting for the upcoming
merge window to comple before they are pushed to linux-next.
So for now the patches are only visible if one pulls drm-misc and checks
out drm-misc-next branch.

Pulling a fresh drm-misc tree just to be 100% sure .......

Yep, patches are there when I pull a fresh tree and mripard just
confirmed on irc that he sees them in his drm-misc-next repo.

I am working with git.freedesktop.org/git/drm/drm-misc.

	Sam

[PATCH 2/3] fbdev/sbuslib: refactor sbusfb_ioctl_helper

[Christoph Hellwig]

Refactor sbusfb_ioctl_helper into a bunch of self-contained helpers,
to prepare for improvements to the compat ioctl handler.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/video/fbdev/sbuslib.c | 157 ++++++++++++++++------------------
 1 file changed, 75 insertions(+), 82 deletions(-)

diff --git a/drivers/video/fbdev/sbuslib.c b/drivers/video/fbdev/sbuslib.c
index 176dbfb5d3efca..1c3bf1cb8dccd7 100644
--- a/drivers/video/fbdev/sbuslib.c
+++ b/drivers/video/fbdev/sbuslib.c
@@ -97,94 +97,87 @@ int sbusfb_mmap_helper(struct sbus_mmap_map *map,
 }
 EXPORT_SYMBOL(sbusfb_mmap_helper);
 
-int sbusfb_ioctl_helper(unsigned long cmd, unsigned long arg,
-			struct fb_info *info,
-			int type, int fb_depth, unsigned long fb_size)
+static int sbufsfb_ioctl_gtype(struct fbtype __user *f, struct fb_info *info,
+		int type, int fb_depth, unsigned long fb_size)
 {
-	switch(cmd) {
-	case FBIOGTYPE: {
-		struct fbtype __user *f = (struct fbtype __user *) arg;
-
-		if (put_user(type, &f->fb_type) ||
-		    put_user(info->var.yres, &f->fb_height) ||
-		    put_user(info->var.xres, &f->fb_width) ||
-		    put_user(fb_depth, &f->fb_depth) ||
-		    put_user(0, &f->fb_cmsize) ||
-		    put_user(fb_size, &f->fb_cmsize))
-			return -EFAULT;
-		return 0;
-	}
-	case FBIOPUTCMAP_SPARC: {
-		struct fbcmap __user *c = (struct fbcmap __user *) arg;
-		struct fb_cmap cmap;
-		u16 red, green, blue;
-		u8 red8, green8, blue8;
-		unsigned char __user *ured;
-		unsigned char __user *ugreen;
-		unsigned char __user *ublue;
-		unsigned int index, count, i;
-
-		if (get_user(index, &c->index) ||
-		    get_user(count, &c->count) ||
-		    get_user(ured, &c->red) ||
-		    get_user(ugreen, &c->green) ||
-		    get_user(ublue, &c->blue))
+	if (put_user(type, &f->fb_type) ||
+	    put_user(info->var.yres, &f->fb_height) ||
+	    put_user(info->var.xres, &f->fb_width) ||
+	    put_user(fb_depth, &f->fb_depth) ||
+	    put_user(fb_size, &f->fb_cmsize))
+		return -EFAULT;
+	return 0;
+}
+
+static int sbusfb_ioctl_putcmap(struct fbcmap *c, struct fb_info *info)
+{
+	u8 red8, green8, blue8;
+	u16 red, green, blue;
+	struct fb_cmap cmap;
+	unsigned int i;
+	int err;
+
+	cmap.len = 1;
+	cmap.red = &red;
+	cmap.green = &green;
+	cmap.blue = &blue;
+	cmap.transp = NULL;
+	for (i = 0; i < c->count; i++) {
+		if (get_user(red8, c->red + i) ||
+		    get_user(green8, c->green + i) ||
+		    get_user(blue8, c->blue + i))
 			return -EFAULT;
 
-		cmap.len = 1;
-		cmap.red = &red;
-		cmap.green = &green;
-		cmap.blue = &blue;
-		cmap.transp = NULL;
-		for (i = 0; i < count; i++) {
-			int err;
-
-			if (get_user(red8, &ured[i]) ||
-			    get_user(green8, &ugreen[i]) ||
-			    get_user(blue8, &ublue[i]))
-				return -EFAULT;
-
-			red = red8 << 8;
-			green = green8 << 8;
-			blue = blue8 << 8;
-
-			cmap.start = index + i;
-			err = fb_set_cmap(&cmap, info);
-			if (err)
-				return err;
-		}
-		return 0;
+		red = red8 << 8;
+		green = green8 << 8;
+		blue = blue8 << 8;
+
+		cmap.start = c->index + i;
+		err = fb_set_cmap(&cmap, info);
+		if (err)
+			return err;
 	}
-	case FBIOGETCMAP_SPARC: {
-		struct fbcmap __user *c = (struct fbcmap __user *) arg;
-		unsigned char __user *ured;
-		unsigned char __user *ugreen;
-		unsigned char __user *ublue;
-		struct fb_cmap *cmap = &info->cmap;
-		unsigned int index, count, i;
-		u8 red, green, blue;
-
-		if (get_user(index, &c->index) ||
-		    get_user(count, &c->count) ||
-		    get_user(ured, &c->red) ||
-		    get_user(ugreen, &c->green) ||
-		    get_user(ublue, &c->blue))
-			return -EFAULT;
+	return 0;
+}
 
-		if (index > cmap->len || count > cmap->len - index)
-			return -EINVAL;
-
-		for (i = 0; i < count; i++) {
-			red = cmap->red[index + i] >> 8;
-			green = cmap->green[index + i] >> 8;
-			blue = cmap->blue[index + i] >> 8;
-			if (put_user(red, &ured[i]) ||
-			    put_user(green, &ugreen[i]) ||
-			    put_user(blue, &ublue[i]))
-				return -EFAULT;
-		}
-		return 0;
+static int sbusfb_ioctl_getcmap(struct fbcmap *c, struct fb_info *info)
+{
+	unsigned int i;
+
+	if (c->index > info->cmap.len || c->count > info->cmap.len - c->index)
+		return -EINVAL;
+
+	for (i = 0; i < c->count; i++) {
+		u8 red = info->cmap.red[c->index + i] >> 8;
+		u8 green = info->cmap.green[c->index + i] >> 8;
+		u8 blue = info->cmap.blue[c->index + i] >> 8;
+
+		if (put_user(red, c->red + i) ||
+		    put_user(green, c->green + i) ||
+		    put_user(blue, c->blue + i))
+			return -EFAULT;
 	}
+	return 0;
+}
+
+int sbusfb_ioctl_helper(unsigned long cmd, unsigned long arg,
+			struct fb_info *info,
+			int type, int fb_depth, unsigned long fb_size)
+{
+	void __user *argp = (void __user *)arg;
+	struct fbcmap kc;
+
+	switch (cmd) {
+	case FBIOGTYPE:
+		return sbufsfb_ioctl_gtype(argp, info, type, fb_depth, fb_size);
+	case FBIOPUTCMAP_SPARC:
+		if (copy_from_user(&kc, argp, sizeof(kc)))
+			return -EFAULT;
+		return sbusfb_ioctl_putcmap(&kc, info);
+	case FBIOGETCMAP_SPARC:
+		if (copy_from_user(&kc, argp, sizeof(kc)))
+			return -EFAULT;
+		return sbusfb_ioctl_getcmap(&kc, info);
 	default:
 		return -EINVAL;
 	}
-- 
2.28.0

[PATCH 2/3] fbdev/sbuslib: refactor sbusfb_ioctl_helper

[Christoph Hellwig]

Refactor sbusfb_ioctl_helper into a bunch of self-contained helpers,
to prepare for improvements to the compat ioctl handler.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/video/fbdev/sbuslib.c | 157 ++++++++++++++++------------------
 1 file changed, 75 insertions(+), 82 deletions(-)

diff --git a/drivers/video/fbdev/sbuslib.c b/drivers/video/fbdev/sbuslib.c
index 176dbfb5d3efca..1c3bf1cb8dccd7 100644
--- a/drivers/video/fbdev/sbuslib.c
+++ b/drivers/video/fbdev/sbuslib.c
@@ -97,94 +97,87 @@ int sbusfb_mmap_helper(struct sbus_mmap_map *map,
 }
 EXPORT_SYMBOL(sbusfb_mmap_helper);
 
-int sbusfb_ioctl_helper(unsigned long cmd, unsigned long arg,
-			struct fb_info *info,
-			int type, int fb_depth, unsigned long fb_size)
+static int sbufsfb_ioctl_gtype(struct fbtype __user *f, struct fb_info *info,
+		int type, int fb_depth, unsigned long fb_size)
 {
-	switch(cmd) {
-	case FBIOGTYPE: {
-		struct fbtype __user *f = (struct fbtype __user *) arg;
-
-		if (put_user(type, &f->fb_type) ||
-		    put_user(info->var.yres, &f->fb_height) ||
-		    put_user(info->var.xres, &f->fb_width) ||
-		    put_user(fb_depth, &f->fb_depth) ||
-		    put_user(0, &f->fb_cmsize) ||
-		    put_user(fb_size, &f->fb_cmsize))
-			return -EFAULT;
-		return 0;
-	}
-	case FBIOPUTCMAP_SPARC: {
-		struct fbcmap __user *c = (struct fbcmap __user *) arg;
-		struct fb_cmap cmap;
-		u16 red, green, blue;
-		u8 red8, green8, blue8;
-		unsigned char __user *ured;
-		unsigned char __user *ugreen;
-		unsigned char __user *ublue;
-		unsigned int index, count, i;
-
-		if (get_user(index, &c->index) ||
-		    get_user(count, &c->count) ||
-		    get_user(ured, &c->red) ||
-		    get_user(ugreen, &c->green) ||
-		    get_user(ublue, &c->blue))
+	if (put_user(type, &f->fb_type) ||
+	    put_user(info->var.yres, &f->fb_height) ||
+	    put_user(info->var.xres, &f->fb_width) ||
+	    put_user(fb_depth, &f->fb_depth) ||
+	    put_user(fb_size, &f->fb_cmsize))
+		return -EFAULT;
+	return 0;
+}
+
+static int sbusfb_ioctl_putcmap(struct fbcmap *c, struct fb_info *info)
+{
+	u8 red8, green8, blue8;
+	u16 red, green, blue;
+	struct fb_cmap cmap;
+	unsigned int i;
+	int err;
+
+	cmap.len = 1;
+	cmap.red = &red;
+	cmap.green = &green;
+	cmap.blue = &blue;
+	cmap.transp = NULL;
+	for (i = 0; i < c->count; i++) {
+		if (get_user(red8, c->red + i) ||
+		    get_user(green8, c->green + i) ||
+		    get_user(blue8, c->blue + i))
 			return -EFAULT;
 
-		cmap.len = 1;
-		cmap.red = &red;
-		cmap.green = &green;
-		cmap.blue = &blue;
-		cmap.transp = NULL;
-		for (i = 0; i < count; i++) {
-			int err;
-
-			if (get_user(red8, &ured[i]) ||
-			    get_user(green8, &ugreen[i]) ||
-			    get_user(blue8, &ublue[i]))
-				return -EFAULT;
-
-			red = red8 << 8;
-			green = green8 << 8;
-			blue = blue8 << 8;
-
-			cmap.start = index + i;
-			err = fb_set_cmap(&cmap, info);
-			if (err)
-				return err;
-		}
-		return 0;
+		red = red8 << 8;
+		green = green8 << 8;
+		blue = blue8 << 8;
+
+		cmap.start = c->index + i;
+		err = fb_set_cmap(&cmap, info);
+		if (err)
+			return err;
 	}
-	case FBIOGETCMAP_SPARC: {
-		struct fbcmap __user *c = (struct fbcmap __user *) arg;
-		unsigned char __user *ured;
-		unsigned char __user *ugreen;
-		unsigned char __user *ublue;
-		struct fb_cmap *cmap = &info->cmap;
-		unsigned int index, count, i;
-		u8 red, green, blue;
-
-		if (get_user(index, &c->index) ||
-		    get_user(count, &c->count) ||
-		    get_user(ured, &c->red) ||
-		    get_user(ugreen, &c->green) ||
-		    get_user(ublue, &c->blue))
-			return -EFAULT;
+	return 0;
+}
 
-		if (index > cmap->len || count > cmap->len - index)
-			return -EINVAL;
-
-		for (i = 0; i < count; i++) {
-			red = cmap->red[index + i] >> 8;
-			green = cmap->green[index + i] >> 8;
-			blue = cmap->blue[index + i] >> 8;
-			if (put_user(red, &ured[i]) ||
-			    put_user(green, &ugreen[i]) ||
-			    put_user(blue, &ublue[i]))
-				return -EFAULT;
-		}
-		return 0;
+static int sbusfb_ioctl_getcmap(struct fbcmap *c, struct fb_info *info)
+{
+	unsigned int i;
+
+	if (c->index > info->cmap.len || c->count > info->cmap.len - c->index)
+		return -EINVAL;
+
+	for (i = 0; i < c->count; i++) {
+		u8 red = info->cmap.red[c->index + i] >> 8;
+		u8 green = info->cmap.green[c->index + i] >> 8;
+		u8 blue = info->cmap.blue[c->index + i] >> 8;
+
+		if (put_user(red, c->red + i) ||
+		    put_user(green, c->green + i) ||
+		    put_user(blue, c->blue + i))
+			return -EFAULT;
 	}
+	return 0;
+}
+
+int sbusfb_ioctl_helper(unsigned long cmd, unsigned long arg,
+			struct fb_info *info,
+			int type, int fb_depth, unsigned long fb_size)
+{
+	void __user *argp = (void __user *)arg;
+	struct fbcmap kc;
+
+	switch (cmd) {
+	case FBIOGTYPE:
+		return sbufsfb_ioctl_gtype(argp, info, type, fb_depth, fb_size);
+	case FBIOPUTCMAP_SPARC:
+		if (copy_from_user(&kc, argp, sizeof(kc)))
+			return -EFAULT;
+		return sbusfb_ioctl_putcmap(&kc, info);
+	case FBIOGETCMAP_SPARC:
+		if (copy_from_user(&kc, argp, sizeof(kc)))
+			return -EFAULT;
+		return sbusfb_ioctl_getcmap(&kc, info);
 	default:
 		return -EINVAL;
 	}
-- 
2.28.0

[PATCH 3/3] fbdev/sbuslib: avoid compat_alloc_user_space in fbiogetputcmap

[Christoph Hellwig]

Rewrite fbiogetputcmap to call the low-level sbusfb_ioctl_{put,get}cmap
helpers directly and thus avoid usage of the deprecated
compat_alloc_user_space API.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/video/fbdev/sbuslib.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/video/fbdev/sbuslib.c b/drivers/video/fbdev/sbuslib.c
index 1c3bf1cb8dccd7..ef74e3e6326a06 100644
--- a/drivers/video/fbdev/sbuslib.c
+++ b/drivers/video/fbdev/sbuslib.c
@@ -188,23 +188,23 @@ EXPORT_SYMBOL(sbusfb_ioctl_helper);
 static int fbiogetputcmap(struct fb_info *info, unsigned int cmd, unsigned long arg)
 {
 	struct fbcmap32 __user *argp = (void __user *)arg;
-	struct fbcmap __user *p = compat_alloc_user_space(sizeof(*p));
+	struct fbcmap kc;
 	u32 addr;
 	int ret;
 
-	ret = copy_in_user(p, argp, 2 * sizeof(int));
+	ret = get_user(kc.index, &argp->index);
+	ret |= get_user(kc.count, &argp->count);
 	ret |= get_user(addr, &argp->red);
-	ret |= put_user(compat_ptr(addr), &p->red);
+	kc.red = compat_ptr(addr);
 	ret |= get_user(addr, &argp->green);
-	ret |= put_user(compat_ptr(addr), &p->green);
+	kc.green = compat_ptr(addr);
 	ret |= get_user(addr, &argp->blue);
-	ret |= put_user(compat_ptr(addr), &p->blue);
+	kc.blue = compat_ptr(addr);
 	if (ret)
 		return -EFAULT;
-	return info->fbops->fb_ioctl(info,
-			(cmd = FBIOPUTCMAP32) ?
-			FBIOPUTCMAP_SPARC : FBIOGETCMAP_SPARC,
-			(unsigned long)p);
+	if (cmd = FBIOPUTCMAP32)
+		return sbusfb_ioctl_putcmap(&kc, info);
+	return sbusfb_ioctl_getcmap(&kc, info);
 }
 
 int sbusfb_compat_ioctl(struct fb_info *info, unsigned int cmd, unsigned long arg)
-- 
2.28.0

[PATCH 3/3] fbdev/sbuslib: avoid compat_alloc_user_space in fbiogetputcmap

[Christoph Hellwig]

Rewrite fbiogetputcmap to call the low-level sbusfb_ioctl_{put,get}cmap
helpers directly and thus avoid usage of the deprecated
compat_alloc_user_space API.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/video/fbdev/sbuslib.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/video/fbdev/sbuslib.c b/drivers/video/fbdev/sbuslib.c
index 1c3bf1cb8dccd7..ef74e3e6326a06 100644
--- a/drivers/video/fbdev/sbuslib.c
+++ b/drivers/video/fbdev/sbuslib.c
@@ -188,23 +188,23 @@ EXPORT_SYMBOL(sbusfb_ioctl_helper);
 static int fbiogetputcmap(struct fb_info *info, unsigned int cmd, unsigned long arg)
 {
 	struct fbcmap32 __user *argp = (void __user *)arg;
-	struct fbcmap __user *p = compat_alloc_user_space(sizeof(*p));
+	struct fbcmap kc;
 	u32 addr;
 	int ret;
 
-	ret = copy_in_user(p, argp, 2 * sizeof(int));
+	ret = get_user(kc.index, &argp->index);
+	ret |= get_user(kc.count, &argp->count);
 	ret |= get_user(addr, &argp->red);
-	ret |= put_user(compat_ptr(addr), &p->red);
+	kc.red = compat_ptr(addr);
 	ret |= get_user(addr, &argp->green);
-	ret |= put_user(compat_ptr(addr), &p->green);
+	kc.green = compat_ptr(addr);
 	ret |= get_user(addr, &argp->blue);
-	ret |= put_user(compat_ptr(addr), &p->blue);
+	kc.blue = compat_ptr(addr);
 	if (ret)
 		return -EFAULT;
-	return info->fbops->fb_ioctl(info,
-			(cmd == FBIOPUTCMAP32) ?
-			FBIOPUTCMAP_SPARC : FBIOGETCMAP_SPARC,
-			(unsigned long)p);
+	if (cmd == FBIOPUTCMAP32)
+		return sbusfb_ioctl_putcmap(&kc, info);
+	return sbusfb_ioctl_getcmap(&kc, info);
 }
 
 int sbusfb_compat_ioctl(struct fb_info *info, unsigned int cmd, unsigned long arg)
-- 
2.28.0

Re: [PATCH 3/3] fbdev/sbuslib: avoid compat_alloc_user_space in fbiogetputcmap

[Arnd Bergmann]

On Wed, Oct 7, 2020 at 9:44 AM Christoph Hellwig <hch@lst.de> wrote:
>
> Rewrite fbiogetputcmap to call the low-level sbusfb_ioctl_{put,get}cmap
> helpers directly and thus avoid usage of the deprecated
> compat_alloc_user_space API.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

This is another one that I had already addressed in a somewhat inferior way.
I attempted the same kind of cleanup that you did in patch 2/3 but failed to
convince myself that I had managed to avoid regressions, so my patch just
copy-pasted the native implementation into the compat handler with minor
changes.

Provided that your preparation patch is correct, this version is clearly better
than mine, but again I don't know the state of that patch after Sam had
said he applied it already.

     Arnd

Re: [PATCH 3/3] fbdev/sbuslib: avoid compat_alloc_user_space in fbiogetputcmap

[Arnd Bergmann]

On Wed, Oct 7, 2020 at 9:44 AM Christoph Hellwig <hch@lst.de> wrote:
>
> Rewrite fbiogetputcmap to call the low-level sbusfb_ioctl_{put,get}cmap
> helpers directly and thus avoid usage of the deprecated
> compat_alloc_user_space API.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

This is another one that I had already addressed in a somewhat inferior way.
I attempted the same kind of cleanup that you did in patch 2/3 but failed to
convince myself that I had managed to avoid regressions, so my patch just
copy-pasted the native implementation into the compat handler with minor
changes.

Provided that your preparation patch is correct, this version is clearly better
than mine, but again I don't know the state of that patch after Sam had
said he applied it already.

     Arnd