Date: Fri, 22 Jun 2001 05:40:03 -0700 (PDT) From: Bruce Evans <bde@zeta.org.au> To: freebsd-bugs@FreeBSD.org Subject: Re: kern/28164: [PATCH] crashdump can trash disklabel/other partitions Message-ID: <200106221240.f5MCe3r60087@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/28164; it has been noted by GNATS. From: Bruce Evans <bde@zeta.org.au> To: Phil Homewood <pdh@moreton.com.au> Cc: FreeBSD-gnats-submit@FreeBSD.ORG Subject: Re: kern/28164: [PATCH] crashdump can trash disklabel/other partitions Date: Fri, 22 Jun 2001 22:31:24 +1000 (EST) On Fri, 15 Jun 2001, Phil Homewood wrote: > >Description: > Crashdumps can overwrite the last few blocks of the device > immediately before the dumpdev, if the dumpdev is approximately > the same size as physical memory. > > >How-To-Repeat: > Set up a swap device of the same size as physical memory and > force a crashdump (eg, from DDB). This did not repeat it for me :-). > >Fix: > > The following patch works but is probably incorrect (in tests > on this machine, I needed an extra 10 blocks of disk space, and > one page of physical memory requires 8 blocks of disk). > Someone with a better understanding than I have should review > this patch, but it should provide a good starting point. > > --- kern/kern_shutdown.c.orig Mon Jun 11 23:12:10 2001 > +++ kern/kern_shutdown.c Fri Jun 15 14:46:53 2001 > @@ -421,7 +421,7 @@ > /* > * XXX should clean up checking in dumpsys() to be more like this. > */ > - newdumplo = psize - Maxmem * PAGE_SIZE / DEV_BSIZE; > + newdumplo = psize - (Maxmem + 2) * PAGE_SIZE / DEV_BSIZE; > if (newdumplo < 0) > return (ENOSPC); > dumpdev = dev; > --- kern/subr_disk.c.orig Fri Jun 1 02:47:45 2001 > +++ kern/subr_disk.c Fri Jun 15 14:46:53 2001 > @@ -91,7 +91,7 @@ > dl = dsgetlabel(dev, dp->d_slice); > if (!dl) > return (ENXIO); > - *count = (u_long)Maxmem * PAGE_SIZE / dl->d_secsize; > + *count = (u_long)(Maxmem + 2) * PAGE_SIZE / dl->d_secsize; > if (dumplo < 0 || > (dumplo + *count > dl->d_partitions[dkpart(dev)].p_size)) > return (EINVAL); I don't see how these patches can help. The first hunk prevents dumping if the device size (in bytes) is precisely the same as the memory size (according to Maxmem). But dumping will still occur if the device size is 2 pages larger, and then the second hunk almost ensures that any overrun still occurs (since it adjusts the dump size up by the same amount that the first hunk adjusts the dump start down). It also has bad side effects: - it causes 2 nonexistent pages to be dumped. This might cause NMIs or worse. - it causes overflow on machines with 4GB less 2 pages of memory instead of only on machines with 4GB of memory, if Maxmem can reach 4GB. Better original code: *count = (u_long)Maxmem * (PAGE_SIZE / dl->d_secsize); This assumes that PAGE_SIZE is a multiple of dl->d_secsize, but all dump routines already assume this. The patch might help by avoidng rounding bugs in the dump routines (e.g., they might round *count up to a multiple of 128, so it's best to have *count a multiple of 128 to begin with), but I couldn't see any bugs like that. Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-bugs" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200106221240.f5MCe3r60087>