mountinfo: parse empty strings in source #1829
Conversation
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 <[email protected]>
| // Source can be an empty string ("") so strings.Fields() won't work here. | ||
| // Test with: | ||
| // $ sudo mount -t tmpfs "" /tmp/bb | ||
| postSeparatorFields := strings.Split(text[index+3:], " ") | ||
| if len(postSeparatorFields) < 3 { | ||
| return nil, fmt.Errorf("Error found less than 3 fields post '-' in %q", text) | ||
| } |
There was a problem hiding this comment.
maybe we should extract this out into an internal helper and add a test for this.
There was a problem hiding this comment.
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?
|
@alban I just double checked whether that issue exists in Cilium. It doesn't! I forgot that doing I mean, after trying to reproduce the issue with your commands, value of that attribute was |
|
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. |
|
It is likely that we will switch to github.com/moby/sys mountinfo parser #2256 |
The source of a mount in /proc/self/mountinfo can unfortunately be an
empty string. Before this patch, 'mount' and 'mountpoint' fail as
following:
And docker fails with the following:
This patch fixes the parsing.
Signed-off-by: Alban Crequy [email protected]
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/