mountinfo: parse empty strings in source

[alban]

The source of a mount in /proc/self/mountinfo can unfortunately be an
empty string. Before this patch, 'mount' and 'mountpoint' fail as
following:

$ sudo mount -t tmpfs "" /tmp/bb
$ mount
mount: /proc/self/mountinfo: parse error at line 64 -- ignored
$ mountpoint /tmp/bb
/tmp/bb is not a mountpoint

And docker fails with the following:

$ sudo docker run -ti --rm busybox
/usr/bin/docker-current: Error response from daemon: devmapper: Error mounting
'/dev/mapper/docker-253:1-5778711-4a86058b044487d3ee73dbc1349f1f9a4b4f78c9b026494f8d3e75a37ee6fcb6-init'
on '/var/lib/docker/devicemapper/mnt/4a86058b044487d3ee73dbc1349f1f9a4b4f78c9b026494f8d3e75a37ee6fcb6-init'.
fstype=xfs options=nouuid: Error found less than 3 fields post '-' in
"242 45 0:66 / /tmp/bb rw,relatime shared:212 - tmpfs  rw,seclabel"

This patch fixes the parsing.

Signed-off-by: Alban Crequy alban@kinvolk.io

---

Similar patch in util-linux: https://www.spinics.net/lists/linux-fsdevel/msg128896.html
Similar PR in runtime-tools: opencontainers/runtime-tools#652
Related patch that would fix this kernel-side: https://patchwork.kernel.org/patch/10349095/

[dqminh]

maybe we should extract this out into an internal helper and add a test for this.

[vadorovsky]

First of all, I see that the same code reading mountinfo is in three places. Can we maybe move it to some util function?

The other thing (which is really minor and I don't mind if you don't agree :) ) - when I dealt with reading mountinfo in Cilium, I decided to just do two splits. First split on - separator, second split (on both substrings) just on space " ". I think that would be a bit more readable approach than operating on index. You can see that implementation here:

https://github.com/cilium/cilium/blob/master/pkg/mountinfo/mountinfo.go

Of course I will need to fix it, because of possibility of empty source. So I will need to rely less on indexes of splitted slices. But I just mean doing splits, without pointing to index. :)

Any thoughts?

[vadorovsky]

@alban I just double checked whether that issue exists in Cilium. It doesn't! I forgot that doing strings.Split(s, " ") in Golang doesn't trim empty strings. So my implementation of mountinfo parser, which is currently on Cilium's master, works perfectly with empty mount sources.

I mean, after trying to reproduce the issue with your commands, value of that attribute was "" https://github.com/cilium/cilium/blob/master/pkg/mountinfo/mountinfo.go#L113

[vadorovsky]

I'm also thinking whether it would make sense to create some Go library (usable for everyone, to be vendored) for mountinfo parsing - there are probably many Go projects which are doing that and many of them might be affected by this bug. From what I see, Kubernetes is affected too.

[AkihiroSuda]

It is likely that we will switch to github.com/moby/sys mountinfo parser #2256