Date: Fri, 25 Jan 2008 13:37:25 -0800 From: "Kip Macy" <kip.macy@gmail.com> To: "Peter Jeremy" <peterjeremy@optushome.com.au> Cc: current@freebsd.org Subject: Re: minidumps are unsafe on amd64 Message-ID: <b1fa29170801251337u6f034f4fxba4f909ac8667cb0@mail.gmail.com> In-Reply-To: <20080125204735.GQ53741@server.vk2pj.dyndns.org> References: <20080125180740.GA1646@team.vega.ru> <479A305E.3020801@samsco.org> <20080125204735.GQ53741@server.vk2pj.dyndns.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Jan 25, 2008 12:47 PM, Peter Jeremy <peterjeremy@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.
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?b1fa29170801251337u6f034f4fxba4f909ac8667cb0>