[pbs-devel] [PATCH] fix #2882: correctly parse optional fields from mountinfo

[Stefan Reiter <s.reiter@proxmox.com>]

As per the linux kernel documentation[0], the "optional fields" (called
"tags" in our parser) are not comma-separated, but are a variable number
(0 or more) of space-separated fields, always followed by a dash to mark
the end.

Fix the /proc/<pid>/mountinfo parser to correctly parse these and add
test cases (l3 is the line that produced the original warning from the
bug report).

[0] https://www.kernel.org/doc/html/latest/filesystems/proc.html#proc-pid-mountinfo-information-about-mounts

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---

I still think the way we parse here (with the fixed next()'s) is kind of weird,
but I don't have a better idea and the rest of the "mountinfo" format looks
pretty set in stone, so I suppose it's fine.

 proxmox/src/sys/linux/procfs/mountinfo.rs | 53 ++++++++++++++++++-----
 1 file changed, 42 insertions(+), 11 deletions(-)

diff --git a/proxmox/src/sys/linux/procfs/mountinfo.rs b/proxmox/src/sys/linux/procfs/mountinfo.rs
index 55f74c2..b57f132 100644
--- a/proxmox/src/sys/linux/procfs/mountinfo.rs
+++ b/proxmox/src/sys/linux/procfs/mountinfo.rs
@@ -135,17 +135,18 @@ impl Entry {
             root: OsStr::from_bytes(next()?).to_owned().into(),
             mount_point: OsStr::from_bytes(next()?).to_owned().into(),
             mount_options: OsStr::from_bytes(next()?).to_owned(),
-            tags: next()?.split(|b| *b == b',').try_fold(
-                Vec::new(),
-                |mut acc, tag| -> Result<_, Error> {
-                    acc.push(Tag::parse(tag)?);
-                    Ok(acc)
-                },
-            )?,
-            fs_type: std::str::from_utf8({
-                next()?;
-                next()?
-            })?
+            tags: {
+                let mut tags = Vec::new();
+                loop {
+                    let tval = next()?;
+                    if tval == b"-" {
+                        break;
+                    }
+                    tags.push(Tag::parse(tval)?);
+                }
+                tags
+            },
+            fs_type: std::str::from_utf8(next()?)?
             .to_string(),
             mount_source: next().map(|src| match src {
                 b"none" => None,
@@ -350,6 +351,36 @@ fn test_entry() {
         "rw,fd=26,pgrp=1,timeout=0,minproto=5,maxproto=5,direct,pipe_ino=27726"
     );
 
+    // test different tag configurations
+    let l3: &[u8] =
+        b"225 224 0:46 / /proc rw,nosuid,nodev,noexec,relatime - proc proc rw";
+    let entry = Entry::parse(l3).expect("failed to parse third mountinfo test entry");
+    assert_eq!(entry.tags, &[]);
+
+    let l4: &[u8] =
+        b"48 32 0:43 / /sys/fs/cgroup/blkio rw,nosuid,nodev,noexec,relatime \
+          shared:5 master:7 propagate_from:2 unbindable \
+          - cgroup cgroup rw,blkio";
+    let entry = Entry::parse(l4).expect("failed to parse fourth mountinfo test entry");
+    assert_eq!(entry.tags, &[
+        Tag {
+            tag: OsString::from("shared"),
+            value: Some(OsString::from("5")),
+        },
+        Tag {
+            tag: OsString::from("master"),
+            value: Some(OsString::from("7")),
+        },
+        Tag {
+            tag: OsString::from("propagate_from"),
+            value: Some(OsString::from("2")),
+        },
+        Tag {
+            tag: OsString::from("unbindable"),
+            value: None,
+        },
+    ]);
+
     let mount_info = [l1, l2].join(&b"\n"[..]);
     MountInfo::parse(&mount_info).expect("failed to parse mount info file");
 }
-- 
2.20.1