[linux-cifs-client] clean-up entries in /etc/mtab after unmounting a cifs filesystem
diff mbox

Message ID 4a4634330902030644k319feb09v20b639320912b229@mail.gmail.com
State New, archived
Headers show

Commit Message

Shirish Pargaonkar Feb. 3, 2009, 2:44 p.m. UTC
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.

Comments

Shirish Pargaonkar Feb. 4, 2009, 6:22 p.m. UTC | #1
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
Jeff Layton Feb. 5, 2009, 1:58 a.m. UTC | #2
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,
Jeff Layton Feb. 5, 2009, 7:21 p.m. UTC | #3
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,

Patch
diff mbox

--- 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");