Linux kernel: Samba doesn't detect coredump path override by sysctl kernel.core_pattern

[Tim Prouty]

On Feb 26, 2009, at 6:37 AM, Timur I. Bakeyev wrote

> I've looked into the patch and got few comments regarding it.
>
> > + * On FreeBSD the current working directory is ignored when  
> creating a core file
>
> That's not entirely correct statement, default value of  
> kern.corefile is "%N.core", which assumes dumping core in the CWD.  
> So, I guess, at least comment should be rephrased.

Ahh, I was not aware of this.  I'll have to change some of the code to  
fall back to the current working directory in the case where  
kern.corefile[0] != '/'.

> +               if (len > 1024)  {
> +                       goto err_out;
> +               }
>
> I'd use MAXPATHLEN instead of hardcoded 1024 to be more portable.
>
> In general, instead of doing loop to approximate necessary buffer  
> size it could be cheaper to call sysctl*() with the NULL buffer and  
> receive exact size of the MIB value.

It was unclear from the man page whether the *oldplen would actually  
be set to the necessary size if the *oldp buffer was too small or  
NULL.  If this is how the API works, there is still the possibility of  
a race where someone sets the sysctl between the time it is checked  
with the NULL buffer, and the second call, so a loop is still the most  
robust approach.  I'll test the API and check out how it's implemented  
in the kernel to see if I can make this better.

> Another thing that should be kept in mind is the usage of expandable  
> variables. For example, core(5) gives such an example:
>
> kern.corefile="/var/coredumps/%U/%N.core"
>
> Which means that all the core files will go into the /var/coredumps/$ 
> {UID}/ directory. It could be any other combination of the flags, so  
> if we want to show exact path where the core file will be dumped we  
> need to expand those variables first.
>
> Code example for such a routine can be found in /usr/src/sys/kern/ 
> kern_sig.c:expand_name().
>
> In general, I'm not sure does one string on a core dump worth all  
> this hassle :)

Yes, I agree that it's not worth doing the full expansion, and I have  
no plans to do it :).

Thanks for the feedback Timur. I'll work on implementing your  
suggestions.

-Tim