diff mbox series

[7/9,3.18-stable] i2o: hide unsafe ioctl on 64-bit

Message ID 20170505115725.1424772-8-arnd@arndb.de
State Superseded
Headers show
Series fixes for all remaining known warnings | expand

Commit Message

Arnd Bergmann May 5, 2017, 11:57 a.m. UTC
We get a warning about a broken pointer conversion on 64-bit architectures:

drivers/message/i2o/i2o_config.c: In function 'i2o_cfg_passthru':
drivers/message/i2o/i2o_config.c:893:19: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
         (p->virt, (void __user *)sg[i].addr_bus,
                   ^
drivers/message/i2o/i2o_config.c:953:10: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
         ((void __user *)sg[j].addr_bus, sg_list[j].virt,
          ^

This has clearly never worked right, so we can add an #ifdef around the code.
The driver was moved to staging in linux-4.0 and finally removed in 4.2,
so upstream does not have a fix for it.

The driver originally got this mostly right, though probably by accident.

Fixes: f4c2c15b930b ("[PATCH] Convert i2o to compat_ioctl")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 drivers/message/i2o/i2o_config.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.9.0

Comments

Greg Kroah-Hartman May 5, 2017, 5:13 p.m. UTC | #1
On Fri, May 05, 2017 at 01:57:23PM +0200, Arnd Bergmann wrote:
> We get a warning about a broken pointer conversion on 64-bit architectures:

> 

> drivers/message/i2o/i2o_config.c: In function 'i2o_cfg_passthru':

> drivers/message/i2o/i2o_config.c:893:19: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]

>          (p->virt, (void __user *)sg[i].addr_bus,

>                    ^

> drivers/message/i2o/i2o_config.c:953:10: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]

>          ((void __user *)sg[j].addr_bus, sg_list[j].virt,

>           ^

> 

> This has clearly never worked right, so we can add an #ifdef around the code.

> The driver was moved to staging in linux-4.0 and finally removed in 4.2,

> so upstream does not have a fix for it.

> 

> The driver originally got this mostly right, though probably by accident.


I fixed this in a different way, doing the cast to make things be quiet,
as was done in other places in the driver.  You were copied on that
patch when it was added to my tree, right?

And yes, this thing probably never really worked, so I don't worry about
it :)

thanks,

greg k-h
Arnd Bergmann May 5, 2017, 6:36 p.m. UTC | #2
On Fri, May 5, 2017 at 7:13 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, May 05, 2017 at 01:57:23PM +0200, Arnd Bergmann wrote:

>> We get a warning about a broken pointer conversion on 64-bit architectures:

>>

>> drivers/message/i2o/i2o_config.c: In function 'i2o_cfg_passthru':

>> drivers/message/i2o/i2o_config.c:893:19: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]

>>          (p->virt, (void __user *)sg[i].addr_bus,

>>                    ^

>> drivers/message/i2o/i2o_config.c:953:10: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]

>>          ((void __user *)sg[j].addr_bus, sg_list[j].virt,

>>           ^

>>

>> This has clearly never worked right, so we can add an #ifdef around the code.

>> The driver was moved to staging in linux-4.0 and finally removed in 4.2,

>> so upstream does not have a fix for it.

>>

>> The driver originally got this mostly right, though probably by accident.

>

> I fixed this in a different way, doing the cast to make things be quiet,

> as was done in other places in the driver.  You were copied on that

> patch when it was added to my tree, right?


I missed it when you sent it, but looking at it now, I think it introduces
another warning, on 32-bit architectures:

-                                   (p->virt, (void __user *)sg[i].addr_bus,
+                                   (p->virt, (void __user
*)(u64)sg[i].addr_bus,

I think you need '(void __user *)(uintptr_t)sg[i].addr_bus' to
shut up the warning for all builds.

    Arnd
Greg Kroah-Hartman May 5, 2017, 8:07 p.m. UTC | #3
On Fri, May 05, 2017 at 08:36:55PM +0200, Arnd Bergmann wrote:
> On Fri, May 5, 2017 at 7:13 PM, Greg KH <gregkh@linuxfoundation.org> wrote:

> > On Fri, May 05, 2017 at 01:57:23PM +0200, Arnd Bergmann wrote:

> >> We get a warning about a broken pointer conversion on 64-bit architectures:

> >>

> >> drivers/message/i2o/i2o_config.c: In function 'i2o_cfg_passthru':

> >> drivers/message/i2o/i2o_config.c:893:19: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]

> >>          (p->virt, (void __user *)sg[i].addr_bus,

> >>                    ^

> >> drivers/message/i2o/i2o_config.c:953:10: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]

> >>          ((void __user *)sg[j].addr_bus, sg_list[j].virt,

> >>           ^

> >>

> >> This has clearly never worked right, so we can add an #ifdef around the code.

> >> The driver was moved to staging in linux-4.0 and finally removed in 4.2,

> >> so upstream does not have a fix for it.

> >>

> >> The driver originally got this mostly right, though probably by accident.

> >

> > I fixed this in a different way, doing the cast to make things be quiet,

> > as was done in other places in the driver.  You were copied on that

> > patch when it was added to my tree, right?

> 

> I missed it when you sent it, but looking at it now, I think it introduces

> another warning, on 32-bit architectures:

> 

> -                                   (p->virt, (void __user *)sg[i].addr_bus,

> +                                   (p->virt, (void __user

> *)(u64)sg[i].addr_bus,

> 

> I think you need '(void __user *)(uintptr_t)sg[i].addr_bus' to

> shut up the warning for all builds.


Ah good point.  I've now dropped my version and taken yours instead :)

thanks,

greg k-h
diff mbox series

Patch

diff --git a/drivers/message/i2o/i2o_config.c b/drivers/message/i2o/i2o_config.c
index 04bd3b6de401..67ceb3010a10 100644
--- a/drivers/message/i2o/i2o_config.c
+++ b/drivers/message/i2o/i2o_config.c
@@ -772,7 +772,7 @@  static long i2o_cfg_compat_ioctl(struct file *file, unsigned cmd,
 
 #endif
 
-#ifdef CONFIG_I2O_EXT_ADAPTEC
+#if defined(CONFIG_I2O_EXT_ADAPTEC) && !defined(CONFIG_64BIT)
 static int i2o_cfg_passthru(unsigned long arg)
 {
 	struct i2o_cmd_passthru __user *cmd =
@@ -1045,7 +1045,7 @@  static long i2o_cfg_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
 		ret = i2o_cfg_evt_get(arg, fp);
 		break;
 
-#ifdef CONFIG_I2O_EXT_ADAPTEC
+#if defined(CONFIG_I2O_EXT_ADAPTEC) && !defined(CONFIG_64BIT)
 	case I2OPASSTHRU:
 		ret = i2o_cfg_passthru(arg);
 		break;