Re: [pbs-devel] [PATH proxmox-backup] fix #4380: stat() is run when file is executed

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On August 4, 2023 9:42 am, Fiona Ebner wrote:
> Am 03.08.23 um 17:22 schrieb Gabriel Goller:
>> diff --git a/pbs-client/src/pxar/create.rs b/pbs-client/src/pxar/create.rs
>> index 2577cf98..c573c2a3 100644
>> --- a/pbs-client/src/pxar/create.rs
>> +++ b/pbs-client/src/pxar/create.rs
>> @@ -434,6 +434,15 @@ impl Archiver {
>>              assert_single_path_component(os_file_name)?;
>>              let full_path = self.path.join(os_file_name);
>>  
>> +            let match_path = PathBuf::from("/").join(full_path.clone());
>> +            if self
>> +                .patterns
>> +                .matches(match_path.as_os_str().as_bytes(), None)
> 
> Is it fine to call matches() without the file mode in all cases? Can't
> it make a difference for directory matching? If it's okay, please
> explain why in the commit message.

good catch, thanks.

I guess we need something like this if we want to support it - the
second hunk is only needed in case we ever differentiate between the
different types other than directories ('/' at the end of the pattern)
and regular files.

in the end, it might make more sense to try the other approach I
indicated as follow-up in my first reply? we already have the stat info
of each dir we encounter, so we can decide if a dir is a "weird
unreadable one" and treat that specially, moving the pattern match here
back below the stat, and just never go down that code path for affected
dirs?

diff --git a/pbs-client/src/pxar/create.rs b/pbs-client/src/pxar/create.rs
index c573c2a3..eaa84c76 100644
--- a/pbs-client/src/pxar/create.rs
+++ b/pbs-client/src/pxar/create.rs
@@ -435,9 +435,15 @@ impl Archiver {
             let full_path = self.path.join(os_file_name);
 
             let match_path = PathBuf::from("/").join(full_path.clone());
+            let entry_type = if file.file_type() == Some(nix::dir::Type::Directory) {
+                Some(libc::S_IFDIR)
+            } else {
+                Some(libc::S_IFREG)
+            };
+
             if self
                 .patterns
-                .matches(match_path.as_os_str().as_bytes(), None)
+                .matches(match_path.as_os_str().as_bytes(), entry_type)
                 == Some(MatchType::Exclude)
             {
                 continue;
@@ -453,6 +459,14 @@ impl Archiver {
                 Err(err) => return Err(err).context(format!("stat failed on {:?}", full_path)),
             };
 
+            if self
+                .patterns
+                .matches(match_path.as_os_str().as_bytes(), Some(stat.st_mode))
+                == Some(MatchType::Exclude)
+            {
+                continue;
+            }
+
             self.entry_counter += 1;
             if self.entry_counter > self.entry_limit {
                 bail!(