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. --- grub-core/osdep/aros/hostdisk.c | 38 ++++++++++++++++++------------ grub-core/osdep/unix/hostdisk.c | 18 +++++++------- grub-core/osdep/windows/hostdisk.c | 37 ++++++++++++++++++++++------- include/grub/emu/hostfile.h | 4 ++-- include/grub/emu/misc.h | 2 +- util/editenv.c | 3 ++- util/grub-editenv.c | 3 ++- util/grub-install-common.c | 16 +++++++++---- util/grub-mkimage.c | 8 +++++-- util/setup.c | 6 +++-- 10 files changed, 90 insertions(+), 45 deletions(-) diff --git a/grub-core/osdep/aros/hostdisk.c b/grub-core/osdep/aros/hostdisk.c index 7d99b54b8..2be654ca3 100644 --- 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 5450cf416..91150969b 100644 --- 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 85507af88..355100789 100644 --- 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 8e37d5acb..cfb1e2b56 100644 --- 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 df6085bcb..59b8b35fc 100644 --- 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 c6f8d2298..eb2d0c03a 100644 --- 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 118e89fe5..f3662c95b 100644 --- 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 0a7d68b9b..ca0ac612a 100644 --- 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 98d24cc06..912564e36 100644 --- 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 9c1e1b7da..8954dd479 100644 --- 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 (); -- 2.20.1 _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel