[prev in list] [next in list] [prev in thread] [next in thread]
List: busybox
Subject: Re: [PATCH] mount, cifs bug in 1.20.2 Kernel > 3.3.7
From: Denys Vlasenko <vda.linux () googlemail ! com>
Date: 2013-01-17 1:21:00
Message-ID: 201301170221.00883.vda.linux () googlemail ! com
[Download RAW message or body]
On Wednesday 16 January 2013 18:10, Bernhard Reutner-Fischer wrote:
> On 16 January 2013 00:48, Denys Vlasenko <vda.linux@googlemail.com> wrote:
> > On Monday 14 January 2013 14:02, Cristian Ionescu-Idbohrn wrote:
> > > Let me bump this patch one more time Denys, now that you seem being
> > > back into busybox maintainership ;)
> >
> > Applied just now. Thanks!
>
> This does not work for me. Apparently we have to also handle
> prefixpath, Denys, can you do the attached properly, if you have a
> minute?
> With current master i got:
> $ busybox_unstripped mount -tcifs -oro,user=me,password=ppp
> //hostname/share/path1/path2 /media/cdrom
> mount("//hostname/share/path1/path2", "/media/cdrom", "cifs",
> MS_RDONLY|MS_MANDLOCK|MS_SILENT|0x10000000,
> "password=ppp,unc=\\\\hostname\\share/path1/path2,ip=1.1.1.1") = -1
> EINVAL (Invalid argument)
>
> 1) it misinterpretes "user=me" to set vfsflags 1<<28
Fixing...
> 2) it sets vfsflags MANDLOCK although i do not want nor need it
I wondered about that too (as you see from the comment)
> 3) it passes an invalid unc (containing a path) to the kernel
> 4) it could use "pass" as shorthand for "password"
Why would we bother to even parse that. We pass thru as much as we can.
> This command yields with util-linux mount:
> mount("//hostname/share/path1/path2", ".", "cifs", MS_RDONLY,
> "ip=1.1.1.1,unc=\\\\hostname\\share,user=me,prefixpath=path1/path2,pass=ppp)
> = 0
>
> ok, so s/user=/username=/ to bypass the broken vfsflags handling, master yields:
> mount("//hostname/share/path1/path2", "/media/cdrom", "cifs",
> MS_RDONLY|MS_MANDLOCK|MS_SILENT|0x10000000,
> "username=me,password=ppp,unc=\\\\hostname\\share/path1/path2,ip=1.1.1.1")
> = -1 ENXIO (No such device or address)
>
> With attached proposed patchlet, we are able to mount the thing:
> mount("//hostname/share/path1/path2", "/media/cdrom", "cifs",
> MS_RDONLY|MS_SILENT,
> "username=me,password=ppp,unc=\\\\hostname\\share,prefixpath=path1/path2,ip=1.1.1.1")
> = 0
Thanks! And btw, we have nice -fvvv options for debugging:
I reworked it a bit, result:
# ./busybox mount -fvvv -tcifs -oro,user=me,password=ppp //hostname/share/path1/path2 \
/qwe/rty
mount: would do mount('//hostname/share/path1/path2','/qwe/rty','cifs',0x00008001,'use \
r=me,password=ppp,unc=\\hostname\share,prefixpath=path1/path2,ip=176.74.176.167')
(he he, "hostname.com" exists out there :)
diff -ad -urpN busybox.4/util-linux/mount.c busybox.5/util-linux/mount.c
--- busybox.4/util-linux/mount.c 2013-01-16 00:44:43.000000000 +0100
+++ busybox.5/util-linux/mount.c 2013-01-17 02:08:27.000000000 +0100
@@ -224,7 +224,7 @@ static const int32_t mount_options[] = {
IF_DESKTOP(/* "user" */ MOUNT_USERS,)
IF_DESKTOP(/* "users" */ MOUNT_USERS,)
/* "_netdev" */ 0,
- IF_DESKTOP(/* "comment" */ 0,) /* systemd uses this in fstab */
+ IF_DESKTOP(/* "comment=" */ 0,) /* systemd uses this in fstab */
)
IF_FEATURE_MOUNT_FLAGS(
@@ -283,7 +283,7 @@ static const char mount_option_str[] =
IF_DESKTOP("user\0")
IF_DESKTOP("users\0")
"_netdev\0"
- IF_DESKTOP("comment\0") /* systemd uses this in fstab */
+ IF_DESKTOP("comment=\0") /* systemd uses this in fstab */
)
IF_FEATURE_MOUNT_FLAGS(
// vfs flags
@@ -475,10 +475,13 @@ static unsigned long parse_mount_options
// FIXME: use hasmntopt()
// Find this option in mount_options
for (i = 0; i < ARRAY_SIZE(mount_options); i++) {
- /* We support "option=" match for "comment=" thingy */
unsigned opt_len = strlen(option_str);
+
if (strncasecmp(option_str, options, opt_len) == 0
- && (options[opt_len] == '\0' || options[opt_len] == '=')
+ && (options[opt_len] == '\0'
+ /* or is it "comment=" thingy in fstab? */
+ IF_FEATURE_MOUNT_FSTAB(IF_DESKTOP( || option_str[opt_len-1] == '=' ))
+ )
) {
unsigned long fl = mount_options[i];
if (fl & BB_MS_INVERTED_VALUE)
@@ -1817,31 +1820,44 @@ static int singlemount(struct mntent *mp
) {
int len;
char c;
+ char *hostname, *share;
+ char *dotted, *ip;
len_and_sockaddr *lsa;
- char *hostname, *dotted, *ip;
+
+ // Parse mp->mnt_fsname of the form "//hostname/share[/dir1/dir2]"
hostname = mp->mnt_fsname + 2;
len = strcspn(hostname, "/\\");
- if (len == 0 // 1st char is a [back]slash (IOW: empty hostname)
- || hostname[len] == '\0' // no [back]slash after hostname
- || hostname[len+1] == '\0' // empty share name
+ share = hostname + len + 1;
+ if (len == 0 // 3rd char is a [back]slash (IOW: empty hostname)
+ || share[-1] == '\0' // no [back]slash after hostname
+ || share[0] == '\0' // empty share name
) {
goto report_error;
}
- c = hostname[len];
- hostname[len] = '\0';
+ c = share[-1];
+ share[-1] = '\0';
+ len = strcspn(share, "/\\");
// "unc=\\hostname\share" option is mandatory
// after CIFS option parsing was rewritten in Linux 3.4.
- // Must pass it to the kernel. Must use backslashes.
+ // Must use backslashes.
+ // If /dir1/dir2 is present, also add "prefixpath=dir1/dir2"
{
- char *unc = xasprintf("unc=\\\\%s\\%s", hostname, hostname + len + 1);
- parse_mount_options(unc, &filteropts);
- if (ENABLE_FEATURE_CLEAN_UP) free(unc);
+ char *unc = xasprintf(
+ share[len] != '\0' /* "/dir1/dir2" exists? */
+ ? "unc=\\\\%s\\%.*s,prefixpath=%s"
+ : "unc=\\\\%s\\%.*s",
+ hostname,
+ len, share,
+ share + len + 1 /* "dir1/dir2" */
+ );
+ parse_mount_options(unc, &filteropts);
+ if (ENABLE_FEATURE_CLEAN_UP) free(unc);
}
lsa = host2sockaddr(hostname, 0);
- hostname[len] = c;
+ share[-1] = c;
if (!lsa)
goto report_error;
@@ -1853,8 +1869,6 @@ static int singlemount(struct mntent *mp
parse_mount_options(ip, &filteropts);
if (ENABLE_FEATURE_CLEAN_UP) free(ip);
- // "-o mand" is required [why?]
- vfsflags |= MS_MANDLOCK;
mp->mnt_type = (char*)"cifs";
rc = mount_it_now(mp, vfsflags, filteropts);
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic