Re: [PATCH] Ignore symlink traversal failures in grub-mount readdir

[Andrey Borzenkov]

В Fri, 12 Oct 2012 17:09:54 +0100
Colin Watson <address@hidden> пишет:

> This is very much a temporary hack, so I'm sending it here for
> discussion rather than just committing it even though it's quite simple.
> 
> r3036.1.15 introduced support for filling in the attributes of files in
> fuse_readdir.  However, symlinks to directories are passed to call_fill
> with info.dir unset; call_fill will then try to open them with
> grub_file_open, and get GRUB_ERR_BAD_FILE_TYPE because it's ultimately a
> directory not a regular file.  It then causes the whole readdir call to
> fail.  The net effect is that if you, for example, have a symlink
> anywhere in the top level of a filesystem, then the entire filesystem
> appears empty in grub-mount.  This is the root cause of
> https://bugs.launchpad.net/bugs/1051306.
> 
> I think that the proper solution is to pass the full
> grub_fshelp_filetype to dirhook functions, which would permit
> implementing true symlink support in grub-mount.  That would be a fairly
> large change that I don't have time for at the moment.  As a stopgap, I
> suggest that we ignore errors from individual grub_file_open calls
> during fuse_readdir.  How does this patch look?
> 

The only reason to call grub_file_open() is to fetch file size, and file
size is already available when hooks are called. So what about patch
below? It fixes problem for me, and it trivial enough. This allows
directory listing to work again. I can extend it with info.is_link to
return proper filetype to FUSE, but implementing full support needs
adding readlink that is a separate topic.

I tested it with ext4 and cpio and it works. Testing on more systems
(in particular, NTFS, which is the only one with non-trivial change) is
appreciated.

---

From: Andrey Borzenkov <address@hidden>
Subject: [PATCH] return file size in grub_dirhook_info for FUSE

The only reason for using grub_file_open() in fuse_getattr() and
fuse_readdir_call_fill() was to get file size. But file size is already
available at the place when these hooks are called in all filesystems
except NTFS. So the call is redundant; instead simply return file size
as part of struct grub_dirhook_info.

For NTFS this patch additionally adds reading "inode" information from
disk, as it was not available otherwise.

All additional code is under #ifdef GRUB_UTLI to not impact boot time
sizes.

This fixes problem with listing directories containing symlinks
(http://lists.gnu.org/archive/html/grub-devel/2012-10/msg00021.html).
Attempt to access these symlinks still fails of course, full support
for symlinks requires more efforts.

Signed-off-by: Andrey Borzenkov <address@hidden>

---
 grub-core/fs/affs.c     |    3 +++
 grub-core/fs/bfs.c      |    3 +++
 grub-core/fs/btrfs.c    |    3 +++
 grub-core/fs/cpio.c     |    3 +++
 grub-core/fs/ext2.c     |    4 ++++
 grub-core/fs/fat.c      |    3 +++
 grub-core/fs/hfs.c      |    3 +++
 grub-core/fs/hfsplus.c  |    3 +++
 grub-core/fs/iso9660.c  |    3 +++
 grub-core/fs/jfs.c      |    3 +++
 grub-core/fs/minix.c    |    3 +++
 grub-core/fs/nilfs2.c   |    3 +++
 grub-core/fs/ntfs.c     |    7 +++++++
 grub-core/fs/reiserfs.c |    3 +++
 grub-core/fs/romfs.c    |    3 +++
 grub-core/fs/sfs.c      |    3 +++
 grub-core/fs/squash4.c  |   11 +++++++++++
 grub-core/fs/udf.c      |    3 +++
 grub-core/fs/ufs.c      |    3 +++
 grub-core/fs/xfs.c      |    3 +++
 grub-core/fs/zfs/zfs.c  |    3 +++
 include/grub/fs.h       |    3 +++
 util/grub-mount.c       |   25 ++-----------------------
 23 files changed, 81 insertions(+), 23 deletions(-)

diff --git a/grub-core/fs/affs.c b/grub-core/fs/affs.c
index 726704e..fead360 100644
--- a/grub-core/fs/affs.c
+++ b/grub-core/fs/affs.c
@@ -565,6 +565,9 @@ grub_affs_dir_iter (const char *filename, enum 
grub_fshelp_filetype filetype,
   info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);
   info.mtimeset = 1;
   info.mtime = aftime2ctime (&node->di.mtime);
+#ifdef GRUB_UTIL
+  info.size = grub_be_to_cpu32 (node->di.size);
+#endif
   grub_free (node);
   return ctx->hook (filename, &info, ctx->hook_data);
 }
diff --git a/grub-core/fs/bfs.c b/grub-core/fs/bfs.c
index f2e39d3..3b8357b 100644
--- a/grub-core/fs/bfs.c
+++ b/grub-core/fs/bfs.c
@@ -891,6 +891,9 @@ grub_bfs_dir_iter (const char *name, grub_uint64_t value,
   info.mtime = grub_bfs_to_cpu64 (ino.ino.mtime) >> 16;
 #endif
   info.dir = ((grub_bfs_to_cpu32 (ino.ino.mode) & ATTR_TYPE) == ATTR_DIR);
+#ifdef GRUB_UTIL
+  info.size = grub_bfs_to_cpu64 (ino.ino.size);
+#endif
   return ctx->hook (name, &info, ctx->hook_data);
 }
 
diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
index 196f301..2b169aa 100644
--- a/grub-core/fs/btrfs.c
+++ b/grub-core/fs/btrfs.c
@@ -1581,6 +1581,9 @@ grub_btrfs_dir (grub_device_t device, const char *path,
            {
              info.mtime = grub_le_to_cpu64 (inode.mtime.sec);
              info.mtimeset = 1;
+#ifdef GRUB_UTIL
+             info.size = grub_le_to_cpu64 (inode.size);
+#endif
            }
          c = cdirel->name[grub_le_to_cpu16 (cdirel->n)];
          cdirel->name[grub_le_to_cpu16 (cdirel->n)] = 0;
diff --git a/grub-core/fs/cpio.c b/grub-core/fs/cpio.c
index 71d7fa4..c6885d4 100644
--- a/grub-core/fs/cpio.c
+++ b/grub-core/fs/cpio.c
@@ -573,6 +573,9 @@ grub_cpio_dir (grub_device_t device, const char *path_in,
              info.dir = (p != NULL) || ((mode & ATTR_TYPE) == ATTR_DIR);
              info.mtime = mtime;
              info.mtimeset = 1;
+#ifdef GRUB_UTIL
+             info.size = data->size;
+#endif
 
              if (hook (n, &info, hook_data))
                {
diff --git a/grub-core/fs/ext2.c b/grub-core/fs/ext2.c
index 18429ac..215d1d2 100644
--- a/grub-core/fs/ext2.c
+++ b/grub-core/fs/ext2.c
@@ -884,6 +884,10 @@ grub_ext2_dir_iter (const char *filename, enum 
grub_fshelp_filetype filetype,
     {
       info.mtimeset = 1;
       info.mtime = grub_le_to_cpu32 (node->inode.mtime);
+#ifdef GRUB_UTIL
+      info.size = grub_cpu_to_le32 (node->inode.size) |
+                 (((grub_off_t) grub_cpu_to_le32 (node->inode.size_high)) << 
32);
+#endif
     }
 
   info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);
diff --git a/grub-core/fs/fat.c b/grub-core/fs/fat.c
index db28158..f6cb0cd 100644
--- a/grub-core/fs/fat.c
+++ b/grub-core/fs/fat.c
@@ -894,6 +894,9 @@ grub_fat_find_dir (grub_disk_t disk, struct grub_fat_data 
*data,
 
       info.dir = !! (ctxt.dir.attr & GRUB_FAT_ATTR_DIRECTORY);
       info.case_insensitive = 1;
+#ifdef GRUB_UTIL
+      info.size = data->file_size;
+#endif
 
 #ifdef MODE_EXFAT
       if (!ctxt.dir.have_stream)
diff --git a/grub-core/fs/hfs.c b/grub-core/fs/hfs.c
index 73ac7f9..39fec63 100644
--- a/grub-core/fs/hfs.c
+++ b/grub-core/fs/hfs.c
@@ -1225,6 +1225,9 @@ grub_hfs_dir_hook (struct grub_hfs_record *rec, void 
*hook_arg)
       info.dir = 0;
       info.mtimeset = 1;
       info.mtime = grub_be_to_cpu32 (frec->mtime) - 2082844800;
+#ifdef GRUB_UTIL
+      info.size = grub_be_to_cpu32 (frec->size);
+#endif
       return ctx->hook (fname, &info, ctx->hook_data);
     }
 
diff --git a/grub-core/fs/hfsplus.c b/grub-core/fs/hfsplus.c
index a507c0f..47e4e58 100644
--- a/grub-core/fs/hfsplus.c
+++ b/grub-core/fs/hfsplus.c
@@ -1013,6 +1013,9 @@ grub_hfsplus_dir_iter (const char *filename,
   info.mtimeset = 1;
   info.mtime = node->mtime;
   info.case_insensitive = !! (filetype & GRUB_FSHELP_CASE_INSENSITIVE);
+#ifdef GRUB_UTIL
+  info.size = node->size;
+#endif
   grub_free (node);
   return ctx->hook (filename, &info, ctx->hook_data);
 }
diff --git a/grub-core/fs/iso9660.c b/grub-core/fs/iso9660.c
index cdbd6dc..291ee00 100644
--- a/grub-core/fs/iso9660.c
+++ b/grub-core/fs/iso9660.c
@@ -861,6 +861,9 @@ grub_iso9660_dir_iter (const char *filename,
   grub_memset (&info, 0, sizeof (info));
   info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);
   info.mtimeset = !!iso9660_to_unixtime2 (&node->dirents[0].mtime, 
&info.mtime);
+#ifdef GRUB_UTIL
+  info.size = get_node_size (node);
+#endif
 
   grub_free (node);
   return ctx->hook (filename, &info, ctx->hook_data);
diff --git a/grub-core/fs/jfs.c b/grub-core/fs/jfs.c
index 4122eff..f800c50 100644
--- a/grub-core/fs/jfs.c
+++ b/grub-core/fs/jfs.c
@@ -832,6 +832,9 @@ grub_jfs_dir (grub_device_t device, const char *path,
                  & GRUB_JFS_FILETYPE_MASK) == GRUB_JFS_FILETYPE_DIR;
       info.mtimeset = 1;
       info.mtime = grub_le_to_cpu32 (inode.mtime.sec);
+#ifdef GRUB_UTIL
+      info.size = grub_le_to_cpu64 (inode.size);
+#endif
       if (hook (diro->name, &info, hook_data))
        goto fail;
     }
diff --git a/grub-core/fs/minix.c b/grub-core/fs/minix.c
index 225770a..1678adc 100644
--- a/grub-core/fs/minix.c
+++ b/grub-core/fs/minix.c
@@ -587,6 +587,9 @@ grub_minix_dir (grub_device_t device, const char *path,
                   & GRUB_MINIX_IFDIR) == GRUB_MINIX_IFDIR);
       info.mtimeset = 1;
       info.mtime = grub_minix_to_cpu32 (data->inode.mtime);
+#ifdef GRUB_UTIL
+      info.size = GRUB_MINIX_INODE_SIZE (data);
+#endif
 
       if (hook (filename, &info, hook_data) ? 1 : 0)
        break;
diff --git a/grub-core/fs/nilfs2.c b/grub-core/fs/nilfs2.c
index 3f28bd7..525690f 100644
--- a/grub-core/fs/nilfs2.c
+++ b/grub-core/fs/nilfs2.c
@@ -1056,6 +1056,9 @@ grub_nilfs2_dir_iter (const char *filename, enum 
grub_fshelp_filetype filetype,
     {
       info.mtimeset = 1;
       info.mtime = grub_le_to_cpu64 (node->inode.i_mtime);
+#ifdef GRUB_UTIL
+      info.size = grub_le_to_cpu64 (node->inode.i_size);
+#endif
     }
 
   info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);
diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c
index 5ea2e1b..5850ef2 100644
--- a/grub-core/fs/ntfs.c
+++ b/grub-core/fs/ntfs.c
@@ -1023,6 +1023,13 @@ grub_ntfs_dir_iter (const char *filename, enum 
grub_fshelp_filetype filetype,
   info.mtime = grub_divmod64 (node->mtime, 10000000, 0) 
     - 86400ULL * 365 * (1970 - 1601)
     - 86400ULL * ((1970 - 1601) / 4) + 86400ULL * ((1970 - 1601) / 100);
+#ifdef GRUB_UTIL
+  if (!node->inode_read)
+    init_file (node, node->ino);
+  if (node->inode_read)
+    info.size = node->size;
+  free_file (node);
+#endif
   grub_free (node);
   return ctx->hook (filename, &info, ctx->hook_data);
 }
diff --git a/grub-core/fs/reiserfs.c b/grub-core/fs/reiserfs.c
index eaa7ade..6aef49b 100644
--- a/grub-core/fs/reiserfs.c
+++ b/grub-core/fs/reiserfs.c
@@ -1271,6 +1271,9 @@ grub_reiserfs_dir_iter (const char *filename,
   info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);
   info.mtimeset = 1;
   info.mtime = node->mtime;
+#ifdef GRUB_UTIL
+  info.size = node->size;
+#endif
   grub_free (node);
   return ctx->hook (filename, &info, ctx->hook_data);
 }
diff --git a/grub-core/fs/romfs.c b/grub-core/fs/romfs.c
index 2e35444..c140d25 100644
--- a/grub-core/fs/romfs.c
+++ b/grub-core/fs/romfs.c
@@ -331,6 +331,9 @@ grub_romfs_dir_iter (const char *filename, enum 
grub_fshelp_filetype filetype,
   grub_memset (&info, 0, sizeof (info));
 
   info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);
+#ifdef GRUB_UTIL
+  info.size = grub_be_to_cpu32 (node->file.size);
+#endif
   grub_free (node);
   return ctx->hook (filename, &info, ctx->hook_data);
 }
diff --git a/grub-core/fs/sfs.c b/grub-core/fs/sfs.c
index e7d2f72..67fdee6 100644
--- a/grub-core/fs/sfs.c
+++ b/grub-core/fs/sfs.c
@@ -671,6 +671,9 @@ grub_sfs_dir_iter (const char *filename, enum 
grub_fshelp_filetype filetype,
   info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);
   info.mtime = node->mtime + 8 * 365 * 86400 + 86400 * 2;
   info.mtimeset = 1;
+#ifdef GRUB_UTIL
+  info.size = node->size;
+#endif
   grub_free (node->cache);
   grub_free (node);
   return ctx->hook (filename, &info, ctx->hook_data);
diff --git a/grub-core/fs/squash4.c b/grub-core/fs/squash4.c
index cb3cc3a..4eea4b2 100644
--- a/grub-core/fs/squash4.c
+++ b/grub-core/fs/squash4.c
@@ -656,6 +656,17 @@ grub_squash_dir_iter (const char *filename, enum 
grub_fshelp_filetype filetype,
   info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);
   info.mtimeset = 1;
   info.mtime = grub_le_to_cpu32 (node->ino.mtime);
+#ifdef GRUB_UTIL
+  switch (node->ino.type)
+    {
+    case grub_cpu_to_le16_compile_time (SQUASH_TYPE_LONG_REGULAR):
+      info.size = grub_le_to_cpu64 (node->ino.long_file.size);
+      break;
+    case grub_cpu_to_le16_compile_time (SQUASH_TYPE_REGULAR):
+      info.size = grub_le_to_cpu32 (node->ino.file.size);
+      break;
+    }
+#endif
   grub_free (node);
   return ctx->hook (filename, &info, ctx->hook_data);
 }
diff --git a/grub-core/fs/udf.c b/grub-core/fs/udf.c
index 405935e..e2d45aa 100644
--- a/grub-core/fs/udf.c
+++ b/grub-core/fs/udf.c
@@ -1053,6 +1053,9 @@ grub_udf_dir_iter (const char *filename, enum 
grub_fshelp_filetype filetype,
 
       info.mtime -= 60 * tz;
     }
+#ifdef GRUB_UTIL
+  info.size = U64 (node->block.fe.file_size);
+#endif
   grub_free (node);
   return ctx->hook (filename, &info, ctx->hook_data);
 }
diff --git a/grub-core/fs/ufs.c b/grub-core/fs/ufs.c
index c155912..956bfee 100644
--- a/grub-core/fs/ufs.c
+++ b/grub-core/fs/ufs.c
@@ -695,6 +695,9 @@ grub_ufs_dir (grub_device_t device, const char *path,
        info.mtime = grub_ufs_to_cpu32 (inode.mtime);
 #endif
        info.mtimeset = 1;
+#ifdef GRUB_UTIL
+       info.size = grub_ufs_to_cpu64 (inode.size);
+#endif
 
        if (hook (filename, &info, hook_data))
          break;
diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c
index a5a1700..40851a8 100644
--- a/grub-core/fs/xfs.c
+++ b/grub-core/fs/xfs.c
@@ -733,6 +733,9 @@ grub_xfs_dir_iter (const char *filename, enum 
grub_fshelp_filetype filetype,
       info.mtime = grub_be_to_cpu32 (node->inode.mtime.sec);
     }
   info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);
+#ifdef GRUB_UTIL
+  info.size = grub_be_to_cpu64 (node->inode.size);
+#endif
   grub_free (node);
   return ctx->hook (filename, &info, ctx->hook_data);
 }
diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c
index 822d65b..5635fca 100644
--- a/grub-core/fs/zfs/zfs.c
+++ b/grub-core/fs/zfs/zfs.c
@@ -3821,6 +3821,9 @@ iterate_zap (const char *name, grub_uint64_t val, struct 
grub_zfs_dir_ctx *ctx)
       info.mtimeset = 1;
       info.mtime = grub_zfs_to_cpu64 (grub_get_unaligned64 ((char *) sahdrp + 
hdrsize + SA_MTIME_OFFSET), dn.endian);
       info.case_insensitive = ctx->data->subvol.case_insensitive;
+#ifdef GRUB_UTIL
+      info.size = grub_zfs_to_cpu64 (grub_get_unaligned64 ((char *) sahdrp + 
hdrsize + SA_SIZE_OFFSET), dn.endian);
+#endif
     }
   
   if (dn.dn.dn_bonustype == DMU_OT_ZNODE)
diff --git a/include/grub/fs.h b/include/grub/fs.h
index e451797..e6663d8 100644
--- a/include/grub/fs.h
+++ b/include/grub/fs.h
@@ -39,6 +39,9 @@ struct grub_dirhook_info
   unsigned mtimeset:1;
   unsigned case_insensitive:1;
   grub_int32_t mtime;
+#ifdef GRUB_UTIL
+  grub_off_t size;
+#endif
 };
 
 typedef int (*grub_fs_dir_hook_t) (const char *filename,
diff --git a/util/grub-mount.c b/util/grub-mount.c
index d0ab6a2..5a055ab 100644
--- a/util/grub-mount.c
+++ b/util/grub-mount.c
@@ -205,17 +205,7 @@ fuse_getattr (const char *path, struct stat *st)
   st->st_uid = 0;
   st->st_gid = 0;
   st->st_rdev = 0;
-  if (!ctx.file_info.dir)
-    {
-      grub_file_t file;
-      file = grub_file_open (path);
-      if (! file)
-       return translate_error ();
-      st->st_size = file->size;
-      grub_file_close (file);
-    }
-  else
-    st->st_size = 0;
+  st->st_size = ctx.file_info.size;
   st->st_blksize = 512;
   st->st_blocks = (st->st_size + 511) >> 9;
   st->st_atime = st->st_mtime = st->st_ctime = ctx.file_info.mtimeset
@@ -297,18 +287,7 @@ fuse_readdir_call_fill (const char *filename,
 
   grub_memset (&st, 0, sizeof (st));
   st.st_mode = info->dir ? (0555 | S_IFDIR) : (0444 | S_IFREG);
-  if (!info->dir)
-    {
-      grub_file_t file;
-      char *tmp;
-      tmp = xasprintf ("%s/%s", ctx->path, filename);
-      file = grub_file_open (tmp);
-      free (tmp);
-      if (! file)
-       return translate_error ();
-      st.st_size = file->size;
-      grub_file_close (file);
-    }
+  st.st_size = info->size;
   st.st_blksize = 512;
   st.st_blocks = (st.st_size + 511) >> 9;
   st.st_atime = st.st_mtime = st.st_ctime
-- 
tg: (475ef5e..) symlink/file-size (depends on: master)