nfs-utils: mount: non-nfs mount options

PATCH In review Steve Dickson 3 participants 5 messages Feb 26, 2007 – Mar 22, 2007

Subsystems (2)

NFS client NFS server

lore

Share & tools

lore

Steve Dickson

author

2007-02-26 11:14 UTC #0 lore raw

PATCH 1/11 nfs-utils: mount: non-nfs mount options

commit 806b02f63fe6e01946d84a1f14c74e977be76530

Author: Amit Gud <[email protected]>
Date:   Sat Feb 24 14:31:13 2007 -0500

Added the following non-nfs mount options as suggested but upstream code reviews:

[no]auto, [no]users, [no]user, [no]owner, [no]group
_netdev, comment

Signed-off-by Amit Gud <[email protected]>

Signed-off-by Steve Dickson <[email protected]>

support/include/fstab.h +8 -0

index 52af743..1134a2a 100644
@@ -3,6 +3,10 @@
 
 #include "nfs_mntent.h"
 
+#ifndef _PATH_FSTAB
+#define _PATH_FSTAB "/etc/fstab"
+#endif
+
 int mtab_is_writable(void);
 int mtab_does_not_exist(void);

@@ -16,6 +20,10 @@ struct mntentchn *getmntoptfile (const char *file);
 struct mntentchn *getmntdirbackward (const char *dir, struct mntentchn *mc);
 struct mntentchn *getmntdevbackward (const char *dev, struct mntentchn *mc);
 
+struct mntentchn *fstab_head (void);
+struct mntentchn *getfsfile (const char *file);
+struct mntentchn *getfsspec (const char *spec);
+
 void lock_mtab (void);
 void unlock_mtab (void);
 void update_mtab (const char *special, nfs_mntent_t *with);

support/include/nfs_mntent.h +4 -4

index 76f348e..ec5dfd4 100644
@@ -7,10 +7,10 @@
 #define _NFS_MNTENT_H
 
 typedef struct nfs_mntent_s {
-	const char *mnt_fsname;
-	const char *mnt_dir;
-	const char *mnt_type;
-	const char *mnt_opts;
+	char *mnt_fsname;
+	char *mnt_dir;
+	char *mnt_type;
+	char *mnt_opts;
 	int mnt_freq;
 	int mnt_passno;
 } nfs_mntent_t;

support/nfs/fstab.c +54 -3

82 lines not on this page · raw message

utils/mount/Makefile.am +4 -3

index 4a2f437..1fbbd1d 100644
@@ -14,9 +14,10 @@ MAINTAINERCLEANFILES = Makefile.in
 
 install-exec-hook:
 	(cd $(DESTDIR)$(sbindir) && \
-	  ln -sf $(sbin_PROGRAMS) mount.nfs4 && \
-	  ln -sf $(sbin_PROGRAMS) umount.nfs && \
-	  ln -sf $(sbin_PROGRAMS) umount.nfs4)
+	  chmod +s $(sbin_PROGRAMS) && \
+	  cp -p $(sbin_PROGRAMS) $(DESTDIR)$(sbindir)/mount.nfs4 && \
+	  cp -p $(sbin_PROGRAMS) $(DESTDIR)$(sbindir)/umount.nfs && \
+	  cp -p $(sbin_PROGRAMS) $(DESTDIR)$(sbindir)/umount.nfs4)
 uninstall-hook:
 	(cd $(DESTDIR)$(sbindir) && \
 	    rm -f mount.nfs4 umount.nfs umount.nfs4)

utils/mount/mount.c +123 ~~-18~~

231 lines not on this page · raw message

utils/mount/nfs_mount.h +2 -0

index 1fd74c8..2200362 100644
@@ -80,5 +80,7 @@ struct nfs_mount_data {
 
 int nfsmount(const char *, const char *, int *, char **, char **, int *, int);
 void mount_errors(char *, int, int);
+int contains(const char *, const char *);
+char *get_value(const char *, const char *);
 
 #endif /* _NFS_MOUNT_H */

utils/mount/nfsumount.c +18 -1

index 28f4244..dfbcb7c 100644
@@ -23,12 +23,14 @@
 #include <mntent.h>
 #include <sys/mount.h>
 #include <ctype.h>
+#include <pwd.h>
 
 #include "xcommon.h"
 #include "fstab.h"
 #include "nls.h"
 #include "conn.h"
 
+#include "nfs_mount.h"
 #include "mount_constants.h"
 #include "mount.h"
 #include "nfsumount.h"

@@ -307,7 +309,7 @@ int _nfsumount(const char *spec, const char *opts)
 		goto out_bad;
 	return nfs_call_umount(&mnt_server, &dirname);
  out_bad:
-	printf("%s: %s: not found or not mounted\n", progname, spec);
+	fprintf(stderr, "%s: %s: not found or not mounted\n", progname, spec);
 	return 0;
 }

@@ -376,6 +378,21 @@ int nfsumount(int argc, char *argv[])
 		printf(_("Could not find %s in mtab\n"), spec);
 
 	if(mc) {
+		if(contains(mc->m.mnt_opts, "user") && getuid() != 0) {
+			struct passwd *pw = getpwuid(getuid());
+			if(!pw || strcmp(pw->pw_name, get_value(mc->m.mnt_opts, "user="))) {
+				fprintf(stderr, "%s: permission denied to unmount %s\n",
+						progname, spec);
+				exit(1);
+			}
+		} else {
+			if(!contains(mc->m.mnt_opts, "users") && getuid() != 0) {
+				fprintf(stderr, "%s: only root can unmount %s from %s\n",
+						progname, mc->m.mnt_fsname, mc->m.mnt_dir);
+				exit(1);
+			}
+		}
+
 		ret = _nfsumount(mc->m.mnt_fsname, mc->m.mnt_opts);
 		if(ret)
 			ret = add_mtab2(mc->m.mnt_fsname, mc->m.mnt_dir,

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

Neil Brown

2007-02-27 06:06 UTC #1 lore raw

Quoted 9 lines · On Monday February 26, [email protected] wrote:

commit 806b02f63fe6e01946d84a1f14c74e977be76530
Author: Amit Gud <[email protected]>
Date:   Sat Feb 24 14:31:13 2007 -0500
Added the following non-nfs mount options as suggested but upstream code reviews:
[no]auto, [no]users, [no]user, [no]owner, [no]group
_netdev, comment

This patch does a lot more than that brief description suggests.

- The 'users' / 'user' changes are enough to be a patch in itself, particularly as it is a security issue and needs careful review. mount and umount do similar tests here and it would be good to include them in the library (nfs/fstab.c).

- 'const' is removed from nfs_mntent_s, and copy_mntent is introduced .. are those related?

- You copy instead of link the 4 different mount.* programs, which might be a good thing, but deserves a comment in the changelog.

- Some printf change to fprintf, which is good, but seems to be independent of the rest.

and possible some other things that I didn't notice. As this is a setuid program, I really want to be able to review changes in isolation please.

Thanks,
NeilBrown

Steve Dickson

author

2007-03-16 15:25 UTC #2 lore raw

Quoted 11 lines · Neil Brown

Quoted 9 lines · On Monday February 26, [email protected] wrote:
commit 806b02f63fe6e01946d84a1f14c74e977be76530
Author: Amit Gud <[email protected]>
Date:   Sat Feb 24 14:31:13 2007 -0500
Added the following non-nfs mount options as suggested but upstream code reviews:
[no]auto, [no]users, [no]user, [no]owner, [no]group
_netdev, comment

Quoted 6 lines

This patch does a lot more than that brief description suggests.
- The 'users' / 'user' changes are enough to be a patch in itself, particularly as it is a security issue and needs careful review. mount and umount do similar tests here and it would be good to include them in the library (nfs/fstab.c).

I believe this was just a port of nfs/fstab.c which was need, but I forget why... Amit would you happen to remember?

Quoted 3 lines

- 'const' is removed from nfs_mntent_s, and copy_mntent is introduced .. are those related?

Yes... these have to do with the remount support, which was part of getting all the non-nfs options to work..

The 'const' was remove to stop the following warning:
mount.c: In function 'copy_mntent':
mount.c:168: warning: passing argument 1 of '__builtin___strcpy_chk' discards qualifiers from pointer target type

Quoted 3 lines

- You copy instead of link the 4 different mount.* programs, which might be a good thing, but deserves a comment in the changelog.

This was because of set-uid issues... I believe Amit tried to use links but there was an issue with it that cause us to do the copies...

Quoted 3 lines

- Some printf change to fprintf, which is good, but seems to be independent of the rest.

This was general clean up...

Quoted 4 lines

and possible some other things that I didn't notice. As this is a setuid program, I really want to be able to review changes in isolation please.

Ok... I broke the original patch up into 3 separate ones. Pleas see attachments...

steved.

Author: Amit Gud <[email protected]>
Date:   Sat Feb 24 14:31:13 2007 -0500

Added the following non-nfs mount options as suggested but upstream code reviews:

[no]auto, [no]users, [no]user, [no]owner, [no]group
_netdev, comment, remount

Signed-off-by Steve Dickson <[email protected]>

------------------------------------------------

utils/mount/nfsumount.c renamed from utils/mount/nfsumount.c.orig +17 -0

@@ -23,12 +23,14 @@
 #include <mntent.h>
 #include <sys/mount.h>
 #include <ctype.h>
+#include <pwd.h>
 
 #include "xcommon.h"
 #include "fstab.h"
 #include "nls.h"
 #include "conn.h"
 
+#include "nfs_mount.h"
 #include "mount_constants.h"
 #include "mount.h"
 #include "nfsumount.h"

@@ -376,6 +378,21 @@ int nfsumount(int argc, char *argv[])
 		printf(_("Could not find %s in mtab\n"), spec);
 
 	if(mc) {
+		if(contains(mc->m.mnt_opts, "user") && getuid() != 0) {
+			struct passwd *pw = getpwuid(getuid());
+			if(!pw || strcmp(pw->pw_name, get_value(mc->m.mnt_opts, "user="))) {
+				fprintf(stderr, "%s: permission denied to unmount %s\n",
+						progname, spec);
+				exit(1);
+			}
+		} else {
+			if(!contains(mc->m.mnt_opts, "users") && getuid() != 0) {
+				fprintf(stderr, "%s: only root can unmount %s from %s\n",
+						progname, mc->m.mnt_fsname, mc->m.mnt_dir);
+				exit(1);
+			}
+		}
+
 		ret = _nfsumount(mc->m.mnt_fsname, mc->m.mnt_opts);
 		if(ret)
 			ret = add_mtab2(mc->m.mnt_fsname, mc->m.mnt_dir,

utils/mount/mount.c renamed from utils/mount/mount.c.orig +119 ~~-14~~

213 lines not on this page · raw message

utils/mount/nfs_mount.h renamed from utils/mount/nfs_mount.h.orig +2 -0

@@ -80,5 +80,7 @@ struct nfs_mount_data {
 
 int nfsmount(const char *, const char *, int *, char **, char **, int *, int);
 void mount_errors(char *, int, int);
+int contains(const char *, const char *);
+char *get_value(const char *, const char *);
 
 #endif /* _NFS_MOUNT_H */

support/include/nfs_mntent.h renamed from support/include/nfs_mntent.h.orig +4 -4

@@ -7,10 +7,10 @@
 #define _NFS_MNTENT_H
 
 typedef struct nfs_mntent_s {
-	const char *mnt_fsname;
-	const char *mnt_dir;
-	const char *mnt_type;
-	const char *mnt_opts;
+	char *mnt_fsname;
+	char *mnt_dir;
+	char *mnt_type;
+	char *mnt_opts;
 	int mnt_freq;
 	int mnt_passno;
 } nfs_mntent_t;

support/include/fstab.h renamed from support/include/fstab.h.orig +8 -0

@@ -3,6 +3,10 @@
 
 #include "nfs_mntent.h"
 
+#ifndef _PATH_FSTAB
+#define _PATH_FSTAB "/etc/fstab"
+#endif
+
 int mtab_is_writable(void);
 int mtab_does_not_exist(void);

@@ -16,6 +20,10 @@ struct mntentchn *getmntoptfile (const c
 struct mntentchn *getmntdirbackward (const char *dir, struct mntentchn *mc);
 struct mntentchn *getmntdevbackward (const char *dev, struct mntentchn *mc);
 
+struct mntentchn *fstab_head (void);
+struct mntentchn *getfsfile (const char *file);
+struct mntentchn *getfsspec (const char *spec);
+
 void lock_mtab (void);
 void unlock_mtab (void);
 void update_mtab (const char *special, nfs_mntent_t *with);

support/nfs/fstab.c renamed from support/nfs/fstab.c.orig +54 -3

82 lines not on this page · raw message

Author: Amit Gud <[email protected]>
Date:   Sat Feb 24 14:31:13 2007 -0500

Due set uid/gid issues, actual copies of the mount binaries are needed.

Signed-off-by Steve Dickson <[email protected]>

-----------------------------------------------

utils/mount/Makefile.am renamed from utils/mount/Makefile.am.orig +4 -3

@@ -14,9 +14,10 @@ MAINTAINERCLEANFILES = Makefile.in
 
 install-exec-hook:
 	(cd $(DESTDIR)$(sbindir) && \
-	  ln -sf $(sbin_PROGRAMS) mount.nfs4 && \
-	  ln -sf $(sbin_PROGRAMS) umount.nfs && \
-	  ln -sf $(sbin_PROGRAMS) umount.nfs4)
+	  chmod +s $(sbin_PROGRAMS) && \
+	  cp -p $(sbin_PROGRAMS) $(DESTDIR)$(sbindir)/mount.nfs4 && \
+	  cp -p $(sbin_PROGRAMS) $(DESTDIR)$(sbindir)/umount.nfs && \
+	  cp -p $(sbin_PROGRAMS) $(DESTDIR)$(sbindir)/umount.nfs4)
 uninstall-hook:
 	(cd $(DESTDIR)$(sbindir) && \
 	    rm -f mount.nfs4 umount.nfs umount.nfs4)

Author: Amit Gud <[email protected]>
Date:   Sat Feb 24 14:31:13 2007 -0500

Clean up of some error messages.

Signed-off-by Steve Dickson <[email protected]>

-----------------------------------------------

utils/mount/nfsumount.c renamed from utils/mount/nfsumount.c.orig +1 -1

@@ -309,7 +309,7 @@ int _nfsumount(const char *spec, const c
 		goto out_bad;
 	return nfs_call_umount(&mnt_server, &dirname);
  out_bad:
-	printf("%s: %s: not found or not mounted\n", progname, spec);
+	fprintf(stderr, "%s: %s: not found or not mounted\n", progname, spec);
 	return 0;
 }

utils/mount/mount.c renamed from utils/mount/mount.c.orig +4 -4

@@ -324,16 +324,16 @@ static void mount_error(char *node)
 {
 	switch(errno) {
 		case ENOTDIR:
-			printf("%s: mount point %s is not a directory\n", progname, node);
+			fprintf(stderr, "%s: mount point %s is not a directory\n", progname, node);
 			break;
 		case EBUSY:
-			printf("%s: %s is already mounted or busy\n", progname, node);
+			fprintf(stderr, "%s: %s is already mounted or busy\n", progname, node);
 			break;
 		case ENOENT:
-			printf("%s: mount point %s does not exist\n", progname, node);
+			fprintf(stderr, "%s: mount point %s does not exist\n", progname, node);
 			break;
 		default:
-			printf("%s: %s\n", progname, strerror(errno));
+			fprintf(stderr, "%s: %s\n", progname, strerror(errno));
 	}
 }

_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

Neil Brown

2007-03-18 23:41 UTC #3 lore raw

Quoted 7 lines · On Friday March 16, [email protected] wrote:

Quoted 5 lines
and possible some other things that I didn't notice. As this is a setuid program, I really want to be able to review changes in isolation please.
Ok... I broke the original patch up into 3 separate ones. Pleas see attachments...

Thanks. I've applied the last.
I'd really like to understand the ln/cp issue before applying. And I'm not quite ready to turn on setuid yet. Maybe when I do that, testing will show something.

The first patch - I have already applied that conceptually.. A lot of the code is different as I wasn't very comfortable with what was there.

e.g. When checking that a 'user' mount was in fstab, it just checked that the mountpoint or filesystem was listed in fstab with the 'user' flag. It didn't check that the mountpoint and filesystem and options all matched.
So I think that if you put in fstab that host:/place was allowed to be mounted on /foo by any user, than any user could run mount.nfs directly, ask for host:/place to be mounted on /etc and it would work.

My version is at
http://linux-nfs.org/cgi-bin/gitweb.cgi?p=nfs-utils;a=commitdiff;h=ae5ec51e0ddc99dc5552e51bd5c095084dbb61aa

Thanks,
NeilBrown

Amit Gud

2007-03-22 21:40 UTC #4 lore raw

Quoted 4 lines · Neil Brown

Quoted 3 lines · On Friday March 16, [email protected] wrote:
Quoted 3 lines
and possible some other things that I didn't notice. As this is a setuid program, I really want to be able to review changes in isolation please.

Quoted 8 lines

Quoted 2 lines
Ok... I broke the original patch up into 3 separate ones. Pleas see attachments...
Thanks. I've applied the last.
I'd really like to understand the ln/cp issue before applying. And I'm not quite ready to turn on setuid yet. Maybe when I do that, testing will show something.

I was not able to retain the setuid bit with ln, so I copied the files over with -p option.

Signature · 12 lines

May the source be with you.
http://www.cis.ksu.edu/~gud


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs