Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 6 Oct 2002 01:40:26 +0100
From:      Brian Somers <brian@Awfulhak.org>
To:        Bruce Evans <bde@zeta.org.au>
Cc:        brian@FreeBSD.org, cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org
Subject:   Re: cvs commit: src/sys/kern subr_disk.c
Message-ID:  <20021006014026.63604067.brian@Awfulhak.org>
In-Reply-To: <20021005235134.J12119-100000@gamplex.bde.org>
References:  <200210051124.g95BOMk2092338@freefall.freebsd.org> <20021005235134.J12119-100000@gamplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 6 Oct 2002 00:15:37 +1000 (EST), Bruce Evans <bde@zeta.org.au> wrote:
> On Sat, 5 Oct 2002, Brian Somers wrote:
> 
> > brian       2002/10/05 04:24:22 PDT
> >
> >   Modified files:
> >     sys/kern             subr_disk.c
> >   Log:
> >   If dsgetlabel() returns a label with a size of zero in diskdumpconf(),
> >   treat it as an invalid partition.
> >
> >   This fixes a bug where ``dumpon <device>'' will configure the dump
> >   device at a random offset on the disk if <device> isn't a valid
> >   partition.
> 
> This seems to only unbreak the case where the partition size is 0.  At
> least the i386 dumpsys() has no bounds checking at the partition level.
> It clobbers sectors outside of the partition starting at the non-random
> offset dumplo given by:
> 
> 	dumplo = di->mediaoffset + di->mediasize - Maxmem * (off_t)PAGE_SIZE;
> 	dumplo -= sizeof kdh * 2;
> 
> except in the following cases:
> - if di_mediasize is actually large enough to hold the data
>   (di_mediasize >= Maxmem * (off_t)PAGE_SIZE + sizeof(kdh) * 2 +
>        <space for metadata, e.g., LABELSECTOR sectors>)
> - if di_mediasize is 0 (or just small), then dumplo is negative and the
>   disk driver's or hardware's bounds checking (of physical sector numbers)
>   should prevent problems.
> 
> Bruce

I'm not sure what ``space for metadata'' is for.  The attached patch seems to
get the answer right here on i386, although I can't vouch for ia64 or sparc64
working correctly (their dumpsys() seems different).

Do you think I should commit this ?

-- 
Brian <brian@Awfulhak.org>                        <brian@[uk.]FreeBSD.org>
      <http://www.Awfulhak.org>;                   <brian@[uk.]OpenBSD.org>
Don't _EVER_ lose your sense of humour !

Index: subr_disk.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/subr_disk.c,v
retrieving revision 1.61
diff -u -r1.61 subr_disk.c
--- subr_disk.c	5 Oct 2002 16:35:31 -0000	1.61
+++ subr_disk.c	6 Oct 2002 00:31:43 -0000
@@ -20,6 +20,7 @@
 #include <sys/disk.h>
 #include <sys/diskslice.h>
 #include <sys/disklabel.h>
+#include <sys/kerneldump.h>
 #ifdef NO_GEOM
 #include <sys/kernel.h>
 #include <sys/sysctl.h>
@@ -222,6 +223,9 @@
 	di.mediasize =
 	    (off_t)(dl->d_partitions[dkpart(dev)].p_size) * DEV_BSIZE;
 	if (di.mediasize == 0)
+		return (EINVAL);
+	if (di.mediasize < Maxmem * (off_t)PAGE_SIZE +
+	    sizeof(struct kerneldumpheader) * 2)
 		return (EINVAL);
 	return(set_dumper(&di));
 }

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message




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