Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 3 Feb 2022 14:38:19 GMT
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: f6e755d1b26b - stable/13 - geom: Handle partial I/O in g_{read,write,delete}_data()
Message-ID:  <202202031438.213EcJ68037122@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=f6e755d1b26bf0ef476cf81749c3c83534f3a9a7

commit f6e755d1b26bf0ef476cf81749c3c83534f3a9a7
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2022-01-20 13:25:27 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2022-02-03 14:34:27 +0000

    geom: Handle partial I/O in g_{read,write,delete}_data()
    
    These routines are used internally by GEOM to dispatch I/O requests to a
    provider, typically for tasting or for updating GEOM class metadata
    blocks.
    
    These routines assumed that partial I/O did not occur without setting
    BIO_ERROR, but this is possible in at least two cases:
    - Some or all of the I/O range is beyond the provider's mediasize.
      In this scenario g_io_check() truncates the bounds of the request
      before it is handed to the target provider.
    - A read from vnode-backed md(4) device returns EOF (the backing vnode
      is allowed to be smaller than the device itself) or partial vnode I/O
      occurs.
    In these scenarios g_read_data() could return a partially uninitialized
    buffer.  Many consumers are not affected by the first case, since the
    offsets used for provider metadata or tasting are relative to the
    provider's mediasize, but in some cases metadata is read at fixed
    offsets, such as when searching for a UFS superblock using the offsets
    defined by SBLOCKSEARCH.
    
    Thus, modify the routines to explicitly check for a non-zero residual
    and return EIO in that case.  Remove a related check from the
    DIOCGDELETE ioctl handler, it is handled within g_delete_data() now.
    
    Reviewed by:    mav, imp, kib
    Reported by:    KMSAN
    Sponsored by:   The FreeBSD Foundation
    
    (cherry picked from commit d91d2b513eb30a226e87f0e52e2f9f232a2e1ca3)
---
 sys/geom/geom_dev.c | 13 -------------
 sys/geom/geom_io.c  |  6 ++++++
 2 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/sys/geom/geom_dev.c b/sys/geom/geom_dev.c
index 9c33ab71e6c8..24ab14e90c1b 100644
--- a/sys/geom/geom_dev.c
+++ b/sys/geom/geom_dev.c
@@ -634,19 +634,6 @@ g_dev_ioctl(struct cdev *dev, u_long cmd, caddr_t data, int fflag, struct thread
 			error = EINVAL;
 			break;
 		}
-		if ((pp->mediasize > 0) && (offset >= pp->mediasize)) {
-			/*
-			 * Catch out-of-bounds requests here. The problem is
-			 * that due to historical GEOM I/O implementation
-			 * peculatities, g_delete_data() would always return
-			 * success for requests starting just the next byte
-			 * after providers media boundary. Condition check on
-			 * non-zero media size, since that condition would
-			 * (most likely) cause ENXIO instead.
-			 */
-			error = EIO;
-			break;
-		}
 		while (length > 0) {
 			chunk = length;
 			if (g_dev_del_max_sectors != 0 &&
diff --git a/sys/geom/geom_io.c b/sys/geom/geom_io.c
index 4134645fe618..9bdcd12bf240 100644
--- a/sys/geom/geom_io.c
+++ b/sys/geom/geom_io.c
@@ -886,6 +886,8 @@ g_read_data(struct g_consumer *cp, off_t offset, off_t length, int *error)
 	bp->bio_data = ptr;
 	g_io_request(bp, cp);
 	errorc = biowait(bp, "gread");
+	if (errorc == 0 && bp->bio_completed != length)
+		errorc = EIO;
 	if (error != NULL)
 		*error = errorc;
 	g_destroy_bio(bp);
@@ -940,6 +942,8 @@ g_write_data(struct g_consumer *cp, off_t offset, void *ptr, off_t length)
 	bp->bio_data = ptr;
 	g_io_request(bp, cp);
 	error = biowait(bp, "gwrite");
+	if (error == 0 && bp->bio_completed != length)
+		error = EIO;
 	g_destroy_bio(bp);
 	return (error);
 }
@@ -971,6 +975,8 @@ g_delete_data(struct g_consumer *cp, off_t offset, off_t length)
 	bp->bio_data = NULL;
 	g_io_request(bp, cp);
 	error = biowait(bp, "gdelete");
+	if (error == 0 && bp->bio_completed != length)
+		error = EIO;
 	g_destroy_bio(bp);
 	return (error);
 }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202202031438.213EcJ68037122>