Re: minidumps are unsafe on amd64

On Fri, Jan 25, 2008 at 11:54:22AM -0700, Scott Long wrote:
>Ruslan Ermilov wrote:
>> Kernel minidumps on amd64 SMP can write beyond the bounds
>> of the configured dump device causing (as in our case) the
>> file system data following swap partition to be overwritten
>> with the dump contents.
...
>> This only affects 7.x/8.x amd64 SMP systems configured with
>> minidump.  i386 systems aren't affected.
>> 
>
>Is this a case where you are manually triggering a dump on a
>system that is otherwise running fine?

IMO, this is irrelevant.  Over-writing data outside the defined
partition boundaries is unacceptable on a production system.

It would be nice if there were some sanity checks to pick this up.
Somewhere down the chain, one of the lower-level write functions
should verify that each write is contained within
[dumperinfo.mediaoffset .. dumperinfo.mediaoffset+dumperinfo.mediasize)
Ideally this would be inside dumperinfo.dumper() but that function
doesn't currently get passed dumperinfo so this change is too
intrusive for 7.0.  Likewise dumperinfo.dumper() is called in too
many places to reasonably add the code to the callers.  Maybe a
MI wrapper function replacing each of the existing dumperinfo.dumper()
calls would be the least intrusive fix:  Replace each existing
        di->dumper(di->priv, va, pa, offset, len);
with
        dumper_write(di, va, pa, offset, len);
and add the following in (probably) kern/kern_shutdown:
void
dumper_write(struct dumperinfo *di, void *va, vm_offset_t *pa, off_t offset, size_t length)
{
	if (offset >= di->mediaoffset &&
	    offset + size <= di->mediaoffset + di->mediasize)
		di->dumper(di->priv, va, pa, offset, len);
	else
		printf("Attempt to write outside dumpdev boundaries ignored\n");
}

>that's one thing.  If it's a case where you're trying to fix
>something that isn't broken, then I'm very cautious about the
>added complexity that you're proposing.

I'd suggest that, for 7.0-RELEASE, either amd64 minidumps, or manually
triggered amd64 minidumps, needs to be disabled (or hidden behind a
"do you really want to shoot yourself in the foot" check).  This can
be noted in ERRATA and fixed in 7.1.

-- 
Peter Jeremy
Please excuse any delays as the result of my ISP's inability to implement
an MTA that is either RFC2821-compliant or matches their claimed behaviour.