* [pbs-devel] [PATCH] fix #2882: correctly parse optional fields from mountinfo
@ 2020-07-27 15:28 Stefan Reiter
2020-07-27 15:30 ` Stefan Reiter
2020-07-28 5:27 ` [pbs-devel] applied: " Dietmar Maurer
0 siblings, 2 replies; 3+ messages in thread
From: Stefan Reiter @ 2020-07-27 15:28 UTC (permalink / raw)
To: pbs-devel
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [pbs-devel] [PATCH] fix #2882: correctly parse optional fields from mountinfo
2020-07-27 15:28 [pbs-devel] [PATCH] fix #2882: correctly parse optional fields from mountinfo Stefan Reiter
@ 2020-07-27 15:30 ` Stefan Reiter
2020-07-28 5:27 ` [pbs-devel] applied: " Dietmar Maurer
1 sibling, 0 replies; 3+ messages in thread
From: Stefan Reiter @ 2020-07-27 15:30 UTC (permalink / raw)
To: pbs-devel
Sorry, this is for the "proxmox" crate obviously, my clever patch script
stripped the entire name as the prefix -.-
On 7/27/20 5:28 PM, Stefan Reiter wrote:
> 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");
> }
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* [pbs-devel] applied: [PATCH] fix #2882: correctly parse optional fields from mountinfo
2020-07-27 15:28 [pbs-devel] [PATCH] fix #2882: correctly parse optional fields from mountinfo Stefan Reiter
2020-07-27 15:30 ` Stefan Reiter
@ 2020-07-28 5:27 ` Dietmar Maurer
1 sibling, 0 replies; 3+ messages in thread
From: Dietmar Maurer @ 2020-07-28 5:27 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Stefan Reiter
applied
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-07-28 5:28 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-27 15:28 [pbs-devel] [PATCH] fix #2882: correctly parse optional fields from mountinfo Stefan Reiter
2020-07-27 15:30 ` Stefan Reiter
2020-07-28 5:27 ` [pbs-devel] applied: " Dietmar Maurer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox