Re: minidumps are unsafe on amd64

On Jan 25, 2008 12:47 PM, Peter Jeremy <peterjeremy_at_optushome.com.au> wrote:
> 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.


Uhm, not really. If the caller is violating the conditions expected /
required by the dump routines then it is the caller (i.e. the code
path for creating dumps artificially) that needs to be fixed.



>
> 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.
>