[PATCH] Finish the task 'Validate coredump format string'

[Mateusz Guzik]

On Sun, Mar 22, 2015 at 05:19:40PM +0800, Tiwei Bie wrote:
> Sorry, I introduced a bug... allproc_lock could not be used to protect
> the access to corefilename[].
> 

First off I committed the code, so the fault is on me.

> Because, sysctl_kern_corefile() could be called very early:
> 
[..]
> That is to say, when the tunable `kern.corefile' is set in loader.conf,
> sysctl_kern_corefile() will be called as the priority of (SI_SUB_KMEM,
> SI_ORDER_FIRST).
> 
> At this time, allproc_lock is not initialized.
> 
> I couldn't find a proper existing lock for this task. Maybe a dedicated
> lock needs to be created. And initialize it together with sysctlmemlock:
> 
[..]
> Or maybe sysctlmemlock could be used, which is only acuqired when
> req.oldlen > PAGE_SIZE.
> 
> 

I was somehow convinced that tunables are dealt with other code.

If such sysctl handler is also called for tunables, the kernel should
pass a flag or some other indicator so that the function knows it is
dealing with a tunable and that would avoid locking and thus solve the
problem.

I'm wondering if we should go a little bit further and get rid of
static char corefilename[MAXPATHLEN]

and have a static char *corefilename instead.

A dedicated sysinit func could fetch and validate the tunable, if any.
If no tunable was provided it would alloc memory for the default.

-- 
Mateusz Guzik <mjguzik gmail.com>