Some of our code iterates over a directory and does some
decisions based on entry type (e.g. if it's not a regular file
continue with next iteration). This is done by looking at
ent->d_type struct member. However, in some cases (e.g. for some
filesystems) the d_type can be set to DT_UNKNOWN which
effectively defeats the aforementioned checks. For instance, some
XFS' (depending on how they were created) might not bother
filling the member.

Fix this by running stat() and filling in the member ourselves.
If needed.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---

With Dan's patch that fixes stat() mocking, I'm passing tests
successfully everywhere I tried (32bit fedora and gentoo on RPi, RHEL-7,
amd64 gentoo, fedora rawhide on x86_64)

 src/util/virfile.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index ec8d85929c..7d7c27a7d2 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2893,6 +2893,39 @@ virDirOpenQuiet(DIR **dirp, const char *name)
     return virDirOpenInternal(dirp, name, false, true);
 }
 
+static int
+virDirReadFixDType(DIR *dirp, struct dirent *ent)
+{
+    /* DO NOT CLOSE THIS FD */
+    const int fd = dirfd(dirp);
+    struct stat sb;
+
+    if (fd < 0)
+        return -1;
+
+    if (fstatat(fd, ent->d_name, &sb, AT_NO_AUTOMOUNT | AT_SYMLINK_NOFOLLOW) < 0)
+        return -1;
+
+    if (S_ISDIR(sb.st_mode))
+        ent->d_type = DT_DIR;
+    else if (S_ISCHR(sb.st_mode))
+        ent->d_type = DT_CHR;
+    else if (S_ISBLK(sb.st_mode))
+        ent->d_type = DT_BLK;
+    else if (S_ISREG(sb.st_mode))
+        ent->d_type = DT_REG;
+    else if (S_ISFIFO(sb.st_mode))
+        ent->d_type = DT_FIFO;
+    else if (S_ISLNK(sb.st_mode))
+        ent->d_type = DT_LNK;
+    else if (S_ISSOCK(sb.st_mode))
+        ent->d_type = DT_SOCK;
+    else
+        return -1;
+
+    return 0;
+}
+
 /**
  * virDirRead:
  * @dirp: directory to read
@@ -2926,6 +2959,17 @@ int virDirRead(DIR *dirp, struct dirent **ent, const char *name)
         }
     } while (*ent && (STREQ((*ent)->d_name, ".") ||
                       STREQ((*ent)->d_name, "..")));
+
+    if (*ent &&
+        (*ent)->d_type == DT_UNKNOWN &&
+        virDirReadFixDType(dirp, *ent) < 0) {
+        if (name)
+            virReportSystemError(errno,
+                                 _("Unable to fix d_type of '%s/%s'"),
+                                 name, (*ent)->d_name);
+        return -1;
+    }
+
     return !!*ent;
 }
 
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Apr 02, 2019 at 02:17:35PM +0200, Michal Privoznik wrote:
> Some of our code iterates over a directory and does some
> decisions based on entry type (e.g. if it's not a regular file
> continue with next iteration). This is done by looking at
> ent->d_type struct member. However, in some cases (e.g. for some
> filesystems) the d_type can be set to DT_UNKNOWN which
> effectively defeats the aforementioned checks. For instance, some
> XFS' (depending on how they were created) might not bother
> filling the member.
> 
> Fix this by running stat() and filling in the member ourselves.
> If needed.

We really don't want to do this. It adds an extra syscall for every
single file in the directory. I've got an alternative patch that
simply puts a stat call into the firmware code & also adds a syntax
check rule to blacklist use of d_type.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list