Skip to contentSkip to conversation

Add 32-bit compatibility for NFSv4 mount

PATCH In review David Howells 3 participants 5 messages Apr 15, 2005 – Apr 27, 2005 VFS

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Add 32-bit compatibility for NFSv4 mount

The attached patch adds 32-bit compatibility for mounting an NFSv4 mount on a 64-bit kernel (such as happens with PPC64).

The problem is that the mount data for the NFS4 mount process includes auxilliary data pointers, probably because the NFS4 mount data may conceivably exceed PAGE_SIZE in size - thus breaking against the hard limit imposed by sys_mount().

---
warthog>diffstat -p1 /tmp/linux-2.6.9-nfs4-compat-mount.patch

 fs/compat.c |   74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 74 insertions(+)
fs/compat.c renamed from fs/compat.c.orig +74 -0
@@ -30,6 +30,7 @@
 #include <linux/smb.h>
 #include <linux/smb_mount.h>
 #include <linux/ncp_mount.h>
+#include <linux/nfs4_mount.h>
 #include <linux/smp_lock.h>
 #include <linux/syscalls.h>
 #include <linux/ctype.h>
@@ -746,10 +747,79 @@ static void *do_smb_super_data_conv(void
 	return raw_data;
 }
 
+struct compat_nfs_string {
+	compat_uint_t len;
+	compat_uptr_t __user data;
+};
+
+static inline void compat_nfs_string(struct nfs_string *dst,
+				     struct compat_nfs_string *src)
+{
+	dst->data = compat_ptr(src->data);
+	dst->len = src->len;
+}
+
+struct compat_nfs4_mount_data_v1 {
+	compat_int_t version;
+	compat_int_t flags;
+	compat_int_t rsize;
+	compat_int_t wsize;
+	compat_int_t timeo;
+	compat_int_t retrans;
+	compat_int_t acregmin;
+	compat_int_t acregmax;
+	compat_int_t acdirmin;
+	compat_int_t acdirmax;
+	struct compat_nfs_string client_addr;
+	struct compat_nfs_string mnt_path;
+	struct compat_nfs_string hostname;
+	compat_uint_t host_addrlen;
+	compat_uptr_t __user host_addr;
+	compat_int_t proto;
+	compat_int_t auth_flavourlen;
+	compat_uptr_t __user auth_flavours;
+};
+
+static int do_nfs4_super_data_conv(void *raw_data)
+{
+	int version = *(compat_uint_t *) raw_data;
+
+	if (version == 1) {
+		struct compat_nfs4_mount_data_v1 *raw = raw_data;
+		struct nfs4_mount_data *real = raw_data;
+
+		/* copy the fields backwards */
+		real->auth_flavours = compat_ptr(raw->auth_flavours);
+		real->auth_flavourlen = raw->auth_flavourlen;
+		real->proto = raw->proto;
+		real->host_addr = compat_ptr(raw->host_addr);
+		real->host_addrlen = raw->host_addrlen;
+		compat_nfs_string(&real->hostname, &raw->hostname);
+		compat_nfs_string(&real->mnt_path, &raw->mnt_path);
+		compat_nfs_string(&real->client_addr, &raw->client_addr);
+		real->acdirmax = raw->acdirmax;
+		real->acdirmin = raw->acdirmin;
+		real->acregmax = raw->acregmax;
+		real->acregmin = raw->acregmin;
+		real->retrans = raw->retrans;
+		real->timeo = raw->timeo;
+		real->wsize = raw->wsize;
+		real->rsize = raw->rsize;
+		real->flags = raw->flags;
+		real->version = raw->version;
+	}
+	else {
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 extern int copy_mount_options (const void __user *, unsigned long *);
 
 #define SMBFS_NAME      "smbfs"
 #define NCPFS_NAME      "ncpfs"
+#define NFS4_NAME	"nfs4"
 
 asmlinkage long compat_sys_mount(char __user * dev_name, char __user * dir_name,
 				 char __user * type, unsigned long flags,
@@ -785,6 +855,9 @@ asmlinkage long compat_sys_mount(char __
 			do_smb_super_data_conv((void *)data_page);
 		} else if (!strcmp((char *)type_page, NCPFS_NAME)) {
 			do_ncp_super_data_conv((void *)data_page);
+		} else if (!strcmp((char *)type_page, NFS4_NAME)) {
+			if (do_nfs4_super_data_conv((void *) data_page))
+				goto out4;
 		}
 	}
 
@@ -793,6 +866,7 @@ asmlinkage long compat_sys_mount(char __
 			flags, (void*)data_page);
 	unlock_kernel();
 
+ out4:
 	free_page(data_page);
  out3:
 	free_page(dev_page);
#1 lore raw

Hi David,

fs/compat.c static void *do_smb_super_data_conv(void · line 752

Quoted 8 lines · On Fri, 15 Apr 2005 15:45:02 +0100 David Howells <[email protected]> wrote:
fs/compat.c renamed from fs/compat.c.orig +3 -0
@@ -746,10 +747,79 @@ static void *do_smb_super_data_conv(void
 	return raw_data;
 }
 
+struct compat_nfs_string {
+	compat_uint_t len;
+	compat_uptr_t __user data;
                      ^^^^^^

This __user (and the others later) add nothing (I think) as compat_uptr_t is generally just u32 and compat_ptr() does the right casting.

Otherwise, the patch looks fine for a minimal fix. I agree with David and Bryan that we need to get the fs specific stuff out of compat.c in the longer term.

Signature · 3 lines
Cheers,
Stephen Rothwell                    [email protected]
http://www.canb.auug.org.au/~sfr/
#2 lore raw

Using __user with compat_uptr_t

Hi Linus,

I've added a couple of structures for dealing with 32-bit -> 64-bit upgrade of NFS4 mounts. They represent the NFS4 mount information provided by userspace and they contain some pointers to further userspace data. Should these further userspace pointers be labelled __user?

For example, is this right?:

	struct compat_nfs_string {
		compat_uint_t len;
		compat_uptr_t __user data;
	};

Or is this right?:

	struct compat_nfs_string {
		compat_uint_t len;
		compat_uptr_t data;
	};

Now it makes no difference to the compiler, but it might affect the checker tool.

David

#3 lore raw

Re: Using __user with compat_uptr_t

Quoted 7 lines · David Howells

For example, is this right?:

	struct compat_nfs_string {
		compat_uint_t len;
		compat_uptr_t __user data;
	};

Nope. That makes "data" itself (the compat-pointer, not the thing it points to) be marked as being in user space, which is not right.

Quoted 6 lines

Or is this right?:

	struct compat_nfs_string {
		compat_uint_t len;
		compat_uptr_t data;
	};

Yes. Now, when you use "compat_ptr()" on that thing, compat_ptr() will do the proper thing, and results in a "void __user *", so assuming you convert the pointer correctly, it will always end up having the right user annotation.

(Except on MIPS, where compat_ptr() doesn't, but MIPS hasn't been updated to do any sparse checking anyway).

Linus

#4 lore raw

PATCH 1/1 NFS4: Don't use __user with compat_uptr_t

The attached patch removes __user from compat_uptr_t types in the NFS4 mount 32-bit->64-bit compatibility structures.

---
warthog>diffstat -p1 nfs4-compat-2612rc3.diff

 fs/compat.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
fs/compat.c +3 -3
diff -uNrp linux-2.6.12-rc3/fs/compat.c linux-2.6.12-rc3-nfs4compat/fs/compat.c
@@ -809,7 +809,7 @@ static void *do_smb_super_data_conv(void
 
 struct compat_nfs_string {
 	compat_uint_t len;
-	compat_uptr_t __user data;
+	compat_uptr_t data;
 };
 
 static inline void compat_nfs_string(struct nfs_string *dst,
@@ -834,10 +834,10 @@ struct compat_nfs4_mount_data_v1 {
 	struct compat_nfs_string mnt_path;
 	struct compat_nfs_string hostname;
 	compat_uint_t host_addrlen;
-	compat_uptr_t __user host_addr;
+	compat_uptr_t host_addr;
 	compat_int_t proto;
 	compat_int_t auth_flavourlen;
-	compat_uptr_t __user auth_flavours;
+	compat_uptr_t auth_flavours;
 };
 
 static int do_nfs4_super_data_conv(void *raw_data)