[PATCH] Fix ignorance of pipe symbol(|) in dumping core under linux
Michael Adam
obnox at samba.org
Wed Nov 16 11:55:49 UTC 2016
Hi Anoop,
This looks like a good finding, thanks for the patch!
Comments inline below:
On 2016-11-16 at 15:06 +0530, Anoop C S wrote:
> Hi all,
>
> Please see the attached patch which enables smbd/nmbd/winbindd to honour pipe symbol (|) if
> specified inside /proc/sys/kernel/core_pattern under linux. Commit message from the patch explains
> this in detail.
>
> Reviews are appreciated.
>
> Thanks,
> Anoop C S.
> From de0e4e0395bf69ecf2e853d1844c89a9718f3a3f Mon Sep 17 00:00:00 2001
> From: Anoop C S <anoopcs at redhat.com>
> Date: Wed, 16 Nov 2016 08:55:13 +0000
> Subject: [PATCH] s3/dump_core: Honour pipe symbol (|) in system-wide
> core_pattern under linux
>
> From man core(5):
>
> "Since kernel 2.6.19, Linux supports an alternate syntax for the
> /proc/sys/kernel/core_pattern file. If the first character of this
> file is a pipe symbol (|), then the remainder of the line is interpreted
> as a user-space program to be executed."
>
> Discarding this symbol would result in misleading logs for smbd/nmbd/winbindd.
> For example even if core_pattern contains '|', smbd logs would suggest the
> following:
>
> ...
> dumping core in /var/log/samba/cores/smbd
> ...
>
> and coredump may or may not get created at that location depending on which
> helper binary is being used for handling coredumps.
>
> Signed-off-by: Anoop C S <anoopcs at redhat.com>
> ---
> source3/lib/dumpcore.c | 32 ++++++++++++++++++++++++--------
> 1 file changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/source3/lib/dumpcore.c b/source3/lib/dumpcore.c
> index c72aa88..c9b3394 100644
> --- a/source3/lib/dumpcore.c
> +++ b/source3/lib/dumpcore.c
> @@ -37,6 +37,7 @@
> #endif
>
> static char *corepath;
> +static bool helper_binary;
Even though I know it's not needed, i would
like to see explicit initialization to "false"
just for clarity.
Also, wouldn't a name like "using_helper_binary"
be more natural for a bool?
Otherwise the patch looks good to me.
(I.e. RB+ barring changes / comments to the above.)
Michael
> /**
> * Build up the default corepath as "<logbase>/cores/<progname>"
> @@ -169,6 +170,12 @@ static char *get_linux_corepath(void)
> /*
> * No absolute path, use the default (cwd)
> */
> + if (result[0] == '|') {
> + /*
> + * Core dump handled by helper binaries
> + */
> + helper_binary = true;
> + }
> TALLOC_FREE(result);
> return NULL;
> }
> @@ -291,16 +298,25 @@ void dump_core_setup(const char *progname, const char *log_file)
> }
>
> if (*corepath != '\0') {
> - /* The chdir might fail if we dump core before we finish
> - * processing the config file.
> + /*
> + * Check whether coredump is handled by helper binaries or not.
> + * If so skip chdir().
> */
> - if (chdir(corepath) != 0) {
> - DEBUG(0, ("unable to change to %s\n", corepath));
> - DEBUGADD(0, ("refusing to dump core\n"));
> - exit(1);
> - }
> + if (!helper_binary) {
> + /* The chdir might fail if we dump core before we finish
> + * processing the config file.
> + */
> + if (chdir(corepath) != 0) {
> + DEBUG(0, ("unable to change to %s\n", corepath));
> + DEBUGADD(0, ("refusing to dump core\n"));
> + exit(1);
> + }
>
> - DEBUG(0,("dumping core in %s\n", corepath));
> + DEBUG(0,("dumping core in %s\n", corepath));
> + } else {
> + DEBUG(0,("coredump is handled by helper binary "
> + "specified at /proc/sys/kernel/core_pattern"));
> + }
> }
>
> umask(~(0700));
> --
> 2.7.4
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 163 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20161116/2ce487a7/signature.sig>
More information about the samba-technical
mailing list