smbmount maintainer/responsible/guilty/patch-reader wanted

Urban Widmark urban at svenskatest.se
Thu Jul 13 14:12:46 GMT 2000


Hello

As you probably don't know (I sure haven't told almost anyone yet :)
Tridge has handed over maintaining smbfs to me. Yes, I know smbfs and
smbmount are "ugly ducklings" in the samba world ... this still seemed
like the best place to find someone responsible for smbmount.


One of the problems I would like to fix is the dependency between the
kernel sources (includes) and smbmount. This is a major problem when
trying to add something to smbfs that needs some form of user input.

smbmount includes these two (smbumount & smbmnt has the same problem)
	#include <asm/types.h>
	#include <linux/smb_fs.h>
and expects some of the structs in there to match it's knowledge of the
world. The same for some ioctl definitions.


The problem can be illustrated like this:

smbfs changes smb_mount_data to accept some new flags (say nfs-style intr
& soft). Increases the version number on the struct but also accepts the
previous version using default values to avoid breaking old mounts.

smbmount in cvs is changed to use these new options. Someone tries to
compile cvs smbmount with an old kernel source tree, and fails because the
struct has missing fields.

If smbmount is not changed and compiled vs new sources the problem is that
smbmount sends a version 7 struct with only the version 6 fields
initialized (it would be fine if it set the version to 6, but it grabs the
kernel define which now says 7 ...). smbfs can't know this and will fail.

The same problem happens when adding an ioctl, new smbmount with old
kernel includes -> compile error.


Attached is an example patch to show how to get away from this by copying 
some defines to the smbmount sources.

It should NOT be applied to anything, it is intended as a basis for
discussion. The smbumount change is an example and probably broken. There
is also no point in doing that until you can mount using a 32bit uid (and
guess what, that requires changing the smb_mount_data struct or adding an
ugly ioctl).

Cc to Michael since I know he knows smbmount, but I don't know if he is
on this list.


/Urban
-------------- next part --------------
diff -urN --exclude-from=/usr/src/exclude samba-2.0.7-orig/source/client/linux_smbfs.h samba-2.0.7/source/client/linux_smbfs.h
--- samba-2.0.7-orig/source/client/linux_smbfs.h	Thu Jan  1 01:00:00 1970
+++ samba-2.0.7/source/client/linux_smbfs.h	Thu Jul 13 15:26:38 2000
@@ -0,0 +1,106 @@
+/*
+ * Defines from the linux kernel smbfs includes.
+ *
+ * "userspace" programs should not include kernel includes directly,
+ * it causes unwanted dependencies. These defines will be updated when
+ * support for new smbfs features are added to smbmount.
+ *
+ * Copyright (C) Urban Widmark 2000
+ * (if you can get (C) on a cut&paste job ... :)
+ */
+
+/* FIXME: this is cheating ... */
+#include <linux/posix_types.h>
+#include <asm/types.h>
+
+/* Notice how we run into version dependency trouble even for simple
+   things like defining the size of a basic type.
+
+   2.4 defines this, 2.2 doesn't. smbmount should compile both on
+   systems with 2.2 and 2.4 includes, no? */
+#ifndef __kernel_uid32_t
+typedef unsigned int __kernel_uid32_t;
+#endif
+
+
+/* This version of smbmount knows about this version, smbfs may know
+   about a lot of other versions too but we don't have to care. */
+#define SMB_MOUNT_VERSION 6
+
+
+/* For the initial mount */
+
+struct smb_mount_data {
+	int version;
+	__kernel_uid_t mounted_uid; /* Who may umount() this filesystem? */
+	__kernel_uid_t uid;
+	__kernel_gid_t gid;
+	__kernel_mode_t file_mode;
+	__kernel_mode_t dir_mode;
+};
+
+
+/* For passing a connection */
+
+enum smb_protocol { 
+	SMB_PROTOCOL_NONE, 
+	SMB_PROTOCOL_CORE, 
+	SMB_PROTOCOL_COREPLUS, 
+	SMB_PROTOCOL_LANMAN1, 
+	SMB_PROTOCOL_LANMAN2, 
+	SMB_PROTOCOL_NT1 
+};
+
+enum smb_case_hndl {
+	SMB_CASE_DEFAULT,
+	SMB_CASE_LOWER,
+	SMB_CASE_UPPER
+};
+
+struct smb_conn_opt {
+
+        /* The socket */
+	unsigned int fd;
+
+	enum smb_protocol protocol;
+	enum smb_case_hndl case_handling;
+
+	/* Connection-Options */
+	__u32              max_xmit;
+	__u16              server_uid;
+	__u16              tid;
+
+        /* The following are LANMAN 1.0 options */
+        __u16              secmode;
+        __u16              maxmux;
+        __u16              maxvcs;
+        __u16              rawmode;
+        __u32              sesskey;
+
+	/* The following are NT LM 0.12 options */
+	__u32              maxraw;
+	__u32              capabilities;
+	__s16              serverzone;
+};
+
+
+/* For ioctls */
+
+/* undef all first in case some system has picked these up from some
+   funky include files (smbumount.c used to do this, I assume there
+   was a reason). */
+#undef SMB_IOC_GETMOUNTUID
+#undef SMB_IOC_NEWCONN
+#undef SMB_IOC_GETMOUNTUID32
+#define SMB_IOC_GETMOUNTUID		_IOR('u', 1, __kernel_uid_t)
+#define SMB_IOC_NEWCONN                 _IOW('u', 2, struct smb_conn_opt)
+#define SMB_IOC_GETMOUNTUID32           _IOR('u', 3, __kernel_uid32_t)
+
+
+#ifndef	MS_MGC_VAL
+/* This may look strange but MS_MGC_VAL is what we are looking for and
+	is what we need from <linux/fs.h> under libc systems and is
+	provided in standard includes on glibc systems.  So...  We
+	switch on what we need...  */
+#define MS_MGC_VAL 0xC0ED0000
+#endif
diff -urN --exclude-from=/usr/src/exclude samba-2.0.7-orig/source/client/smbmnt.c samba-2.0.7/source/client/smbmnt.c
--- samba-2.0.7-orig/source/client/smbmnt.c	Wed Oct 13 07:26:45 1999
+++ samba-2.0.7/source/client/smbmnt.c	Thu Jul 13 14:48:02 2000
@@ -9,20 +9,7 @@
 #include "includes.h"
 
 #include <mntent.h>
-
-#include <asm/types.h>
-#include <asm/posix_types.h>
-#include <linux/smb.h>
-#include <linux/smb_mount.h>
-#include <asm/unistd.h>
-
-#ifndef	MS_MGC_VAL
-/* This may look strange but MS_MGC_VAL is what we are looking for and
-	is what we need from <linux/fs.h> under libc systems and is
-	provided in standard includes on glibc systems.  So...  We
-	switch on what we need...  */
-#include <linux/fs.h>
-#endif
+#include "linux_smbfs.h"
 
 static uid_t mount_uid;
 static gid_t mount_gid;
diff -urN --exclude-from=/usr/src/exclude samba-2.0.7-orig/source/client/smbmount.c samba-2.0.7/source/client/smbmount.c
--- samba-2.0.7-orig/source/client/smbmount.c	Wed Apr 26 01:06:42 2000
+++ samba-2.0.7/source/client/smbmount.c	Thu Jul 13 14:35:31 2000
@@ -24,8 +24,7 @@
 #include "includes.h"
 
 #include <mntent.h>
-#include <asm/types.h>
-#include <linux/smb_fs.h>
+#include "linux_smbfs.h"
 
 /* Uncomment this to allow debug the mount.smb daemon */
 /* WARNING!  This option is incompatible with autofs/automount because
diff -urN --exclude-from=/usr/src/exclude samba-2.0.7-orig/source/client/smbumount.c samba-2.0.7/source/client/smbumount.c
--- samba-2.0.7-orig/source/client/smbumount.c	Wed Oct 13 07:26:46 1999
+++ samba-2.0.7/source/client/smbumount.c	Thu Jul 13 15:12:54 2000
@@ -9,19 +9,7 @@
 
 #include <mntent.h>
 
-#include <asm/types.h>
-#include <asm/posix_types.h>
-#include <linux/smb.h>
-#include <linux/smb_mount.h>
-#include <linux/smb_fs.h>
-
-/* This is a (hopefully) temporary hack due to the fact that
-	sizeof( uid_t ) != sizeof( __kernel_uid_t ) under glibc.
-	This may change in the future and smb.h may get fixed in the
-	future.  In the mean time, it's ugly hack time - get over it.
-*/
-#undef SMB_IOC_GETMOUNTUID
-#define	SMB_IOC_GETMOUNTUID		_IOR('u', 1, __kernel_uid_t)
+#include "linux_smbfs.h"
 
 #ifndef O_NOFOLLOW
 #define O_NOFOLLOW     0400000
@@ -39,22 +27,30 @@
 	/* we set O_NOFOLLOW to prevent users playing games with symlinks to
 	   umount filesystems they don't own */
         int fid = open(mount_point, O_RDONLY|O_NOFOLLOW, 0);
-        __kernel_uid_t mount_uid;
+        __kernel_uid32_t mount_uid32;
 	
         if (fid == -1) {
                 fprintf(stderr, "Could not open %s: %s\n",
                         mount_point, strerror(errno));
                 return -1;
         }
-        
-        if (ioctl(fid, SMB_IOC_GETMOUNTUID, &mount_uid) != 0) {
-                fprintf(stderr, "%s probably not smb-filesystem\n",
-                        mount_point);
-                return -1;
-        }
+
+	/* try to read a 32bit uid_t first, if uid_t is 32 bit. It may
+           be unsupported by the kernel we are talking to, so quietly
+           try the 16bit if we fail. */
+	if (sizeof(uid_t) != 4 ||
+	    ioctl(fid, SMB_IOC_GETMOUNTUID32, &mount_uid32) != 0) {
+		__kernel_uid_t mount_uid;
+		if (ioctl(fid, SMB_IOC_GETMOUNTUID, &mount_uid) != 0) {
+			fprintf(stderr, "%s probably not smb-filesystem\n",
+				mount_point);
+			return -1;
+		}
+		mount_uid32 = mount_uid;
+	}
 
         if ((getuid() != 0)
-            && (mount_uid != getuid())) {
+            && (mount_uid32 != getuid())) {
                 fprintf(stderr, "You are not allowed to umount %s\n",
                         mount_point);
                 return -1;


More information about the samba-technical mailing list