grub-devel.gnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Ignore symlink traversal failures in grub-mount readdir
@ 2012-10-12 16:09 Colin Watson
  2012-10-15 15:08 ` Dave Vasilevsky
  2013-03-09 16:47 ` Andrey Borzenkov
  0 siblings, 2 replies; 6+ messages in thread
From: Colin Watson @ 2012-10-12 16:09 UTC (permalink / raw)
  To: grub-devel

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?

2012-10-12  Colin Watson  <cjwatson@ubuntu.com>

=== modified file 'util/grub-mount.c'
--- util/grub-mount.c	2012-03-08 18:09:05 +0000
+++ util/grub-mount.c	2012-10-12 16:08:39 +0000
@@ -292,7 +292,20 @@ fuse_readdir (const char *path, void *bu
 	file = grub_file_open (tmp);
 	free (tmp);
 	if (! file)
-	  return translate_error ();
+	  {
+	    /* We cannot handle symlinks properly yet, and symlinks to
+	       directories will cause us to reach here.  Symlink loops or
+	       dangling symlinks will also cause an error.  For the
+	       meantime, while treating these as zero-length files is wrong,
+	       it's better than failing the whole readdir call by returning
+	       translate_error ().
+
+	       Ultimately, we should be able to tell from the
+	       grub_dirhook_info that this is a symlink, and fill in the
+	       attributes of the symlink rather than its target.  */
+	    grub_errno = GRUB_ERR_NONE;
+	    return 0;
+	  }
 	st.st_size = file->size;
 	grub_file_close (file);
       }

Thanks,

-- 
Colin Watson                                       [cjwatson@ubuntu.com]


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Ignore symlink traversal failures in grub-mount readdir
  2012-10-12 16:09 [PATCH] Ignore symlink traversal failures in grub-mount readdir Colin Watson
@ 2012-10-15 15:08 ` Dave Vasilevsky
  2013-03-09 16:47 ` Andrey Borzenkov
  1 sibling, 0 replies; 6+ messages in thread
From: Dave Vasilevsky @ 2012-10-15 15:08 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: cjwatson

> 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.

I used this extremely kludgy fix:
http://bazaar.launchpad.net/~djvasi/grub/mac-grub-mount/revision/4246#util/grub-mount.c
. It just assumes that if we get GRUB_ERR_BAD_FILE_TYPE, it must have
been a symlink to a directory, and presents it as such. Obviously this
is also a temporary hack, not sure if it's any better or worse than
yours.

Cheers,
Dave Vasilevsky


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Ignore symlink traversal failures in grub-mount readdir
  2012-10-12 16:09 [PATCH] Ignore symlink traversal failures in grub-mount readdir Colin Watson
  2012-10-15 15:08 ` Dave Vasilevsky
@ 2013-03-09 16:47 ` Andrey Borzenkov
  2013-03-10 13:01   ` Vladimir 'φ-coder/phcoder' Serbinenko
  1 sibling, 1 reply; 6+ messages in thread
From: Andrey Borzenkov @ 2013-03-09 16:47 UTC (permalink / raw)
  To: grub-devel

В Fri, 12 Oct 2012 17:09:54 +0100
Colin Watson <cjwatson@ubuntu.com> пишет:

> 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 <arvidjaar@gmail.com>
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 <arvidjaar@gmail.com>

---
 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)


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Ignore symlink traversal failures in grub-mount readdir
  2013-03-09 16:47 ` Andrey Borzenkov
@ 2013-03-10 13:01   ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-03-10 13:02     ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-11-28 13:30     ` Andrey Borzenkov
  0 siblings, 2 replies; 6+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-03-10 13:01 UTC (permalink / raw)
  To: The development of GNU GRUB

[-- Attachment #1: Type: text/plain, Size: 2249 bytes --]

On 09.03.2013 17:47, Andrey Borzenkov wrote:

> В Fri, 12 Oct 2012 17:09:54 +0100
> Colin Watson <cjwatson@ubuntu.com> пишет:
> 
>> 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.
> 

Some time ago I made a similar patch but for another motivation: current
code is way too inefficient for large directories as you have to rescan
directory for every file. The problem with this patch is 10 bytes
increase of core.img. This may be acceptable given this problem (it
happens in ls on runtime as well) and inefficency of scanning.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Ignore symlink traversal failures in grub-mount readdir
  2013-03-10 13:01   ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2013-03-10 13:02     ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-11-28 13:30     ` Andrey Borzenkov
  1 sibling, 0 replies; 6+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-03-10 13:02 UTC (permalink / raw)
  To: The development of GNU GRUB


[-- Attachment #1.1: Type: text/plain, Size: 2372 bytes --]

On 10.03.2013 14:01, Vladimir 'φ-coder/phcoder' Serbinenko wrote:

> On 09.03.2013 17:47, Andrey Borzenkov wrote:
> 
>> В Fri, 12 Oct 2012 17:09:54 +0100
>> Colin Watson <cjwatson@ubuntu.com> пишет:
>>
>>> 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.
>>
> 
> Some time ago I made a similar patch but for another motivation: current
> code is way too inefficient for large directories as you have to rescan
> directory for every file. The problem with this patch is 10 bytes
> increase of core.img. This may be acceptable given this problem (it
> happens in ls on runtime as well) and inefficency of scanning.
> 



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: size.diff --]
[-- Type: text/x-diff; name="size.diff", Size: 12849 bytes --]

=== modified file 'grub-core/commands/ls.c'
--- grub-core/commands/ls.c	2013-01-21 16:46:24 +0000
+++ grub-core/commands/ls.c	2013-03-10 00:21:42 +0000
@@ -118,34 +118,12 @@
 
   if (! info->dir)
     {
-      grub_file_t file;
-      char *pathname;
-
-      if (ctx->dirname[grub_strlen (ctx->dirname) - 1] == '/')
-	pathname = grub_xasprintf ("%s%s", ctx->dirname, filename);
-      else
-	pathname = grub_xasprintf ("%s/%s", ctx->dirname, filename);
-
-      if (!pathname)
-	return 1;
-
-      /* XXX: For ext2fs symlinks are detected as files while they
-	 should be reported as directories.  */
-      grub_file_filter_disable_compression ();
-      file = grub_file_open (pathname);
-      if (! file)
-	{
-	  grub_errno = 0;
-	  grub_free (pathname);
-	  return 0;
-	}
-
       if (! ctx->human)
-	grub_printf ("%-12llu", (unsigned long long) file->size);
+	grub_printf ("%-12llu", (unsigned long long) info->size);
       else
 	{
-	  grub_uint64_t fsize = file->size * 100ULL;
-	  grub_uint64_t fsz = file->size;
+	  grub_uint64_t fsize = info->size * 100ULL;
+	  grub_uint64_t fsz = info->size;
 	  int units = 0;
 	  char buf[20];
 
@@ -168,11 +146,9 @@
 	      grub_printf ("%-12s", buf);
 	    }
 	  else
-	    grub_printf ("%-12llu", (unsigned long long) file->size);
+	    grub_printf ("%-12llu", (unsigned long long) info->size);
 
 	}
-      grub_file_close (file);
-      grub_free (pathname);
     }
   else
     grub_printf ("%-12s", _("DIR"));

=== modified file 'grub-core/fs/affs.c'
--- grub-core/fs/affs.c	2013-02-27 16:19:15 +0000
+++ grub-core/fs/affs.c	2013-03-10 00:21:07 +0000
@@ -565,6 +565,7 @@
   info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);
   info.mtimeset = 1;
   info.mtime = aftime2ctime (&node->di.mtime);
+  info.size = grub_be_to_cpu32 (node->di.size);
   grub_free (node);
   return ctx->hook (filename, &info, ctx->hook_data);
 }

=== modified file 'grub-core/fs/bfs.c'
--- grub-core/fs/bfs.c	2013-02-27 16:19:15 +0000
+++ grub-core/fs/bfs.c	2013-03-10 00:11:14 +0000
@@ -890,6 +890,7 @@
 #else
   info.mtime = grub_bfs_to_cpu64 (ino.ino.mtime) >> 16;
 #endif
+  info.size = grub_bfs_to_cpu64 (ino.ino.size);
   info.dir = ((grub_bfs_to_cpu32 (ino.ino.mode) & ATTR_TYPE) == ATTR_DIR);
   return ctx->hook (name, &info, ctx->hook_data);
 }

=== modified file 'grub-core/fs/btrfs.c'
--- grub-core/fs/btrfs.c	2013-01-21 01:33:46 +0000
+++ grub-core/fs/btrfs.c	2013-03-10 00:05:22 +0000
@@ -1581,6 +1581,7 @@
 	    {
 	      info.mtime = grub_le_to_cpu64 (inode.mtime.sec);
 	      info.mtimeset = 1;
+	      info.size = grub_le_to_cpu64 (inode.size);
 	    }
 	  c = cdirel->name[grub_le_to_cpu16 (cdirel->n)];
 	  cdirel->name[grub_le_to_cpu16 (cdirel->n)] = 0;

=== modified file 'grub-core/fs/cpio.c'
--- grub-core/fs/cpio.c	2013-01-21 01:33:46 +0000
+++ grub-core/fs/cpio.c	2013-03-10 00:05:22 +0000
@@ -573,6 +573,7 @@
 	      info.dir = (p != NULL) || ((mode & ATTR_TYPE) == ATTR_DIR);
 	      info.mtime = mtime;
 	      info.mtimeset = 1;
+	      info.size = data->size;
 
 	      if (hook (n, &info, hook_data))
 		{

=== modified file 'grub-core/fs/ext2.c'
--- grub-core/fs/ext2.c	2013-02-27 16:19:15 +0000
+++ grub-core/fs/ext2.c	2013-03-10 12:08:40 +0000
@@ -884,6 +884,8 @@
     {
       info.mtimeset = 1;
       info.mtime = grub_le_to_cpu32 (node->inode.mtime);
+      info.size = grub_le_to_cpu32 (node->inode.size)
+	| (((grub_off_t) grub_le_to_cpu32 (node->inode.size_high)) << 32);
     }
 
   info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);

=== modified file 'grub-core/fs/fat.c'
--- grub-core/fs/fat.c	2013-02-27 16:19:15 +0000
+++ grub-core/fs/fat.c	2013-03-10 12:46:55 +0000
@@ -894,6 +894,7 @@
 
       info.dir = !! (ctxt.dir.attr & GRUB_FAT_ATTR_DIRECTORY);
       info.case_insensitive = 1;
+      info.size = ctxt.dir.file_size;
 
 #ifdef MODE_EXFAT
       if (!ctxt.dir.have_stream)

=== modified file 'grub-core/fs/hfs.c'
--- grub-core/fs/hfs.c	2013-03-02 10:31:00 +0000
+++ grub-core/fs/hfs.c	2013-03-10 00:13:16 +0000
@@ -1225,6 +1225,7 @@
       info.dir = 0;
       info.mtimeset = 1;
       info.mtime = grub_be_to_cpu32 (frec->mtime) - 2082844800;
+      info.size = grub_be_to_cpu32 (frec->size);
       return ctx->hook (fname, &info, ctx->hook_data);
     }
 

=== modified file 'grub-core/fs/hfsplus.c'
--- grub-core/fs/hfsplus.c	2013-03-01 13:02:27 +0000
+++ grub-core/fs/hfsplus.c	2013-03-10 00:13:52 +0000
@@ -1012,6 +1012,7 @@
   info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);
   info.mtimeset = 1;
   info.mtime = node->mtime;
+  info.size = node->size;
   info.case_insensitive = !! (filetype & GRUB_FSHELP_CASE_INSENSITIVE);
   grub_free (node);
   return ctx->hook (filename, &info, ctx->hook_data);

=== modified file 'grub-core/fs/iso9660.c'
--- grub-core/fs/iso9660.c	2013-03-07 08:11:36 +0000
+++ grub-core/fs/iso9660.c	2013-03-10 00:14:17 +0000
@@ -861,6 +861,7 @@
   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);
+  info.size = get_node_size (node);
 
   grub_free (node);
   return ctx->hook (filename, &info, ctx->hook_data);

=== modified file 'grub-core/fs/jfs.c'
--- grub-core/fs/jfs.c	2013-02-28 09:51:32 +0000
+++ grub-core/fs/jfs.c	2013-03-10 00:14:41 +0000
@@ -832,6 +832,7 @@
 		  & GRUB_JFS_FILETYPE_MASK) == GRUB_JFS_FILETYPE_DIR;
       info.mtimeset = 1;
       info.mtime = grub_le_to_cpu32 (inode.mtime.sec);
+      info.size = grub_le_to_cpu64 (inode.size);
       if (hook (diro->name, &info, hook_data))
 	goto fail;
     }

=== modified file 'grub-core/fs/minix.c'
--- grub-core/fs/minix.c	2013-02-28 09:50:01 +0000
+++ grub-core/fs/minix.c	2013-03-10 00:05:22 +0000
@@ -587,6 +587,7 @@
 		   & GRUB_MINIX_IFDIR) == GRUB_MINIX_IFDIR);
       info.mtimeset = 1;
       info.mtime = grub_minix_to_cpu32 (data->inode.mtime);
+      info.size = GRUB_MINIX_INODE_SIZE (data);
 
       if (hook (filename, &info, hook_data) ? 1 : 0)
 	break;

=== modified file 'grub-core/fs/nilfs2.c'
--- grub-core/fs/nilfs2.c	2013-02-27 16:19:15 +0000
+++ grub-core/fs/nilfs2.c	2013-03-10 00:15:05 +0000
@@ -1056,6 +1056,7 @@
     {
       info.mtimeset = 1;
       info.mtime = grub_le_to_cpu64 (node->inode.i_mtime);
+      info.size = grub_le_to_cpu64 (node->inode.i_size);
     }
 
   info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);

=== modified file 'grub-core/fs/ntfs.c'
--- grub-core/fs/ntfs.c	2013-02-27 16:19:15 +0000
+++ grub-core/fs/ntfs.c	2013-03-10 00:15:49 +0000
@@ -1019,10 +1019,13 @@
 
   grub_memset (&info, 0, sizeof (info));
   info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);
+  init_file (node, node->ino);
   info.mtimeset = 1;
   info.mtime = grub_divmod64 (node->mtime, 10000000, 0) 
     - 86400ULL * 365 * (1970 - 1601)
     - 86400ULL * ((1970 - 1601) / 4) + 86400ULL * ((1970 - 1601) / 100);
+  info.size = node->size;
+  free_file (node);
   grub_free (node);
   return ctx->hook (filename, &info, ctx->hook_data);
 }

=== modified file 'grub-core/fs/reiserfs.c'
--- grub-core/fs/reiserfs.c	2013-02-27 16:19:15 +0000
+++ grub-core/fs/reiserfs.c	2013-03-10 00:05:22 +0000
@@ -875,6 +875,7 @@
 				    entry_v1_stat.rdev,
 				    entry_v1_stat.first_direct_byte);
 #endif
+		      entry_item->size = (grub_off_t) grub_le_to_cpu64 (entry_v1_stat.size);
 		      entry_item->mtime = grub_le_to_cpu32 (entry_v1_stat.mtime);
 		      if ((grub_le_to_cpu16 (entry_v1_stat.mode) & S_IFLNK)
 			  == S_IFLNK)
@@ -923,6 +924,7 @@
 				    entry_v2_stat.blocks,
 				    entry_v2_stat.first_direct_byte);
 #endif
+		      entry_item->size = (grub_off_t) grub_le_to_cpu64 (entry_v2_stat.size);
 		      entry_item->mtime = grub_le_to_cpu32 (entry_v2_stat.mtime);
 		      entry_item->size = (grub_off_t) grub_le_to_cpu64 (entry_v2_stat.size);
 		      if ((grub_le_to_cpu16 (entry_v2_stat.mode) & S_IFLNK)

=== modified file 'grub-core/fs/romfs.c'
--- grub-core/fs/romfs.c	2013-02-27 16:19:15 +0000
+++ grub-core/fs/romfs.c	2013-03-10 00:16:08 +0000
@@ -331,6 +331,7 @@
   grub_memset (&info, 0, sizeof (info));
 
   info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);
+  info.size = grub_be_to_cpu32 (node->file.size);
   grub_free (node);
   return ctx->hook (filename, &info, ctx->hook_data);
 }

=== modified file 'grub-core/fs/sfs.c'
--- grub-core/fs/sfs.c	2013-02-27 16:19:15 +0000
+++ grub-core/fs/sfs.c	2013-03-10 00:16:47 +0000
@@ -671,6 +671,7 @@
   info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);
   info.mtime = node->mtime + 8 * 365 * 86400 + 86400 * 2;
   info.mtimeset = 1;
+  info.size = node->size;
   grub_free (node->cache);
   grub_free (node);
   return ctx->hook (filename, &info, ctx->hook_data);

=== modified file 'grub-core/fs/squash4.c'
--- grub-core/fs/squash4.c	2013-01-21 01:33:46 +0000
+++ grub-core/fs/squash4.c	2013-03-10 00:17:17 +0000
@@ -656,6 +656,16 @@
   info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);
   info.mtimeset = 1;
   info.mtime = grub_le_to_cpu32 (node->ino.mtime);
+  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;
+    }
+
   grub_free (node);
   return ctx->hook (filename, &info, ctx->hook_data);
 }

=== modified file 'grub-core/fs/udf.c'
--- grub-core/fs/udf.c	2013-02-27 16:19:15 +0000
+++ grub-core/fs/udf.c	2013-03-10 00:17:39 +0000
@@ -1026,6 +1026,7 @@
 
   grub_memset (&info, 0, sizeof (info));
   info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);
+  info.size = U64 (node->block.fe.file_size);
   if (U16 (node->block.fe.tag.tag_ident) == GRUB_UDF_TAG_IDENT_FE)
     tstamp = &node->block.fe.modification_time;
   else if (U16 (node->block.fe.tag.tag_ident) == GRUB_UDF_TAG_IDENT_EFE)

=== modified file 'grub-core/fs/ufs.c'
--- grub-core/fs/ufs.c	2013-02-27 16:19:15 +0000
+++ grub-core/fs/ufs.c	2013-03-10 00:17:58 +0000
@@ -696,6 +696,12 @@
 #endif
 	info.mtimeset = 1;
 
+#ifdef MODE_UFS2
+	info.size = grub_le_to_cpu64 (inode.size);
+#else
+	info.size = grub_le_to_cpu32 (inode.size);
+#endif
+
 	if (hook (filename, &info, hook_data))
 	  break;
       }

=== modified file 'grub-core/fs/xfs.c'
--- grub-core/fs/xfs.c	2013-02-27 16:19:15 +0000
+++ grub-core/fs/xfs.c	2013-03-10 00:18:18 +0000
@@ -731,6 +731,7 @@
     {
       info.mtimeset = 1;
       info.mtime = grub_be_to_cpu32 (node->inode.mtime.sec);
+      info.size = grub_be_to_cpu64 (node->inode.size);
     }
   info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);
   grub_free (node);

=== modified file 'grub-core/fs/zfs/zfs.c'
--- grub-core/fs/zfs/zfs.c	2013-01-21 01:33:46 +0000
+++ grub-core/fs/zfs/zfs.c	2013-03-10 00:05:22 +0000
@@ -3768,12 +3768,14 @@
       hdrsize = SA_HDR_SIZE (((sa_hdr_phys_t *) sahdrp));
       info->mtimeset = 1;
       info->mtime = grub_zfs_to_cpu64 (grub_get_unaligned64 ((char *) sahdrp + hdrsize + SA_MTIME_OFFSET), dn.endian);
+      info->size = grub_zfs_to_cpu64 (grub_get_unaligned64 ((char *) sahdrp + hdrsize + SA_SIZE_OFFSET), dn.endian);
     }
 
   if (dn.dn.dn_bonustype == DMU_OT_ZNODE)
     {
       info->mtimeset = 1;
       info->mtime = grub_zfs_to_cpu64 (((znode_phys_t *) DN_BONUS (&dn.dn))->zp_mtime[0], dn.endian);
+      info->size = grub_zfs_to_cpu64 (((znode_phys_t *) DN_BONUS (&data->dnode.dn))->zp_size, dn.endian);
     }
   return;
 }

=== modified file 'include/grub/fs.h'
--- include/grub/fs.h	2013-01-21 01:33:46 +0000
+++ include/grub/fs.h	2013-03-10 00:05:22 +0000
@@ -39,6 +39,7 @@
   unsigned mtimeset:1;
   unsigned case_insensitive:1;
   grub_int32_t mtime;
+  grub_off_t size;
 };
 
 typedef int (*grub_fs_dir_hook_t) (const char *filename,

=== modified file 'util/grub-mount.c'
--- util/grub-mount.c	2013-01-21 01:33:46 +0000
+++ util/grub-mount.c	2013-03-10 00:10:49 +0000
@@ -206,14 +206,7 @@
   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);
-    }
+    st->st_size = ctx.file_info.size;
   else
     st->st_size = 0;
   st->st_blksize = 512;


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Ignore symlink traversal failures in grub-mount readdir
  2013-03-10 13:01   ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-03-10 13:02     ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2013-11-28 13:30     ` Andrey Borzenkov
  1 sibling, 0 replies; 6+ messages in thread
From: Andrey Borzenkov @ 2013-11-28 13:30 UTC (permalink / raw)
  To: grub-devel

[-- Attachment #1: Type: text/plain, Size: 2598 bytes --]

В Sun, 10 Mar 2013 14:01:53 +0100
Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:

> On 09.03.2013 17:47, Andrey Borzenkov wrote:
> 
> > В Fri, 12 Oct 2012 17:09:54 +0100
> > Colin Watson <cjwatson@ubuntu.com> пишет:
> > 
> >> 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.
> > 
> 
> Some time ago I made a similar patch but for another motivation: current
> code is way too inefficient for large directories as you have to rescan
> directory for every file. The problem with this patch is 10 bytes
> increase of core.img. This may be acceptable given this problem (it
> happens in ls on runtime as well) and inefficency of scanning.
> 

Any thoughts on it? I'm fine with either approach; there was some
reduction of core size in the meantime so may be slight increase is
acceptable?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-11-28 13:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-12 16:09 [PATCH] Ignore symlink traversal failures in grub-mount readdir Colin Watson
2012-10-15 15:08 ` Dave Vasilevsky
2013-03-09 16:47 ` Andrey Borzenkov
2013-03-10 13:01   ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-03-10 13:02     ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-11-28 13:30     ` Andrey Borzenkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).