| Message ID | 4a4634330902030644k319feb09v20b639320912b229@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Feb 3, 2009 at 8:44 AM, Shirish Pargaonkar <shirishpargaonkar@gmail.com> wrote: > On Mon, Feb 2, 2009 at 10:52 AM, Shirish Pargaonkar > <shirishpargaonkar@gmail.com> wrote: >> Two different patches for the same functionality, to remove the >> remaining entry in /etc/mtab >> after a filesystem is unmounted by canonicalizing the supplied >> mountpoint on the command line. >> >> Please refer to bug 4370 in samba bugzilla. >> >> Regards, >> >> Shirish >> > > version of the patch after making a change suggested by Jeff Layton. > > I'm wonder though whether we need a umount.cifs program at all. All the > teardown is done in-kernel. Is there some reason we can't just rely on > the normal util-linux-ng umount program? Jeff, I tested with /bin/umount and that is good enough. so if we want to get rid of umount.cifs, I have no problem. Regards, Shirish
On Wed, 4 Feb 2009 12:22:40 -0600 Shirish Pargaonkar <shirishpargaonkar@gmail.com> wrote: > On Tue, Feb 3, 2009 at 8:44 AM, Shirish Pargaonkar > <shirishpargaonkar@gmail.com> wrote: > > On Mon, Feb 2, 2009 at 10:52 AM, Shirish Pargaonkar > > <shirishpargaonkar@gmail.com> wrote: > >> Two different patches for the same functionality, to remove the > >> remaining entry in /etc/mtab > >> after a filesystem is unmounted by canonicalizing the supplied > >> mountpoint on the command line. > >> > >> Please refer to bug 4370 in samba bugzilla. > >> > >> Regards, > >> > >> Shirish > >> > > > > version of the patch after making a change suggested by Jeff Layton. > > > > > I'm wonder though whether we need a umount.cifs program at all. All the > > teardown is done in-kernel. Is there some reason we can't just rely on > > the normal util-linux-ng umount program? > > Jeff, > > I tested with /bin/umount and that is good enough. so if we want to get rid of > umount.cifs, I have no problem. > Good to know. After I sent that I had a look at the code. It seems like there is some code in there to handle umounts by unprivileged users. I'm not certain how important that is however, and whether the normal umount command is sufficient to handle that situation as well. I think for now, we should go ahead and apply your patch. I'll see if I can do some research when I get some time and determine whether we might be able to get rid of umount.cifs altogether. I'll plan to apply the patch tomorrow... Cheers,
On Wed, 4 Feb 2009 20:58:08 -0500 Jeff Layton <jlayton@redhat.com> wrote: > On Wed, 4 Feb 2009 12:22:40 -0600 > Shirish Pargaonkar <shirishpargaonkar@gmail.com> wrote: > > > On Tue, Feb 3, 2009 at 8:44 AM, Shirish Pargaonkar > > <shirishpargaonkar@gmail.com> wrote: > > > On Mon, Feb 2, 2009 at 10:52 AM, Shirish Pargaonkar > > > <shirishpargaonkar@gmail.com> wrote: > > >> Two different patches for the same functionality, to remove the > > >> remaining entry in /etc/mtab > > >> after a filesystem is unmounted by canonicalizing the supplied > > >> mountpoint on the command line. > > >> > > >> Please refer to bug 4370 in samba bugzilla. > > >> > > >> Regards, > > >> > > >> Shirish > > >> > > > > > > version of the patch after making a change suggested by Jeff Layton. > > > > > > > > I'm wonder though whether we need a umount.cifs program at all. All the > > > teardown is done in-kernel. Is there some reason we can't just rely on > > > the normal util-linux-ng umount program? > > > > Jeff, > > > > I tested with /bin/umount and that is good enough. so if we want to get rid of > > umount.cifs, I have no problem. > > > > Good to know. After I sent that I had a look at the code. It seems like > there is some code in there to handle umounts by unprivileged users. > I'm not certain how important that is however, and whether the normal > umount command is sufficient to handle that situation as well. > > I think for now, we should go ahead and apply your patch. I'll see if I > can do some research when I get some time and determine whether we > might be able to get rid of umount.cifs altogether. > Patch committed to master, v3-3-test, v3-2-test, v3-0-test branches. Cheers,
--- client.orig/umount.cifs.c 2009-02-02 04:35:20.000000000 -0600 +++ client/umount.cifs.c 2009-02-03 03:28:32.000000000 -0600 @@ -33,6 +33,7 @@ #include <errno.h> #include <string.h> #include <mntent.h> +#include <limits.h> #include "mount.h" #define UNMOUNT_CIFS_VERSION_MAJOR "0" @@ -231,6 +232,37 @@ static int remove_from_mtab(char * mount return rc; } +/* Make a canonical pathname from PATH. Returns a freshly malloced string. + It is up the *caller* to ensure that the PATH is sensible. i.e. + canonicalize ("/dev/fd0/.") returns "/dev/fd0" even though ``/dev/fd0/.'' + is not a legal pathname for ``/dev/fd0'' Anything we cannot parse + we return unmodified. */ +static char * +canonicalize(char *path) +{ + char *canonical = malloc (PATH_MAX + 1); + + if (!canonical) { + fprintf(stderr, "Error! Not enough memory!\n"); + return NULL; + } + + if (strlen(path) > PATH_MAX) { + fprintf(stderr, "Mount point string too long\n"); + return NULL; + } + + if (path == NULL) + return NULL; + + if (realpath (path, canonical)) + return canonical; + + strncpy (canonical, path, PATH_MAX); + canonical[PATH_MAX] = '\0'; + return canonical; +} + int main(int argc, char ** argv) { int c; @@ -304,7 +336,7 @@ int main(int argc, char ** argv) argv += optind; argc -= optind; - mountpoint = argv[0]; + mountpoint = canonicalize(argv[0]); if((argc < 1) || (argv[0] == NULL)) { printf("\nMissing name of unmount directory\n");