Date: Fri, 19 Oct 2018 08:20:33 -0700 From: Maxim Sobolev <sobomax@freebsd.org> To: freebsd-geom@freebsd.org, Poul-Henning Kamp <phk@phk.freebsd.dk> Cc: secteam@freebsd.org Subject: Off-by-1 error in the g_io_check() Message-ID: <CAH7qZfsVvNzndKMhV%2BBehDQDziSYSZ2suyJzH_-bYAvG%2Bco4qw@mail.gmail.com>
next in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
Hi, I came across a bug that possibly affects all versions of FreeBSD since
dawn of the GEOM. There seems to be off-by-one error in the g_io_check()
allowing requests that just past the boundary of the device to be accepted.
I was particularly looking at generating BIO_DELETE requests in the
userland and noticed that GEOM provider would accept request for the first
sector outside of the device area. The following example illustrates the
issue:
---- test.c ----
#include <sys/ioctl.h>
#include <sys/disk.h>
#include <assert.h>
#include <errno.h>
#include <fcntl.h>
#include <stdlib.h>
int main(int argc, char **argv)
{
int fd;
off_t mediasize, ioarg[2];
u_int secsize;
assert(argc == 2);
fd = open(argv[1], O_RDWR, 0);
assert(fd >= 0);
assert(ioctl(fd, DIOCGMEDIASIZE, &mediasize) == 0);
assert(ioctl(fd, DIOCGSECTORSIZE, &secsize) == 0);
ioarg[0] = mediasize - secsize;
ioarg[1] = secsize;
/* Zero out last sector */
assert(ioctl(fd, DIOCGDELETE, ioarg) == 0);
ioarg[0] += secsize;
/* Zero out last sector + 1 */
assert(ioctl(fd, DIOCGDELETE, ioarg) == -1);
assert(errno == EIO);
exit(0);
}
------------
# cc -o test test.c
# mdconfig -a -t malloc -s 1m
md0
# ./test /dev/md0
Assertion failed: (ioctl(fd, DIOCGDELETE, ioarg) == -1), function main,
file a.c, line 25.
Abort trap
#
Patch to correct this is attached. I have not looked at the code md(4) to
see if it actually results in buffer outside of the allocated area being
zeroed out, but it's totally possible that some providers might do some
weird stuff given a BIO_DELETE request like this. So we are possibly
looking at a mild security issue here (hence CC secteam).
-Max
[-- Attachment #2 --]
diff --git a/sys/geom/geom_io.c b/sys/geom/geom_io.c
index 73895b30f7..1444ecd06a 100644
--- a/sys/geom/geom_io.c
+++ b/sys/geom/geom_io.c
@@ -415,6 +415,8 @@ g_io_check(struct bio *bp)
return (EIO);
if (bp->bio_offset > pp->mediasize)
return (EIO);
+ if (bp->bio_offset == pp->mediasize && bp->bio_length > 0)
+ return (EIO);
/* Truncate requests to the end of providers media. */
excess = bp->bio_offset + bp->bio_length;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAH7qZfsVvNzndKMhV%2BBehDQDziSYSZ2suyJzH_-bYAvG%2Bco4qw>