Re: [BUG] libmount misparses mountinfo on Linux v2.6.39-rc1

Re: [BUG] libmount misparses mountinfo on Linux v2.6.39-rc1

[Aneesh Kumar K. V]

On Thu, 31 Mar 2011 22:24:01 +0200, victor.vde@gmail.com wrote:
> /proc/self/mountinfo on Linux v2.6.39-rc1 includes the UUID of ext4
> and ext3 mounts, for example:
> 
> 15 1 8:3 / / rw,noatime uuid:c645234d-9756-4d84-825e-6fe999252a34 - ext4 /dev/sda3 rw,user_xattr,acl,barrier=1,data=ordered
> 
> It seems the hyphens in the uuid confuse the parser
> mnt_parse_mountinfo_line in tab_parse.c of libmount in util-linux.

shouldn't the parser look for " " followed by "-" followed by " " as the
field seperator ? 

I am adding Miklos and Ram Pai to CC to check whether the kernel or the libmount 
should be fixed ?

> 
> My util-linux version is 2.19-4 Arch Linux.
> 
> ~ $ findmnt -l /
> TARGET SOURCE FSTYPE                      OPTIONS
> /      -      9756-4d84-825e-6fe999252a34 rw,noatime,ext4
> 
> For reference, the commits in Linus' tree that introduced these uuids
> are:
> 
> 93f1c20bc8cdb757be50566eff88d65c3b26881f vfs: Export file system uuid via /proc/<pid>/mountinfo
> 03cb5f03dcb26846fcad345d8c15aae91579a53d ext3: Copy fs UUID to superblock.
> f2fa2ffc2046fdc35f96366d1ec8675f4d578522 ext4: Copy fs UUID to superblock

-aneesh

Re: [BUG] libmount misparses mountinfo on Linux v2.6.39-rc1

[Miklos Szeredi]

On Fri, 01 Apr 2011, Aneesh Kumar K. V wrote:
> On Thu, 31 Mar 2011 22:24:01 +0200, victor.vde@gmail.com wrote:
> > /proc/self/mountinfo on Linux v2.6.39-rc1 includes the UUID of ext4
> > and ext3 mounts, for example:
> > 
> > 15 1 8:3 / / rw,noatime uuid:c645234d-9756-4d84-825e-6fe999252a34 - ext4 /dev/sda3 rw,user_xattr,acl,barrier=1,data=ordered
> > 
> > It seems the hyphens in the uuid confuse the parser
> > mnt_parse_mountinfo_line in tab_parse.c of libmount in util-linux.
> 
> shouldn't the parser look for " " followed by "-" followed by " " as the
> field seperator ? 
> 
> I am adding Miklos and Ram Pai to CC to check whether the kernel or the libmount 
> should be fixed ?

I think libmount should be fixed, though there is always the question
of backward compatibility.

However I don't see how the kernel could be fixed, given that libmount
doesn't seem to parse escape sequences (e.g. "\040" for space), which
it also should.

Karel?

Thanks,
Miklos

Re: [BUG] libmount misparses mountinfo on Linux v2.6.39-rc1

[Karel Zak]

On Fri, Apr 01, 2011 at 10:45:48AM +0200, Miklos Szeredi wrote:
> On Fri, 01 Apr 2011, Aneesh Kumar K. V wrote:
> > On Thu, 31 Mar 2011 22:24:01 +0200, victor.vde@gmail.com wrote:
> > > /proc/self/mountinfo on Linux v2.6.39-rc1 includes the UUID of ext4
> > > and ext3 mounts, for example:
> > > 
> > > 15 1 8:3 / / rw,noatime uuid:c645234d-9756-4d84-825e-6fe999252a34 - ext4 /dev/sda3 rw,user_xattr,acl,barrier=1,data=ordered
> > > 
> > > It seems the hyphens in the uuid confuse the parser
> > > mnt_parse_mountinfo_line in tab_parse.c of libmount in util-linux.
> > 
> > shouldn't the parser look for " " followed by "-" followed by " " as the
> > field seperator ? 
> > 
> > I am adding Miklos and Ram Pai to CC to check whether the kernel or the libmount 
> > should be fixed ?
> 
> I think libmount should be fixed, though there is always the question
> of backward compatibility.

 The parser uses

	rc = sscanf(s,	"%u "		/* (1) id */
			"%u "		/* (2) parent */
			"%u:%u "	/* (3) maj:min */
			"%ms "		/* (4) mountroot */
			"%ms "		/* (5) target */
			"%ms"		/* (6) vfs options (fs-independent) */
			"%*[^-]"	/* (7) optional fields */
			"- "		/* (8) separator */
			"%ms "		/* (9) FS type */
			"%ms "		/* (10) source */
			"%ms",		/* (11) fs options (fs specific) */

 note that almost the same code is in systemd.

> However I don't see how the kernel could be fixed, given that libmount
> doesn't seem to parse escape sequences (e.g. "\040" for space), which
> it also should.

 It calls unmangle_string() for all fields, so all sequences should be
 decoded.

    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [BUG] libmount misparses mountinfo on Linux v2.6.39-rc1

[Miklos Szeredi]

On Fri, 1 Apr 2011, Karel Zak wrote:
> On Fri, Apr 01, 2011 at 10:45:48AM +0200, Miklos Szeredi wrote:
> > On Fri, 01 Apr 2011, Aneesh Kumar K. V wrote:
> > > On Thu, 31 Mar 2011 22:24:01 +0200, victor.vde@gmail.com wrote:
> > > > /proc/self/mountinfo on Linux v2.6.39-rc1 includes the UUID of ext4
> > > > and ext3 mounts, for example:
> > > > 
> > > > 15 1 8:3 / / rw,noatime uuid:c645234d-9756-4d84-825e-6fe999252a34 - ext4 /dev/sda3 rw,user_xattr,acl,barrier=1,data=ordered
> > > > 
> > > > It seems the hyphens in the uuid confuse the parser
> > > > mnt_parse_mountinfo_line in tab_parse.c of libmount in util-linux.
> > > 
> > > shouldn't the parser look for " " followed by "-" followed by " " as the
> > > field seperator ? 
> > > 
> > > I am adding Miklos and Ram Pai to CC to check whether the kernel or the libmount 
> > > should be fixed ?
> > 
> > I think libmount should be fixed, though there is always the question
> > of backward compatibility.
> 
>  The parser uses
> 
> 	rc = sscanf(s,	"%u "		/* (1) id */
> 			"%u "		/* (2) parent */
> 			"%u:%u "	/* (3) maj:min */
> 			"%ms "		/* (4) mountroot */
> 			"%ms "		/* (5) target */
> 			"%ms"		/* (6) vfs options (fs-independent) */
> 			"%*[^-]"	/* (7) optional fields */

So this says, "skip everything up to the first hyphen", even though
the rule might be better expressed as "skip all space delimited words
up to the first standalone hyphen".  I guess that's not possible to
express with a scanf expression, though.

> 			"- "		/* (8) separator */
> 			"%ms "		/* (9) FS type */
> 			"%ms "		/* (10) source */
> 			"%ms",		/* (11) fs options (fs specific) */
> 
>  note that almost the same code is in systemd.
> 
> > However I don't see how the kernel could be fixed, given that libmount
> > doesn't seem to parse escape sequences (e.g. "\040" for space), which
> > it also should.
> 
>  It calls unmangle_string() for all fields, so all sequences should be
>  decoded.

Okay, so the kernel can simply escape the troublesome hyphens, which
would make the human parsing of /proc/pid/mountinfo more difficult but
at least would fix compatibility with util-linux's parser.

Thanks,
Miklos

Re: [BUG] libmount misparses mountinfo on Linux v2.6.39-rc1

[Miklos Szeredi]

On Fri, 1 Apr 2011, Karel Zak wrote:
> On Fri, Apr 01, 2011 at 10:45:48AM +0200, Miklos Szeredi wrote:
> > On Fri, 01 Apr 2011, Aneesh Kumar K. V wrote:
> > > On Thu, 31 Mar 2011 22:24:01 +0200, victor.vde-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> > > > /proc/self/mountinfo on Linux v2.6.39-rc1 includes the UUID of ext4
> > > > and ext3 mounts, for example:
> > > > 
> > > > 15 1 8:3 / / rw,noatime uuid:c645234d-9756-4d84-825e-6fe999252a34 - ext4 /dev/sda3 rw,user_xattr,acl,barrier=1,data=ordered
> > > > 
> > > > It seems the hyphens in the uuid confuse the parser
> > > > mnt_parse_mountinfo_line in tab_parse.c of libmount in util-linux.
> > > 
> > > shouldn't the parser look for " " followed by "-" followed by " " as the
> > > field seperator ? 
> > > 
> > > I am adding Miklos and Ram Pai to CC to check whether the kernel or the libmount 
> > > should be fixed ?
> > 
> > I think libmount should be fixed, though there is always the question
> > of backward compatibility.
> 
>  The parser uses
> 
> 	rc = sscanf(s,	"%u "		/* (1) id */
> 			"%u "		/* (2) parent */
> 			"%u:%u "	/* (3) maj:min */
> 			"%ms "		/* (4) mountroot */
> 			"%ms "		/* (5) target */
> 			"%ms"		/* (6) vfs options (fs-independent) */
> 			"%*[^-]"	/* (7) optional fields */

So this says, "skip everything up to the first hyphen", even though
the rule might be better expressed as "skip all space delimited words
up to the first standalone hyphen".  I guess that's not possible to
express with a scanf expression, though.

> 			"- "		/* (8) separator */
> 			"%ms "		/* (9) FS type */
> 			"%ms "		/* (10) source */
> 			"%ms",		/* (11) fs options (fs specific) */
> 
>  note that almost the same code is in systemd.
> 
> > However I don't see how the kernel could be fixed, given that libmount
> > doesn't seem to parse escape sequences (e.g. "\040" for space), which
> > it also should.
> 
>  It calls unmangle_string() for all fields, so all sequences should be
>  decoded.

Okay, so the kernel can simply escape the troublesome hyphens, which
would make the human parsing of /proc/pid/mountinfo more difficult but
at least would fix compatibility with util-linux's parser.

Thanks,
Miklos
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [BUG] libmount misparses mountinfo on Linux v2.6.39-rc1

[Karel Zak]

On Fri, Apr 01, 2011 at 12:12:34PM +0200, Miklos Szeredi wrote:
> On Fri, 1 Apr 2011, Karel Zak wrote:
> > On Fri, Apr 01, 2011 at 10:45:48AM +0200, Miklos Szeredi wrote:
> > > On Fri, 01 Apr 2011, Aneesh Kumar K. V wrote:
> > > > On Thu, 31 Mar 2011 22:24:01 +0200, victor.vde@gmail.com wrote:
> > > > > /proc/self/mountinfo on Linux v2.6.39-rc1 includes the UUID of ext4
> > > > > and ext3 mounts, for example:
> > > > > 
> > > > > 15 1 8:3 / / rw,noatime uuid:c645234d-9756-4d84-825e-6fe999252a34 - ext4 /dev/sda3 rw,user_xattr,acl,barrier=1,data=ordered
> > > > > 
> > > > > It seems the hyphens in the uuid confuse the parser
> > > > > mnt_parse_mountinfo_line in tab_parse.c of libmount in util-linux.
> > > > 
> > > > shouldn't the parser look for " " followed by "-" followed by " " as the
> > > > field seperator ? 
> > > > 
> > > > I am adding Miklos and Ram Pai to CC to check whether the kernel or the libmount 
> > > > should be fixed ?
> > > 
> > > I think libmount should be fixed, though there is always the question
> > > of backward compatibility.
> > 
> >  The parser uses
> > 
> > 	rc = sscanf(s,	"%u "		/* (1) id */
> > 			"%u "		/* (2) parent */
> > 			"%u:%u "	/* (3) maj:min */
> > 			"%ms "		/* (4) mountroot */
> > 			"%ms "		/* (5) target */
> > 			"%ms"		/* (6) vfs options (fs-independent) */
> > 			"%*[^-]"	/* (7) optional fields */
> 
> So this says, "skip everything up to the first hyphen", even though
> the rule might be better expressed as "skip all space delimited words
> up to the first standalone hyphen".  I guess that's not possible to
> express with a scanf expression, though.

 Yes, I'll probably use two sscanf(), %n and strstr(), something like:

	rc = sscanf(s,	"%u "		/* (1) id */
			"%u "		/* (2) parent */
			"%u:%u "	/* (3) maj:min */
			"%ms "		/* (4) mountroot */
			"%ms "		/* (5) target */
			"%ms"		/* (6) vfs options (fs-independent) */
			"%n",

			&id,
			&parent,
			&maj, &min,
			&root,
			&target,
			&vfs_optstr,
			&end);

	s += end;

	/* (7) optional fields, terminated by " - " */
	p = strstr(s, " - ");
	if (p)
		s = p + 3;

	rc += sscanf(s,	"%ms "		/* (8) FS type */
			"%ms "		/* (9) source */
			"%ms",		/* (10) fs options (fs specific) */

			&fstype,
			&src,
			&fs_optstr);


...or anyone has a better idea?

> >  It calls unmangle_string() for all fields, so all sequences should be
> >  decoded.
> 
> Okay, so the kernel can simply escape the troublesome hyphens, which
> would make the human parsing of /proc/pid/mountinfo more difficult but
> at least would fix compatibility with util-linux's parser.

Hmm.. IMHO it will be better to fix the parsers in libmount and systemd. The 
both projects are young, nobody will be surprised with bugs :-)

I think the example in Documentation/filesystems/proc.txt is pretty
obvious, the separator is an extra field with " - ".

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [BUG] libmount misparses mountinfo on Linux v2.6.39-rc1

[Karel Zak]

On Fri, Apr 01, 2011 at 01:16:23PM +0530, Aneesh Kumar K. V wrote:
> On Thu, 31 Mar 2011 22:24:01 +0200, victor.vde@gmail.com wrote:
> > /proc/self/mountinfo on Linux v2.6.39-rc1 includes the UUID of ext4
> > and ext3 mounts, for example:
> > 
> > 15 1 8:3 / / rw,noatime uuid:c645234d-9756-4d84-825e-6fe999252a34 - ext4 /dev/sda3 rw,user_xattr,acl,barrier=1,data=ordered
> > 
> > It seems the hyphens in the uuid confuse the parser
> > mnt_parse_mountinfo_line in tab_parse.c of libmount in util-linux.
> 
> shouldn't the parser look for " " followed by "-" followed by " " as the
> field seperator ? 
> 
> I am adding Miklos and Ram Pai to CC to check whether the kernel or the libmount 
> should be fixed ?

 The libmount (in util-linux git tree) should be fixed now. Thanks for
 the report! 

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com