Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 02 Nov 2013 16:19:42 -0700
From:      Colin Percival <cperciva@freebsd.org>
To:        dt71@gmx.com, freebsd-hackers@freebsd.org
Subject:   Re: Automated submission of kernel panic reports
Message-ID:  <5275888E.6010806@freebsd.org>
In-Reply-To: <5271A465.2030206@gmx.com>
References:  <526F8EB3.1040205@freebsd.org> <5271A465.2030206@gmx.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On 10/30/13 17:29, dt71@gmx.com wrote:
> This smells of having a potential to make an admin accidentally transmit
> undesired information, as well as adding some attack surface.

The default behaviour is to send the information to root with instructions to
forward it if there is nothing overly sensitive.  If you can't deliver email
to root securely, you're leaking lots of information already.

I'm not sure what you mean by "adding some attack surface", can you elaborate?

> Without testing, I bet that a reguler user will be able to read the panicmail.N
> file (which will contain the textdump) -- the umask/permissions are not set up
> properly.

Oops, good catch.

> I very much dislike the non-use of double quotes around variable expansions and
> things like that in the shell code.

Is there anywhere in particular you think this is dangerous?

> The return code of /usr/local/bin/pkesh should be handled.

Fixed.

> Place a comment to the location in the code where an admin could put an add-on
> script that can automatically modify the text to be submitted (both automatic
> and confirmed mode).

Given that these panic reports will be parsed by automated tools, I don't want
to encourage people to modify what's submitted -- I'd prefer that people either
accept or do not accept the submission of the data.

> What if the /var/crash/{info,vmcore}.last symlinks were used as a basis for
> selecting the last dump, instead of the current "$((`cat bounds` - 1))"/"$1"
> method?

Good point -- I'm still running 9.2 on my laptop and I hadn't noticed the new
.last symlinks.

> What's wrong with "our" /bin/sh?

I'm not sure, but that comment appears in /usr/sbin/crashinfo:

# XXX: /bin/sh on 7.0+ is broken so we can't simply pipe the commands to
# kgdb via stdin and have to use a temporary file instead.

> If a temporary file is used for kgdb commands
> anyway, would it not be cleaner to use ``-x ${tmpfile}'' instead of input-piping?

That option doesn't seem to exist.

> How about: ${panicmail_sendto} could be "Full Name <e-mail@address>"?

Good idea.

> "# Remove temporary file" is a bit superfluous.

I tend to over-comment.  It's just my style...

> Choose a consistent commenting style: either use periods/fullstops, or don't.

Fixed, thanks.

> I'd personally use ``>'' instead of ``>>'' first in panicmail_gather().

Yes, that was a mistake.

Thanks for the great review!

-- 
Colin Percival
Security Officer Emeritus, FreeBSD | The power to serve
Founder, Tarsnap | www.tarsnap.com | Online backups for the truly paranoid



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