util: Detect more I/O errors

Many of GRUB's utilities don't check anywhere near all the possible
write errors.  For example, if grub-install runs out of space when
copying a file, it won't notice.  There were missing checks for the
return values of write, fflush, fsync, and close (or the equivalents on
other OSes), all of which must be checked.

I tried to be consistent with the existing logging practices of the
various hostdisk implementations, but they weren't entirely consistent
to start with so I used my judgement.  The result at least looks
reasonable on GNU/Linux when I provoke a write error:

  Installing for x86_64-efi platform.
  grub-install: error: cannot copy `/usr/lib/grub/x86_64-efi-signed/grubx64.efi.signed' to `/boot/efi/EFI/debian/grubx64.efi': No space left on device.

There are more missing checks in other utilities, but this should fix
the most critical ones.

Fixes Debian bug #922741.

Signed-off-by: Colin Watson <cjwatson@ubuntu.com>
Reviewed-by: Steve McIntyre <93sam@debian.org>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

diff --git a/grub-core/osdep/aros/hostdisk.c b/grub-core/osdep/aros/hostdisk.c
index 7d99b54b89c6eb3b1b0fe81c30a36bb3be4f9ce7..2be654ca3bd6262e310205e73d26ea2ff7ce44be 100644 (file)
--- a/grub-core/osdep/aros/hostdisk.c
+++ b/grub-core/osdep/aros/hostdisk.c
@@ -439,36 +439,44 @@ grub_util_get_fd_size (grub_util_fd_t fd,
   return -1;
 }
 
-void
+int
 grub_util_fd_close (grub_util_fd_t fd)
 {
   switch (fd->type)
     {
     case GRUB_UTIL_FD_FILE:
-      close (fd->fd);
-      return;
+      return close (fd->fd);
     case GRUB_UTIL_FD_DISK:
       CloseDevice ((struct IORequest *) fd->ioreq);
       DeleteIORequest((struct IORequest *) fd->ioreq);
       DeleteMsgPort (fd->mp);
-      return;
+      return 0;
     }
+  return 0;
 }
 
 static int allow_fd_syncs = 1;
 
-static void
+static int
 grub_util_fd_sync_volume (grub_util_fd_t fd)
 {
+  LONG err;
+
   fd->ioreq->iotd_Req.io_Command = CMD_UPDATE;
   fd->ioreq->iotd_Req.io_Length = 0;
   fd->ioreq->iotd_Req.io_Data = 0;
   fd->ioreq->iotd_Req.io_Offset = 0;
   fd->ioreq->iotd_Req.io_Actual = 0;
-  DoIO ((struct IORequest *) fd->ioreq);
+  err = DoIO ((struct IORequest *) fd->ioreq);
+  if (err)
+    {
+      grub_util_info ("I/O failed with error %d, IoErr=%d", (int)err, (int) IoErr ());
+      return -1;
+    }
+  return 0;
 }
 
-void
+int
 grub_util_fd_sync (grub_util_fd_t fd)
 {
   if (allow_fd_syncs)
@@ -476,22 +484,22 @@ grub_util_fd_sync (grub_util_fd_t fd)
       switch (fd->type)
        {
        case GRUB_UTIL_FD_FILE:
-         fsync (fd->fd);
-         return;
+         return fsync (fd->fd);
        case GRUB_UTIL_FD_DISK:
-         grub_util_fd_sync_volume (fd);
-         return;
+         return grub_util_fd_sync_volume (fd);
        }
     }
+  return 0;
 }
 
-void
+int
 grub_util_file_sync (FILE *f)
 {
-  fflush (f);
+  if (fflush (f) != 0)
+    return -1;
   if (!allow_fd_syncs)
-    return;
-  fsync (fileno (f));
+    return 0;
+  return fsync (fileno (f));
 }
 
 void

diff --git a/grub-core/osdep/unix/hostdisk.c b/grub-core/osdep/unix/hostdisk.c
index 5450cf4166e04c2cfab806e93dfa63c6abdb9698..91150969bb7892c0586f1be4a482be736ef432c9 100644 (file)
--- a/grub-core/osdep/unix/hostdisk.c
+++ b/grub-core/osdep/unix/hostdisk.c
@@ -177,20 +177,22 @@ grub_util_fd_strerror (void)
 
 static int allow_fd_syncs = 1;
 
-void
+int
 grub_util_fd_sync (grub_util_fd_t fd)
 {
   if (allow_fd_syncs)
-    fsync (fd);
+    return fsync (fd);
+  return 0;
 }
 
-void
+int
 grub_util_file_sync (FILE *f)
 {
-  fflush (f);
+  if (fflush (f) != 0)
+    return -1;
   if (!allow_fd_syncs)
-    return;
-  fsync (fileno (f));
+    return 0;
+  return fsync (fileno (f));
 }
 
 void
@@ -199,10 +201,10 @@ grub_util_disable_fd_syncs (void)
   allow_fd_syncs = 0;
 }
 
-void
+int
 grub_util_fd_close (grub_util_fd_t fd)
 {
-  close (fd);
+  return close (fd);
 }
 
 char *

diff --git a/grub-core/osdep/windows/hostdisk.c b/grub-core/osdep/windows/hostdisk.c
index 85507af88e393c12e56327c94408371cd630cf05..355100789a71f2688f553c87ac5fb7d5192d920f 100644 (file)
--- a/grub-core/osdep/windows/hostdisk.c
+++ b/grub-core/osdep/windows/hostdisk.c
@@ -275,11 +275,18 @@ grub_util_fd_write (grub_util_fd_t fd, const char *buf, size_t len)
 
 static int allow_fd_syncs = 1;
 
-void
+int
 grub_util_fd_sync (grub_util_fd_t fd)
 {
   if (allow_fd_syncs)
-    FlushFileBuffers (fd);
+    {
+      if (!FlushFileBuffers (fd))
+       {
+         grub_util_info ("flush err %x", (int) GetLastError ());
+         return -1;
+       }
+    }
+  return 0;
 }
 
 void
@@ -288,10 +295,15 @@ grub_util_disable_fd_syncs (void)
   allow_fd_syncs = 0;
 }
 
-void
+int
 grub_util_fd_close (grub_util_fd_t fd)
 {
-  CloseHandle (fd);
+  if (!CloseHandle (fd))
+    {
+      grub_util_info ("close err %x", (int) GetLastError ());
+      return -1;
+    }
+  return 0;
 }
 
 const char *
@@ -620,16 +632,25 @@ grub_util_fopen (const char *path, const char *mode)
   return ret;
 }
 
-void
+int
 grub_util_file_sync (FILE *f)
 {
   HANDLE hnd;
 
-  fflush (f);
+  if (fflush (f) != 0)
+    {
+      grub_util_info ("fflush err %x", (int) GetLastError ());
+      return -1;
+    }
   if (!allow_fd_syncs)
-    return;
+    return 0;
   hnd = (HANDLE) _get_osfhandle (fileno (f));
-  FlushFileBuffers (hnd);
+  if (!FlushFileBuffers (hnd))
+    {
+      grub_util_info ("flush err %x", (int) GetLastError ());
+      return -1;
+    }
+  return 0;
 }
 
 int

diff --git a/include/grub/emu/hostfile.h b/include/grub/emu/hostfile.h
index 8e37d5acb42b18c71de5d377dc1ab69cd75e1586..cfb1e2b56612fb666b9e8e2b5b28cf879d5d3bfb 100644 (file)
--- a/include/grub/emu/hostfile.h
+++ b/include/grub/emu/hostfile.h
@@ -47,11 +47,11 @@ grub_util_fd_t
 EXPORT_FUNC(grub_util_fd_open) (const char *os_dev, int flags);
 const char *
 EXPORT_FUNC(grub_util_fd_strerror) (void);
-void
+int
 grub_util_fd_sync (grub_util_fd_t fd);
 void
 grub_util_disable_fd_syncs (void);
-void
+int
 EXPORT_FUNC(grub_util_fd_close) (grub_util_fd_t fd);
 
 grub_uint64_t

diff --git a/include/grub/emu/misc.h b/include/grub/emu/misc.h
index df6085bcb7c20280612a4b6ef5d0c9ac95e29dd4..59b8b35fcbfc97a68fa31d82d1f047d924ef6090 100644 (file)
--- a/include/grub/emu/misc.h
+++ b/include/grub/emu/misc.h
@@ -73,6 +73,6 @@ FILE *
 grub_util_fopen (const char *path, const char *mode);
 #endif
 
-void grub_util_file_sync (FILE *f);
+int grub_util_file_sync (FILE *f);
 
 #endif /* GRUB_EMU_MISC_H */

diff --git a/util/editenv.c b/util/editenv.c
index c6f8d2298c37fbe6735071ac816ea11cdc9992ec..eb2d0c03a98f15bda3b3e127b6613261702c5ca8 100644 (file)
--- a/util/editenv.c
+++ b/util/editenv.c
@@ -55,7 +55,8 @@ grub_util_create_envblk_file (const char *name)
                     strerror (errno));
 
 
-  grub_util_file_sync (fp);
+  if (grub_util_file_sync (fp) < 0)
+    grub_util_error (_("cannot sync `%s': %s"), namenew, strerror (errno));
   free (buf);
   fclose (fp);
 

diff --git a/util/grub-editenv.c b/util/grub-editenv.c
index 118e89fe57fe9ba76b67cfe63d52e94950244777..f3662c95ba66c25662a6827dbfd8bc468c1d07b4 100644 (file)
--- a/util/grub-editenv.c
+++ b/util/grub-editenv.c
@@ -197,7 +197,8 @@ write_envblk (const char *name, grub_envblk_t envblk)
     grub_util_error (_("cannot write to `%s': %s"), name,
                     strerror (errno));
 
-  grub_util_file_sync (fp);
+  if (grub_util_file_sync (fp) < 0)
+    grub_util_error (_("cannot sync `%s': %s"), name, strerror (errno));
   fclose (fp);
 }
 

diff --git a/util/grub-install-common.c b/util/grub-install-common.c
index 0a7d68b9bc7b4dd14adfdefae16fc12d4c680e48..ca0ac612ac510183f133285601d4bcd26e5973fb 100644 (file)
--- a/util/grub-install-common.c
+++ b/util/grub-install-common.c
@@ -112,11 +112,16 @@ grub_install_copy_file (const char *src,
       r = grub_util_fd_read (in, grub_install_copy_buffer, GRUB_INSTALL_COPY_BUFFER_SIZE);
       if (r <= 0)
        break;
-      grub_util_fd_write (out, grub_install_copy_buffer, r);
+      r = grub_util_fd_write (out, grub_install_copy_buffer, r);
+      if (r <= 0)
+       break;
     }
-  grub_util_fd_sync (out);
-  grub_util_fd_close (in);
-  grub_util_fd_close (out);
+  if (grub_util_fd_sync (out) < 0)
+    r = -1;
+  if (grub_util_fd_close (in) < 0)
+    r = -1;
+  if (grub_util_fd_close (out) < 0)
+    r = -1;
 
   if (r < 0)
     grub_util_error (_("cannot copy `%s' to `%s': %s"),
@@ -526,7 +531,8 @@ grub_install_make_image_wrap (const char *dir, const char *prefix,
   grub_install_make_image_wrap_file (dir, prefix, fp, outname,
                                     memdisk_path, config_path,
                                     mkimage_target, note);
-  grub_util_file_sync (fp);
+  if (grub_util_file_sync (fp) < 0)
+    grub_util_error (_("cannot sync `%s': %s"), outname, strerror (errno));
   fclose (fp);
 }
 

diff --git a/util/grub-mkimage.c b/util/grub-mkimage.c
index 98d24cc06ea57b88a4f02f38f177f44ec4f38f3f..912564e3622f782da00f4721353f4d486b18f296 100644 (file)
--- a/util/grub-mkimage.c
+++ b/util/grub-mkimage.c
@@ -311,8 +311,12 @@ main (int argc, char *argv[])
                               arguments.image_target, arguments.note,
                               arguments.comp, arguments.dtb);
 
-  grub_util_file_sync  (fp);
-  fclose (fp);
+  if (grub_util_file_sync (fp) < 0)
+    grub_util_error (_("cannot sync `%s': %s"), arguments.output ? : "stdout",
+                    strerror (errno));
+  if (fclose (fp) == EOF)
+    grub_util_error (_("cannot close `%s': %s"), arguments.output ? : "stdout",
+                    strerror (errno));
 
   for (i = 0; i < arguments.nmodules; i++)
     free (arguments.modules[i]);

diff --git a/util/setup.c b/util/setup.c
index 9c1e1b7da6a85ea7aece6433f2eaf81a6ccde451..8954dd479893bc2d53a2ded7d9356f58fe6b9b7c 100644 (file)
--- a/util/setup.c
+++ b/util/setup.c
@@ -728,8 +728,10 @@ unable_to_embed:
       != GRUB_DISK_SECTOR_SIZE * 2)
     grub_util_error (_("cannot write to `%s': %s"),
                     core_path, strerror (errno));
-  grub_util_fd_sync (fp);
-  grub_util_fd_close (fp);
+  if (grub_util_fd_sync (fp) < 0)
+    grub_util_error (_("cannot sync `%s': %s"), core_path, strerror (errno));
+  if (grub_util_fd_close (fp) < 0)
+    grub_util_error (_("cannot close `%s': %s"), core_path, strerror (errno));
   grub_util_biosdisk_flush (root_dev->disk);
 
   grub_disk_cache_invalidate_all ();