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

["Vladimir 'φ-coder/phcoder' Serbinenko" <phcoder@gmail.com>]

[-- 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 --]