From 605b5690e5c189ae13d35daba0358ae919f04b63 Mon Sep 17 00:00:00 2001 From: krw Date: Mon, 26 Jul 2021 13:05:14 +0000 Subject: [PATCH] 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. --- sbin/fdisk/disk.c | 63 ++++++++++++++++++++++--------- sbin/fdisk/disk.h | 7 ++-- sbin/fdisk/gpt.c | 96 ++++++++++++++++++++--------------------------- sbin/fdisk/mbr.c | 18 +++++---- 4 files changed, 101 insertions(+), 83 deletions(-) diff --git a/sbin/fdisk/disk.c b/sbin/fdisk/disk.c index 645b443b21e..bcc5178d4c2 100644 --- 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 bd2eaeca9ce..06913198d83 100644 --- 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 779b18f5030..b964e647e78 100644 --- 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 * Copyright (c) 2015 Kenneth R Westerback @@ -21,7 +21,7 @@ #include #include -#include +#include #include #include #include @@ -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 4aea7b7261b..eeabdb3dba6 100644 --- 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 #include +#include #include #include #include @@ -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; } -- 2.20.1