From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <s.reiter@proxmox.com>
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 <pbs-devel@lists.proxmox.com>; 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 <pbs-devel@lists.proxmox.com>; 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 <pbs-devel@lists.proxmox.com>; 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 <pbs-devel@lists.proxmox.com>; 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 <s.reiter@proxmox.com>
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
 <pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=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/<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");
>   }
>