Report write() and ioctl() errors encountered when writing GPT or MBR to disk.

Consolidate GPT/MBR read()/write() operations into DISK_writesectors() and
DISK_readsectors(), producing clearer logic and consistent handling of i/o and
errors.

Add some DPRINTF() and #ifdef DEBUG sections to allow more detailed error
reporting when desired.

diff --git a/sbin/fdisk/disk.c b/sbin/fdisk/disk.c
index 645b443..bcc5178 100644 (file)
--- a/sbin/fdisk/disk.c
+++ b/sbin/fdisk/disk.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: disk.c,v 1.69 2021/07/22 13:17:59 krw Exp $   */
+/*     $OpenBSD: disk.c,v 1.70 2021/07/26 13:05:14 krw Exp $   */
 
 /*
  * Copyright (c) 1997 Tobias Weingartner
@@ -123,26 +123,40 @@ DISK_printgeometry(const char *units)
  * The caller must free() the returned memory!
  */
 char *
-DISK_readsector(const uint64_t sector)
+DISK_readsectors(const uint64_t sector, const uint32_t count)
 {
        char                    *secbuf;
        ssize_t                  len;
        off_t                    off, where;
-       int                      secsize;
+       size_t                   bytes;
 
-       secsize = dl.d_secsize;
+       where = sector * dl.d_secsize;
+       bytes = count * dl.d_secsize;
 
-       where = sector * secsize;
        off = lseek(disk.dk_fd, where, SEEK_SET);
-       if (off != where)
+       if (off == -1) {
+#ifdef DEBUG
+               warn("lseek(%lld) for read", (int64_t)where);
+#endif
                return NULL;
+       }
 
-       secbuf = calloc(1, secsize);
+       secbuf = calloc(1, bytes);
        if (secbuf == NULL)
                return NULL;
 
-       len = read(disk.dk_fd, secbuf, secsize);
-       if (len == -1 || len != secsize) {
+       len = read(disk.dk_fd, secbuf, bytes);
+       if (len == -1) {
+#ifdef DEBUG
+               warn("read(%zu @ %lld)", bytes, (int64_t)where);
+#endif
+               free(secbuf);
+               return NULL;
+       }
+       if (len != (ssize_t)bytes) {
+#ifdef DEBUG
+               warnx("short read(%zu @ %lld)", bytes, (int64_t)where);
+#endif
                free(secbuf);
                return NULL;
        }
@@ -151,22 +165,35 @@ DISK_readsector(const uint64_t sector)
 }
 
 int
-DISK_writesector(const char *secbuf, const uint64_t sector)
+DISK_writesectors(const char *buf, const uint64_t sector,
+    const uint32_t count)
 {
-       int                     secsize;
        ssize_t                 len;
        off_t                   off, where;
+       size_t                  bytes;
 
-       len = -1;
-       secsize = dl.d_secsize;
+       where = sector * dl.d_secsize;
+       bytes = count * dl.d_secsize;
 
-       where = secsize * sector;
        off = lseek(disk.dk_fd, where, SEEK_SET);
-       if (off == where)
-               len = write(disk.dk_fd, secbuf, secsize);
+       if (off == -1) {
+#ifdef DEBUG
+               warn("lseek(%lld) for write", (int64_t)where);
+#endif
+               return -1;
+       }
 
-       if (len == -1 || len != secsize) {
-               errno = EIO;
+       len = write(disk.dk_fd, buf, bytes);
+       if (len == -1) {
+#ifdef DEBUG
+               warn("write(%zu @ %lld)", bytes, (int64_t)where);
+#endif
+               return -1;
+       }
+       if (len != (ssize_t)bytes) {
+#ifdef DEBUG
+               warnx("short write(%zu @ %lld)", bytes, (int64_t)where);
+#endif
                return -1;
        }
 

diff --git a/sbin/fdisk/disk.h b/sbin/fdisk/disk.h
index bd2eaec..0691319 100644 (file)
--- a/sbin/fdisk/disk.h
+++ b/sbin/fdisk/disk.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: disk.h,v 1.30 2021/07/16 13:26:04 krw Exp $   */
+/*     $OpenBSD: disk.h,v 1.31 2021/07/26 13:05:14 krw Exp $   */
 
 /*
  * Copyright (c) 1997 Tobias Weingartner
@@ -34,8 +34,9 @@ struct disk {
 
 void            DISK_open(const char *, const int);
 void            DISK_printgeometry(const char *);
-char           *DISK_readsector(const uint64_t);
-int             DISK_writesector(const char *, const uint64_t);
+char           *DISK_readsectors(const uint64_t, const uint32_t);
+int             DISK_writesectors(const char *, const uint64_t,
+       const uint32_t);
 
 extern struct disk             disk;
 extern struct disklabel                dl;

diff --git a/sbin/fdisk/gpt.c b/sbin/fdisk/gpt.c
index 779b18f..b964e64 100644 (file)
--- a/sbin/fdisk/gpt.c
+++ b/sbin/fdisk/gpt.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: gpt.c,v 1.47 2021/07/21 12:22:54 krw Exp $    */
+/*     $OpenBSD: gpt.c,v 1.48 2021/07/26 13:05:14 krw Exp $    */
 /*
  * Copyright (c) 2015 Markus Muller <mmu@grummel.net>
  * Copyright (c) 2015 Kenneth R Westerback <krw@openbsd.org>
@@ -21,7 +21,7 @@
 #include <sys/dkio.h>
 #include <sys/ioctl.h>
 
-#include <errno.h>
+#include <err.h>
 #include <stdio.h>
 #include <stdint.h>
 #include <stdlib.h>
@@ -118,7 +118,7 @@ get_header(const uint64_t sector)
        int                      partspersec;
        uint32_t                 orig_gh_csum, new_gh_csum;
 
-       secbuf = DISK_readsector(sector);
+       secbuf = DISK_readsectors(sector, 1);
        if (secbuf == NULL)
                return -1;
 
@@ -218,10 +218,9 @@ get_header(const uint64_t sector)
 int
 get_partition_table(void)
 {
-       ssize_t                 len;
-       off_t                   off, where;
-       int                     secs;
-       uint32_t                checksum, partspersec;
+       char                    *secbuf;
+       uint64_t                 gpbytes, gpsectors;
+       uint32_t                 checksum, partspersec;
 
        DPRINTF("gpt partition table being read from LBA %llu\n",
            letoh64(gh.gh_part_lba));
@@ -232,25 +231,18 @@ get_partition_table(void)
                    letoh32(gh.gh_part_size));
                return -1;
        }
-       secs = (letoh32(gh.gh_part_num) + partspersec - 1) / partspersec;
-
+       gpbytes = letoh32(gh.gh_part_num) * letoh32(gh.gh_part_size);
+       gpsectors = gpbytes / dl.d_secsize;
        memset(&gp, 0, sizeof(gp));
 
-       where = letoh64(gh.gh_part_lba) * dl.d_secsize;
-       off = lseek(disk.dk_fd, where, SEEK_SET);
-       if (off == -1) {
-               DPRINTF("seek to gpt partition table @ sector %llu failed\n",
-                   (unsigned long long)where / dl.d_secsize);
-               return -1;
-       }
-       len = read(disk.dk_fd, &gp, secs * dl.d_secsize);
-       if (len == -1 || len != secs * dl.d_secsize) {
-               DPRINTF("gpt partition table read failed.\n");
+       secbuf = DISK_readsectors(letoh64(gh.gh_part_lba), gpsectors);
+       if (secbuf == NULL)
                return -1;
-       }
 
-       checksum = crc32((unsigned char *)&gp, letoh32(gh.gh_part_num) *
-           letoh32(gh.gh_part_size));
+       memcpy(&gp, secbuf, gpbytes);
+       free(secbuf);
+
+       checksum = crc32((unsigned char *)&gp, gpbytes);
        if (checksum != letoh32(gh.gh_part_csum)) {
                DPRINTF("gpt partition table checksum: expected %x, got %x\n",
                    checksum, letoh32(gh.gh_part_csum));
@@ -542,25 +534,29 @@ GPT_zap_headers(void)
        char                    *secbuf;
        uint64_t                 sig;
 
-       secbuf = DISK_readsector(GPTSECTOR);
+       secbuf = DISK_readsectors(GPTSECTOR, 1);
        if (secbuf == NULL)
                return;
 
        memcpy(&sig, secbuf, sizeof(sig));
        if (letoh64(sig) == GPTSIGNATURE) {
                memset(secbuf, 0, dl.d_secsize);
-               DISK_writesector(secbuf, GPTSECTOR);
+               if (DISK_writesectors(secbuf, GPTSECTOR, 1))
+                       DPRINTF("Unable to zap GPT header @ sector %d",
+                           GPTSECTOR);
        }
        free(secbuf);
 
-       secbuf = DISK_readsector(DL_GETDSIZE(&dl) - 1);
+       secbuf = DISK_readsectors(DL_GETDSIZE(&dl) - 1, 1);
        if (secbuf == NULL)
                return;
 
        memcpy(&sig, secbuf, sizeof(sig));
        if (letoh64(sig) == GPTSIGNATURE) {
                memset(secbuf, 0, dl.d_secsize);
-               DISK_writesector(secbuf, DL_GETDSIZE(&dl) - 1);
+               if (DISK_writesectors(secbuf, DL_GETDSIZE(&dl) - 1, 1))
+                       DPRINTF("Unable to zap GPT header @ sector %llu",
+                           DL_GETDSIZE(&dl) - 1);
        }
        free(secbuf);
 }
@@ -569,22 +565,23 @@ int
 GPT_write(void)
 {
        char                    *secbuf;
-       ssize_t                  len;
-       off_t                    off;
-       const int                secsize = unit_types[SECTORS].ut_conversion;
-       uint64_t                 altgh, altgp, prigh, prigp, gpbytes;
+       uint64_t                 altgh, altgp, prigh, prigp;
+       uint64_t                 gpbytes, gpsectors;
+       int                      rslt;
 
-       MBR_write(&gmbr);
+       if (MBR_write(&gmbr))
+               return -1;
 
        /*
         * XXX Assume size of gp is multiple of sector size.
         */
        gpbytes = letoh32(gh.gh_part_num) * letoh32(gh.gh_part_size);
+       gpsectors = gpbytes / dl.d_secsize;
 
        prigh = GPTSECTOR;
        prigp = prigh + 1;
        altgh = DL_GETDSIZE(&dl) - 1;
-       altgp = DL_GETDSIZE(&dl) - 1 - (gpbytes / secsize);
+       altgp = altgh - gpsectors;
 
        gh.gh_lba_self = htole64(prigh);
        gh.gh_lba_alt = htole64(altgh);
@@ -593,13 +590,15 @@ GPT_write(void)
        gh.gh_csum = 0;
        gh.gh_csum = crc32((unsigned char *)&gh, letoh32(gh.gh_size));
 
-       secbuf = DISK_readsector(prigh);
+       secbuf = DISK_readsectors(prigh, 1);
        if (secbuf == NULL)
                return -1;
 
        memcpy(secbuf, &gh, sizeof(gh));
-       DISK_writesector(secbuf, prigh);
+       rslt = DISK_writesectors(secbuf, prigh, 1);
        free(secbuf);
+       if (rslt)
+               return -1;
 
        gh.gh_lba_self = htole64(altgh);
        gh.gh_lba_alt = htole64(prigh);
@@ -607,37 +606,24 @@ GPT_write(void)
        gh.gh_csum = 0;
        gh.gh_csum = crc32((unsigned char *)&gh, letoh32(gh.gh_size));
 
-       secbuf = DISK_readsector(altgh);
+       secbuf = DISK_readsectors(altgh, 1);
        if (secbuf == NULL)
                return -1;
 
        memcpy(secbuf, &gh, sizeof(gh));
-       DISK_writesector(secbuf, altgh);
+       rslt = DISK_writesectors(secbuf, altgh, 1);
        free(secbuf);
-
-       off = lseek(disk.dk_fd, secsize * prigp, SEEK_SET);
-       if (off == secsize * prigp)
-               len = write(disk.dk_fd, &gp, gpbytes);
-       else
-               len = -1;
-       if (len == -1 || len != gpbytes) {
-               errno = EIO;
+       if (rslt)
                return -1;
-       }
 
-       off = lseek(disk.dk_fd, secsize * altgp, SEEK_SET);
-       if (off == secsize * altgp)
-               len = write(disk.dk_fd, &gp, gpbytes);
-       else
-               len = -1;
-
-       if (len == -1 || len != gpbytes) {
-               errno = EIO;
+       if (DISK_writesectors((const char *)&gp, prigp, gpsectors))
+               return -1;
+       if (DISK_writesectors((const char *)&gp, altgp, gpsectors))
                return -1;
-       }
 
        /* Refresh in-kernel disklabel from the updated disk information. */
-       ioctl(disk.dk_fd, DIOCRLDINFO, 0);
+       if (ioctl(disk.dk_fd, DIOCRLDINFO, 0) == -1)
+               warn("DIOCRLDINFO");
 
        return 0;
 }

diff --git a/sbin/fdisk/mbr.c b/sbin/fdisk/mbr.c
index 4aea7b7..eeabdb3 100644 (file)
--- a/sbin/fdisk/mbr.c
+++ b/sbin/fdisk/mbr.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: mbr.c,v 1.94 2021/07/21 12:22:54 krw Exp $    */
+/*     $OpenBSD: mbr.c,v 1.95 2021/07/26 13:05:14 krw Exp $    */
 
 /*
  * Copyright (c) 1997 Tobias Weingartner
@@ -21,6 +21,7 @@
 #include <sys/disklabel.h>
 #include <sys/dkio.h>
 
+#include <err.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -170,7 +171,7 @@ MBR_read(const uint64_t lba_self, const uint64_t lba_firstembr, struct mbr *mbr)
        struct dos_mbr           dos_mbr;
        char                    *secbuf;
 
-       secbuf = DISK_readsector(lba_self);
+       secbuf = DISK_readsectors(lba_self, 1);
        if (secbuf == NULL)
                return -1;
 
@@ -187,20 +188,23 @@ MBR_write(const struct mbr *mbr)
 {
        struct dos_mbr           dos_mbr;
        char                    *secbuf;
+       int                      rslt;
 
-       secbuf = DISK_readsector(mbr->mbr_lba_self);
+       secbuf = DISK_readsectors(mbr->mbr_lba_self, 1);
        if (secbuf == NULL)
                return -1;
 
        MBR_make(mbr, &dos_mbr);
        memcpy(secbuf, &dos_mbr, sizeof(dos_mbr));
 
-       DISK_writesector(secbuf, mbr->mbr_lba_self);
+       rslt = DISK_writesectors(secbuf, mbr->mbr_lba_self, 1);
+       free(secbuf);
+       if (rslt)
+               return -1;
 
        /* Refresh in-kernel disklabel from the updated disk information. */
-       ioctl(disk.dk_fd, DIOCRLDINFO, 0);
-
-       free(secbuf);
+       if (ioctl(disk.dk_fd, DIOCRLDINFO, 0) == -1)
+               warn("DIOCRLDINFO");
 
        return 0;
 }