Re: cciss update for 2.6.7-rc1

Re: cciss update for 2.6.7-rc1

[Andi Kleen]

mikem@beardog.cca.cpqcorp.net writes:


> @@ -451,6 +451,140 @@ static int cciss_release(struct inode *i
>  	return 0;
>  }
>  
> +#ifdef __x86_64__
> +/* for AMD 64 bit kernel compatibility with 32-bit userland ioctls */
> +#include <linux/syscalls.h>
> +extern int 
> +register_ioctl32_conversion(unsigned int cmd, int (*handler)(unsigned int,
> +      unsigned int, unsigned long, struct file *));
> +extern int unregister_ioctl32_conversion(unsigned int cmd);

This should be in CONFIG_COMPAT instead of testing for x86-64 
and include  <linux/compat.h> for the prototypes. linux/syscalls.h
should not be needed.

-Andi

cciss update for 2.6.7-rc1

[mikem]

I was having problems with my mailer. Not sure if this made it or not. Sorry for any duplication.

This patch provides a conversion routine for 32-bit user space apps that call into a 64-bit kernel on x86_64 architectures. This is required for the HP Array Configuration utility and the HP management agents. Without this patch the apps will not function. The 2 ioctls affected are the cciss pass thru ioctls.
Caveat: it spits out 2 warnings during compilation. I've tried everything I can think of to clean them up, but...
If anyone has any helpful suggestions I'm all ears.

Code by Stephen Cameron
Tested by Stephen Cameron & Mike Miller

Please consider this for inclusion.

Thanks,
mikem
--------------------------------------------------------------------------------
 drivers/block/cciss.c       |  136 ++++++++++++++++++++++++++++++++++++++++++++ include/linux/cciss_ioctl.h |   28 +++++++++
 2 files changed, 164 insertions(+)

diff -burpN lx267-rc1.orig/drivers/block/cciss.c lx267-rc1/drivers/block/cciss.c
--- lx267-rc1.orig/drivers/block/cciss.c	2004-05-09 21:33:20.000000000 -0500
+++ lx267-rc1/drivers/block/cciss.c	2004-05-28 10:34:17.000000000 -0500
@@ -451,6 +451,140 @@ static int cciss_release(struct inode *i
 	return 0;
 }
 
+#ifdef __x86_64__
+/* for AMD 64 bit kernel compatibility with 32-bit userland ioctls */
+#include <linux/syscalls.h>
+extern int 
+register_ioctl32_conversion(unsigned int cmd, int (*handler)(unsigned int,
+      unsigned int, unsigned long, struct file *));
+extern int unregister_ioctl32_conversion(unsigned int cmd);
+
+static int cciss_ioctl32_passthru(unsigned int fd, unsigned cmd, unsigned long arg, struct file *file);
+static int cciss_ioctl32_big_passthru(unsigned int fd, unsigned cmd, unsigned long arg, 
+	struct file *file);
+
+typedef int (*handler_type) (unsigned int, unsigned int, unsigned long, struct file *);
+
+static struct ioctl32_map {
+	unsigned int cmd; 
+	handler_type handler;
+	int registered;
+} cciss_ioctl32_map[] = {
+	{ CCISS_GETPCIINFO,	(handler_type) sys_ioctl, 0 },
+	{ CCISS_GETINTINFO,	(handler_type) sys_ioctl, 0 },
+	{ CCISS_SETINTINFO,	(handler_type) sys_ioctl, 0 },
+	{ CCISS_GETNODENAME,	(handler_type) sys_ioctl, 0 },
+	{ CCISS_SETNODENAME,	(handler_type) sys_ioctl, 0 },
+	{ CCISS_GETHEARTBEAT,	(handler_type) sys_ioctl, 0 },
+	{ CCISS_GETBUSTYPES,	(handler_type) sys_ioctl, 0 },
+	{ CCISS_GETFIRMVER,	(handler_type) sys_ioctl, 0 },
+	{ CCISS_GETDRIVVER,	(handler_type) sys_ioctl, 0 },
+	{ CCISS_REVALIDVOLS,	(handler_type) sys_ioctl, 0 },
+	{ CCISS_PASSTHRU32,	cciss_ioctl32_passthru, 0 },
+	{ CCISS_DEREGDISK,	(handler_type) sys_ioctl, 0 },
+	{ CCISS_REGNEWDISK,	(handler_type) sys_ioctl, 0 },
+	{ CCISS_REGNEWD,	(handler_type) sys_ioctl, 0 },
+	{ CCISS_RESCANDISK,	(handler_type) sys_ioctl, 0 },
+	{ CCISS_GETLUNINFO,	(handler_type) sys_ioctl, 0 },
+	{ CCISS_BIG_PASSTHRU32,	cciss_ioctl32_big_passthru, 0 },
+};
+#define NCCISS_IOCTL32_ENTRIES (sizeof(cciss_ioctl32_map) / sizeof(cciss_ioctl32_map[0]))
+static void register_cciss_ioctl32(void)
+{
+	int i, rc;
+
+	for (i=0; i < NCCISS_IOCTL32_ENTRIES; i++) {
+		rc = register_ioctl32_conversion(
+			cciss_ioctl32_map[i].cmd,
+			cciss_ioctl32_map[i].handler);
+		if (rc != 0) {
+			printk(KERN_WARNING "cciss: failed to register "
+				"32 bit compatible ioctl 0x%08x\n", 
+				cciss_ioctl32_map[i].cmd);
+			cciss_ioctl32_map[i].registered = 0;
+		} else
+			cciss_ioctl32_map[i].registered = 1;
+	}
+}
+static void unregister_cciss_ioctl32(void)
+{
+	int i, rc;
+
+	for (i=0; i < NCCISS_IOCTL32_ENTRIES; i++) {
+		if (!cciss_ioctl32_map[i].registered)
+			continue;
+		rc = unregister_ioctl32_conversion(
+			cciss_ioctl32_map[i].cmd);
+		if (rc == 0) {
+			cciss_ioctl32_map[i].registered = 0;
+			continue;
+		}
+		printk(KERN_WARNING "cciss: failed to unregister "
+			"32 bit compatible ioctl 0x%08x\n",
+			cciss_ioctl32_map[i].cmd);
+	}
+}
+int cciss_ioctl32_passthru(unsigned int fd, unsigned cmd, unsigned long arg, 
+	struct file *file)
+{
+	IOCTL32_Command_struct *arg32 = 
+		(IOCTL32_Command_struct *) arg;
+	IOCTL_Command_struct arg64;
+	mm_segment_t old_fs; 
+	int err;
+
+	err = 0;
+	err |= copy_from_user(&arg64.LUN_info, &arg32->LUN_info, sizeof(arg64.LUN_info));
+	err |= copy_from_user(&arg64.Request, &arg32->Request, sizeof(arg64.Request));
+	err |= copy_from_user(&arg64.error_info, &arg32->error_info, sizeof(arg64.error_info));
+	err |= get_user(arg64.buf_size, &arg32->buf_size);
+	err |= get_user(arg64.buf, &arg32->buf);
+	if (err) 
+		return -EFAULT; 
+
+	old_fs = get_fs();
+	set_fs(KERNEL_DS);
+	err = sys_ioctl(fd, CCISS_PASSTHRU, (unsigned long) &arg64);
+	set_fs(old_fs);
+	if (err)
+		return err;
+	err |= copy_to_user(&arg32->error_info, &arg64.error_info, sizeof(&arg32->error_info));
+	if (err) 
+		return -EFAULT; 
+	return err;
+}
+int cciss_ioctl32_big_passthru(unsigned int fd, unsigned cmd, unsigned long arg, 
+	struct file *file)
+{
+	BIG_IOCTL32_Command_struct *arg32 = 
+		(BIG_IOCTL32_Command_struct *) arg;
+	BIG_IOCTL_Command_struct arg64;
+	mm_segment_t old_fs; 
+	int err;
+
+	err = 0;
+	err |= copy_from_user(&arg64.LUN_info, &arg32->LUN_info, sizeof(arg64.LUN_info));
+	err |= copy_from_user(&arg64.Request, &arg32->Request, sizeof(arg64.Request));
+	err |= copy_from_user(&arg64.error_info, &arg32->error_info, sizeof(arg64.error_info));
+	err |= get_user(arg64.buf_size, &arg32->buf_size);
+	err |= get_user(arg64.malloc_size, &arg32->malloc_size);
+	err |= get_user(arg64.buf, &arg32->buf);
+	if (err) return -EFAULT; 
+	old_fs = get_fs();
+	set_fs(KERNEL_DS);
+	err = sys_ioctl(fd, CCISS_BIG_PASSTHRU, (unsigned long) &arg64);
+	set_fs(old_fs);
+	if (err)
+		return err;
+	err |= copy_to_user(&arg32->error_info, &arg64.error_info, sizeof(&arg32->error_info));
+	if (err) 
+		return -EFAULT; 
+	return err;
+}
+#else 
+static inline void register_cciss_ioctl32(void) {}
+static inline void unregister_cciss_ioctl32(void) {}
+#endif
 /*
  * ioctl 
  */
@@ -2729,6 +2863,7 @@ int __init cciss_init(void)
 
 static int __init init_cciss_module(void)
 {
+	register_cciss_ioctl32();
 	return ( cciss_init());
 }
 
@@ -2736,6 +2871,7 @@ static void __exit cleanup_cciss_module(
 {
 	int i;
 
+	unregister_cciss_ioctl32();
 	pci_unregister_driver(&cciss_pci_driver);
 	/* double check that all controller entrys have been removed */
 	for (i=0; i< MAX_CTLR; i++) 
diff -burpN lx267-rc1.orig/include/linux/cciss_ioctl.h lx267-rc1/include/linux/cciss_ioctl.h
--- lx267-rc1.orig/include/linux/cciss_ioctl.h	2004-05-09 21:32:29.000000000 -0500
+++ lx267-rc1/include/linux/cciss_ioctl.h	2004-05-28 10:34:17.000000000 -0500
@@ -206,7 +206,35 @@ typedef struct _LogvolInfo_struct{
 #define CCISS_REGNEWDISK  _IOW(CCISS_IOC_MAGIC, 13, int)
 
 #define CCISS_REGNEWD	   _IO(CCISS_IOC_MAGIC, 14)
+#define CCISS_RESCANDISK   _IO(CCISS_IOC_MAGIC, 16)
 #define CCISS_GETLUNINFO   _IOR(CCISS_IOC_MAGIC, 17, LogvolInfo_struct)
 #define CCISS_BIG_PASSTHRU _IOWR(CCISS_IOC_MAGIC, 18, BIG_IOCTL_Command_struct)
 
+#ifdef __KERNEL__
+#ifdef __x86_64__
+
+/* 32 bit compatible ioctl structs */ 
+typedef struct _IOCTL32_Command_struct {
+  LUNAddr_struct	   LUN_info;
+  RequestBlock_struct      Request;
+  ErrorInfo_struct  	   error_info; 
+  WORD			   buf_size;  /* size in bytes of the buf */
+  __u32			   buf; /* 32 bit pointer to data buffer */
+} IOCTL32_Command_struct;
+
+typedef struct _BIG_IOCTL32_Command_struct {
+  LUNAddr_struct	   LUN_info;
+  RequestBlock_struct      Request;
+  ErrorInfo_struct  	   error_info; 
+  DWORD			   malloc_size; /* < MAX_KMALLOC_SIZE in cciss.c */
+  DWORD			   buf_size;    /* size in bytes of the buf */
+  				        /* < malloc_size * MAXSGENTRIES */
+  __u32 		buf;	/* 32 bit pointer to data buffer */
+} BIG_IOCTL32_Command_struct;
+
+#define CCISS_PASSTHRU32   _IOWR(CCISS_IOC_MAGIC, 11, IOCTL32_Command_struct)
+#define CCISS_BIG_PASSTHRU32 _IOWR(CCISS_IOC_MAGIC, 18, BIG_IOCTL32_Command_struct)
+
+#endif /* __x86_64__ */
+#endif /* __KERNEL__ */
 #endif  

Re: cciss update for 2.6.7-rc1

[Andrew Morton]

mikem@beardog.cca.cpqcorp.net wrote:
>
> 
> This patch provides a conversion routine for 32-bit user space apps that
> call into a 64-bit kernel on x86_64 architectures.  This is required for
> the HP Array Configuration utility and the HP management agents.  Without
> this patch the apps will not function.  The 2 ioctls affected are the cciss
> pass thru ioctls.
> 
> Caveat: it spits out 2 warnings during compilation.  I've tried everything
> I can think of to clean them up, but...  If anyone has any helpful
> suggestions I'm all ears.

The below fixes up the warnings.  Please test it?

It's very unusual for a disk driver to have `#ifdef __x86_64__' stuff in
it.  Is this problem really unique to x86_64 or should the new code be
enabled for all 64-bit architectures?


--- 25-x86_64/drivers/block/cciss.c~cciss-update-warning-fix	Fri Jun  4 13:40:20 2004
+++ 25-x86_64-akpm/drivers/block/cciss.c	Fri Jun  4 13:42:22 2004
@@ -532,13 +532,16 @@ int cciss_ioctl32_passthru(unsigned int 
 	IOCTL_Command_struct arg64;
 	mm_segment_t old_fs;
 	int err;
+	unsigned long cp;
 
 	err = 0;
 	err |= copy_from_user(&arg64.LUN_info, &arg32->LUN_info, sizeof(arg64.LUN_info));
 	err |= copy_from_user(&arg64.Request, &arg32->Request, sizeof(arg64.Request));
 	err |= copy_from_user(&arg64.error_info, &arg32->error_info, sizeof(arg64.error_info));
 	err |= get_user(arg64.buf_size, &arg32->buf_size);
-	err |= get_user(arg64.buf, &arg32->buf);
+	err |= get_user(cp, &arg32->buf);
+	arg64.buf = (BYTE *)cp;
+
 	if (err)
 		return -EFAULT;
 
@@ -561,6 +564,7 @@ int cciss_ioctl32_big_passthru(unsigned 
 	BIG_IOCTL_Command_struct arg64;
 	mm_segment_t old_fs;
 	int err;
+	unsigned long cp;
 
 	err = 0;
 	err |= copy_from_user(&arg64.LUN_info, &arg32->LUN_info, sizeof(arg64.LUN_info));
@@ -568,8 +572,12 @@ int cciss_ioctl32_big_passthru(unsigned 
 	err |= copy_from_user(&arg64.error_info, &arg32->error_info, sizeof(arg64.error_info));
 	err |= get_user(arg64.buf_size, &arg32->buf_size);
 	err |= get_user(arg64.malloc_size, &arg32->malloc_size);
-	err |= get_user(arg64.buf, &arg32->buf);
-	if (err) return -EFAULT;
+	err |= get_user(cp, &arg32->buf);
+	arg64.buf = (BYTE *)cp;
+
+	if (err)
+		return -EFAULT;
+
 	old_fs = get_fs();
 	set_fs(KERNEL_DS);
 	err = sys_ioctl(fd, CCISS_BIG_PASSTHRU, (unsigned long) &arg64);
_

Re: cciss update for 2.6.7-rc1

[viro]

On Wed, Jun 02, 2004 at 03:13:26PM -0500, mikem@beardog.cca.cpqcorp.net wrote:

a) use compat_alloc_user_space() and copy_in_user()

> +	err |= copy_from_user(&arg64.LUN_info, &arg32->LUN_info, sizeof(arg64.LUN_info));
> +	err |= copy_from_user(&arg64.Request, &arg32->Request, sizeof(arg64.Request));
> +	err |= copy_from_user(&arg64.error_info, &arg32->error_info, sizeof(arg64.error_info));
> +	err |= get_user(arg64.buf_size, &arg32->buf_size);

b) use compat_ptr() to convert pointers

> +	err |= get_user(arg64.buf, &arg32->buf);


c) and do not bother with get_fs()/set_fs() at all - with
compat_alloc_user_space() variant you won't need it.

> +	old_fs = get_fs();
> +	set_fs(KERNEL_DS);
> +	err = sys_ioctl(fd, CCISS_PASSTHRU, (unsigned long) &arg64);
> +	set_fs(old_fs);