From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 19F6266850 for ; Mon, 27 Jul 2020 17:30:47 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 10EFE1DFC9 for ; Mon, 27 Jul 2020 17:30:47 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 4FB951DFBA for ; Mon, 27 Jul 2020 17:30:45 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 18E8841B1F for ; Mon, 27 Jul 2020 17:30:45 +0200 (CEST) To: pbs-devel@lists.proxmox.com References: <20200727152840.2731-1-s.reiter@proxmox.com> From: Stefan Reiter Message-ID: <22cbc6d8-ec4a-c367-fd05-de36d71430e7@proxmox.com> Date: Mon, 27 Jul 2020 17:30:43 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <20200727152840.2731-1-s.reiter@proxmox.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.433 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.951 Looks like a legit reply (A) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pbs-devel] [PATCH] fix #2882: correctly parse optional fields from mountinfo X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 27 Jul 2020 15:30:47 -0000 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//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 > --- > > 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"); > } >