All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Aurélien Aptel" <aaptel@suse.com>
To: Stanislav Brabec <sbrabec@suse.cz>
Cc: util-linux <util-linux@vger.kernel.org>,
	linux-cifs@vger.kernel.org, Steve French <smfrench@gmail.com>
Subject: Re: [PATCH] libmount: Special handling of root comparison for cifs in mnt_table_is_fs_mounted()
Date: Fri, 02 Sep 2016 20:58:08 +0200	[thread overview]
Message-ID: <mpsbn06ruwv.fsf@suse.com> (raw)
In-Reply-To: <eec8f575-eab2-34bb-882a-11fb8c33d9c0@suse.cz>


[-- Attachment #1.1: Type: text/plain, Size: 1584 bytes --]


Hi Stanislav,

I've looked more into it and I found some issues with the patch:

Stanislav Brabec <sbrabec@suse.cz> writes:
> +	while ((c = *subpath++))
                      ^^^ subpath is always post-incremented
> +	{
> +		if (c == '/') {
> +			if (!last_is_slash) {
> +				component_no++;
> +				if (component_no == 3)
> +					break;
                                         ^^^ when we break subpath
                                             actually 1 past the /
> +			}
> +			last_is_slash = true;
> +		} else
> +			last_is_slash = false;
> +	}
> +	if (component_no == 3) {
> +		int subpath_len = strlen(subpath);
> +		if (strncmp(root, subpath, subpath_len)) {
                             ^^^  ^^^ hence why here
with //localhost/share/dir in fstab we never get here
p root    => $1 = 0x7f28aa9977d0 "/dir"
p subpath => $2 = 0x7f28aa9979a2 "dir"

> +			if (*(root + subpath_len + 1) == 0)
> +				return "/";
> +			return root + subpath_len + 1;
> +		}
> +	}
> +	DBG(FS, ul_debugobj(fs, "cifs: leading part of root \"%s\" does
> not equal to mounted source subdir \"%s\"; should not happen", root,
> src));
^^^ this message gets printed with //localhost/share //localhost/share/ 

That being said, I could not make your patch fail i.e. make mount -a
mount the fs twice. So I'm not sure what happens there... I thought I
would give it a try.

I'm attaching a simpler patch that makes uses of streq_paths() in
lib/strutils.c (Note: streq_paths() and next_path_segment() have to be
backported for older versions of util-linux).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-libmount-src-tab.c-fix-mount-a-for-cifs.patch --]
[-- Type: text/x-patch, Size: 1915 bytes --]

From a29acba1a7c71cdbfc1cc98f2905bfaf19b97a29 Mon Sep 17 00:00:00 2001
From: Aurelien Aptel <aaptel@suse.com>
Date: Thu, 1 Sep 2016 13:57:42 +0200
Subject: [PATCH] libmount/src/tab.c: fix mount -a for cifs

when mounting a cifs share, the src is actually an UNC path which can in
in several forms:

simple:            //host/share, //host/share/
including subpath: //host/share/sub/path

to check if the cifs fs is mounted we have to extract the subpath and
compare *that* to the root.

Signed-off-by: Aurelien Aptel <aaptel@suse.com>
---
 libmount/src/tab.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/libmount/src/tab.c b/libmount/src/tab.c
index 341e5e3..9c49ec8 100644
--- a/libmount/src/tab.c
+++ b/libmount/src/tab.c
@@ -1329,6 +1329,20 @@ err:
 }
 #endif /* HAVE_BTRFS_SUPPORT */
 
+static const char *get_cifs_unc_subdir_path (const char *unc)
+{
+	/*
+	 *  1 or more slash:     %*[/]
+	 *  1 or more non-slash: %*[^/]
+	 *  number of byte read: %n
+	 */
+	int share_end = 0;
+	int r = sscanf(unc, "%*[/]%*[^/]%*[/]%*[^/]%n", &share_end);
+	if (r == EOF || share_end == 0)
+		return NULL;
+	return unc + share_end;
+}
+
 /*
  * tb: /proc/self/mountinfo
  * fs: filesystem
@@ -1563,9 +1577,16 @@ int mnt_table_is_fs_mounted(struct libmnt_table *tb, struct libmnt_fs *fstab_fs)
 		}
 
 		if (root) {
-			const char *r = mnt_fs_get_root(fs);
-			if (!r || strcmp(r, root) != 0)
-				continue;
+			if (strcmp(mnt_fs_get_fstype(fs), "cifs") == 0) {
+				const char *unc_subdir = get_cifs_unc_subdir_path(src);
+				const char *path_on_fs = mnt_fs_get_root(fs);
+				if (!unc_subdir || !path_on_fs || !streq_paths(unc_subdir, path_on_fs))
+					continue;
+			} else {
+				const char *r = mnt_fs_get_root(fs);
+				if (!r || strcmp(r, root) != 0)
+					continue;
+			}
 		}
 
 		/*
-- 
2.1.4


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: "Aurélien Aptel" <aaptel-IBi9RG/b67k@public.gmane.org>
To: Stanislav Brabec <sbrabec-AlSwsSmVLrQ@public.gmane.org>
Cc: util-linux <util-linux-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Subject: Re: [PATCH] libmount: Special handling of root comparison for cifs in mnt_table_is_fs_mounted()
Date: Fri, 02 Sep 2016 20:58:08 +0200	[thread overview]
Message-ID: <mpsbn06ruwv.fsf@suse.com> (raw)
In-Reply-To: <eec8f575-eab2-34bb-882a-11fb8c33d9c0-AlSwsSmVLrQ@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 1605 bytes --]


Hi Stanislav,

I've looked more into it and I found some issues with the patch:

Stanislav Brabec <sbrabec-AlSwsSmVLrQ@public.gmane.org> writes:
> +	while ((c = *subpath++))
                      ^^^ subpath is always post-incremented
> +	{
> +		if (c == '/') {
> +			if (!last_is_slash) {
> +				component_no++;
> +				if (component_no == 3)
> +					break;
                                         ^^^ when we break subpath
                                             actually 1 past the /
> +			}
> +			last_is_slash = true;
> +		} else
> +			last_is_slash = false;
> +	}
> +	if (component_no == 3) {
> +		int subpath_len = strlen(subpath);
> +		if (strncmp(root, subpath, subpath_len)) {
                             ^^^  ^^^ hence why here
with //localhost/share/dir in fstab we never get here
p root    => $1 = 0x7f28aa9977d0 "/dir"
p subpath => $2 = 0x7f28aa9979a2 "dir"

> +			if (*(root + subpath_len + 1) == 0)
> +				return "/";
> +			return root + subpath_len + 1;
> +		}
> +	}
> +	DBG(FS, ul_debugobj(fs, "cifs: leading part of root \"%s\" does
> not equal to mounted source subdir \"%s\"; should not happen", root,
> src));
^^^ this message gets printed with //localhost/share //localhost/share/ 

That being said, I could not make your patch fail i.e. make mount -a
mount the fs twice. So I'm not sure what happens there... I thought I
would give it a try.

I'm attaching a simpler patch that makes uses of streq_paths() in
lib/strutils.c (Note: streq_paths() and next_path_segment() have to be
backported for older versions of util-linux).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-libmount-src-tab.c-fix-mount-a-for-cifs.patch --]
[-- Type: text/x-patch, Size: 1955 bytes --]

From a29acba1a7c71cdbfc1cc98f2905bfaf19b97a29 Mon Sep 17 00:00:00 2001
From: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
Date: Thu, 1 Sep 2016 13:57:42 +0200
Subject: [PATCH] libmount/src/tab.c: fix mount -a for cifs

when mounting a cifs share, the src is actually an UNC path which can in
in several forms:

simple:            //host/share, //host/share/
including subpath: //host/share/sub/path

to check if the cifs fs is mounted we have to extract the subpath and
compare *that* to the root.

Signed-off-by: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
---
 libmount/src/tab.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/libmount/src/tab.c b/libmount/src/tab.c
index 341e5e3..9c49ec8 100644
--- a/libmount/src/tab.c
+++ b/libmount/src/tab.c
@@ -1329,6 +1329,20 @@ err:
 }
 #endif /* HAVE_BTRFS_SUPPORT */
 
+static const char *get_cifs_unc_subdir_path (const char *unc)
+{
+	/*
+	 *  1 or more slash:     %*[/]
+	 *  1 or more non-slash: %*[^/]
+	 *  number of byte read: %n
+	 */
+	int share_end = 0;
+	int r = sscanf(unc, "%*[/]%*[^/]%*[/]%*[^/]%n", &share_end);
+	if (r == EOF || share_end == 0)
+		return NULL;
+	return unc + share_end;
+}
+
 /*
  * tb: /proc/self/mountinfo
  * fs: filesystem
@@ -1563,9 +1577,16 @@ int mnt_table_is_fs_mounted(struct libmnt_table *tb, struct libmnt_fs *fstab_fs)
 		}
 
 		if (root) {
-			const char *r = mnt_fs_get_root(fs);
-			if (!r || strcmp(r, root) != 0)
-				continue;
+			if (strcmp(mnt_fs_get_fstype(fs), "cifs") == 0) {
+				const char *unc_subdir = get_cifs_unc_subdir_path(src);
+				const char *path_on_fs = mnt_fs_get_root(fs);
+				if (!unc_subdir || !path_on_fs || !streq_paths(unc_subdir, path_on_fs))
+					continue;
+			} else {
+				const char *r = mnt_fs_get_root(fs);
+				if (!r || strcmp(r, root) != 0)
+					continue;
+			}
 		}
 
 		/*
-- 
2.1.4


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

  reply	other threads:[~2016-09-02 19:19 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-01 16:25 [PATCH] libmount: Skip root comparison for cifs in mnt_table_is_fs_mounted() Stanislav Brabec
2016-08-03 17:23 ` nfs/cifs mountinfo differences (was: Re: [PATCH] libmount: Skip root comparison for cifs in mnt_table_is_fs_mounted()) Stanislav Brabec
2016-08-03 17:23   ` Stanislav Brabec
     [not found]   ` <5eb96b3e-d463-bf11-49de-f614247d6b29-AlSwsSmVLrQ@public.gmane.org>
2016-08-03 20:32     ` Steve French
2016-08-05 17:22   ` Aurélien Aptel
2016-08-05 17:22     ` Aurélien Aptel
2016-08-16 17:44     ` nfs/cifs mountinfo differences Stanislav Brabec
2016-08-16 17:44       ` Stanislav Brabec
2016-08-23 17:23       ` Stanislav Brabec
2016-08-23 17:23         ` Stanislav Brabec
2016-08-23 17:25         ` [PATCH] libmount: Special handling of root comparison for cifs in mnt_table_is_fs_mounted() Stanislav Brabec
2016-08-23 17:25           ` Stanislav Brabec
2016-09-02 18:58           ` Aurélien Aptel [this message]
2016-09-02 18:58             ` Aurélien Aptel
2016-09-29  9:55             ` Karel Zak
2016-09-29  9:55               ` Karel Zak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=mpsbn06ruwv.fsf@suse.com \
    --to=aaptel@suse.com \
    --cc=linux-cifs@vger.kernel.org \
    --cc=sbrabec@suse.cz \
    --cc=smfrench@gmail.com \
    --cc=util-linux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.