[PATCH] afmongodb: Fix a possible crash during configuration.
During configuration, if the user specified value-pairs(), the driver would crash, because it tried to free a NULL structure. Signed-off-by: Gergely Nagy <algernon@balabit.hu> --- modules/afmongodb/afmongodb.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/modules/afmongodb/afmongodb.c b/modules/afmongodb/afmongodb.c index 68594d7..422058e 100644 --- a/modules/afmongodb/afmongodb.c +++ b/modules/afmongodb/afmongodb.c @@ -148,7 +148,8 @@ afmongodb_dd_set_value_pairs(LogDriver *d, ValuePairs *vp) { MongoDBDestDriver *self = (MongoDBDestDriver *)d; - value_pairs_free (self->vp); + if (self->vp) + value_pairs_free (self->vp); self->vp = vp; } -- 1.7.2.5
If cap_syslog exists, the kernel will complain (once) that we only have cap_sys_admin. Additionally, using cap_syslog instead of cap_sys_admin significantly lowers the unneeded privs we are using. Upon startup, syslog-ng will detect whether CAP_SYSLOG is available, and use capabilities based on that finding. This detection will also have a side-effect, which will make it so that g_process_cap_modify(CAP_SYSLOG) will fall back to CAP_SYS_ADMIN, if CAP_SYSLOG support was not detected. Thanks to Andrew Morgan for pointing out a nice way to detect whether the kernel has CAP_SYSLOG. Original code by Serge Hallyn, with minor changes based on Balazs Scheidler's review by Gergely Nagy. Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> Signed-off-by: Gergely Nagy <algernon@balabit.hu> --- lib/gprocess.c | 24 ++++++++++++++++++++++++ lib/gprocess.h | 5 +++++ modules/affile/affile.c | 2 +- syslog-ng/main.c | 31 +++++++++++++++++++++++-------- 4 files changed, 53 insertions(+), 9 deletions(-) diff --git a/lib/gprocess.c b/lib/gprocess.c index 38bcb12..8876d51 100644 --- a/lib/gprocess.c +++ b/lib/gprocess.c @@ -98,6 +98,7 @@ static gint startup_result_pipe[2] = { -1, -1 }; static gint init_result_pipe[2] = { -1, -1 }; static GProcessKind process_kind = G_PK_STARTUP; static gboolean stderr_present = TRUE; +static int have_capsyslog = FALSE; /* global variables */ static struct @@ -216,6 +217,13 @@ g_process_cap_modify(int capability, int onoff) if (!process_opts.caps) return TRUE; + /* + * if libcap or kernel doesn't support cap_syslog, then resort to + * cap_sys_admin + */ + if (capability == CAP_SYSLOG && (!have_capsyslog || CAP_SYSLOG == -1)) + capability = CAP_SYS_ADMIN; + caps = cap_get_proc(); if (!caps) return FALSE; @@ -885,6 +893,22 @@ g_process_change_dir(void) } +gboolean +g_process_check_cap_syslog(void) +{ + int ret; + + if (CAP_SYSLOG == -1) + return FALSE; + + ret = prctl(PR_CAPBSET_READ, CAP_SYSLOG); + if (ret == -1) + return FALSE; + + have_capsyslog = TRUE; + return TRUE; +} + /** * g_process_send_result: * @ret_num: exit code of the process diff --git a/lib/gprocess.h b/lib/gprocess.h index a6dd7c4..19e1d34 100644 --- a/lib/gprocess.h +++ b/lib/gprocess.h @@ -46,6 +46,10 @@ gboolean g_process_cap_modify(int capability, int onoff); cap_t g_process_cap_save(void); void g_process_cap_restore(cap_t r); +#ifndef CAP_SYSLOG +#define CAP_SYSLOG -1 +#endif + #else typedef gpointer cap_t; @@ -66,6 +70,7 @@ void g_process_set_chroot(const gchar *chroot); void g_process_set_pidfile(const gchar *pidfile); void g_process_set_pidfile_dir(const gchar *pidfile_dir); void g_process_set_working_dir(const gchar *cwd); +gboolean g_process_check_cap_syslog(void); void g_process_set_caps(const gchar *caps); void g_process_set_argv_space(gint argc, gchar **argv); void g_process_set_use_fdlimit(gboolean use); diff --git a/modules/affile/affile.c b/modules/affile/affile.c index 67a4632..6e4f5f0 100644 --- a/modules/affile/affile.c +++ b/modules/affile/affile.c @@ -59,7 +59,7 @@ affile_open_file(gchar *name, gint flags, if (privileged) { g_process_cap_modify(CAP_DAC_READ_SEARCH, TRUE); - g_process_cap_modify(CAP_SYS_ADMIN, TRUE); + g_process_cap_modify(CAP_SYSLOG, TRUE); } else { diff --git a/syslog-ng/main.c b/syslog-ng/main.c index 7e81f2f..fa06492 100644 --- a/syslog-ng/main.c +++ b/syslog-ng/main.c @@ -155,6 +155,26 @@ version(void) } +#define BASE_CAPS "cap_net_bind_service,cap_net_broadcast,cap_net_raw," \ + "cap_dac_read_search,cap_dac_override,cap_chown,cap_fowner=p " + +static void +setup_caps (gboolean with_cap_syslog) +{ + static gchar *capsstr_syslog = BASE_CAPS "cap_syslog=ep"; + static gchar *capsstr_sys_admin = BASE_CAPS "cap_sys_admin=ep"; + + /* Set up the minimal privilege we'll need + * + * NOTE: polling /proc/kmsg requires cap_sys_admin, otherwise it'll always + * indicate readability. Enabling/disabling cap_sys_admin on every poll + * invocation seems to be too expensive. So I enable it for now. + */ + if (with_cap_syslog) + g_process_set_caps(capsstr_syslog); + else + g_process_set_caps(capsstr_sys_admin); +} int main(int argc, char *argv[]) @@ -166,14 +186,9 @@ main(int argc, char *argv[]) z_mem_trace_init("syslog-ng.trace"); g_process_set_argv_space(argc, (gchar **) argv); - - /* NOTE: polling /proc/kmsg requires cap_sys_admin, otherwise it'll always - * indicate readability. Enabling/disabling cap_sys_admin on every poll - * invocation seems to be too expensive. So I enable it for now. */ - - g_process_set_caps("cap_net_bind_service,cap_net_broadcast,cap_net_raw," - "cap_dac_read_search,cap_dac_override,cap_chown,cap_fowner=p " - "cap_sys_admin=ep"); + + setup_caps(g_process_check_cap_syslog ()); + ctx = g_option_context_new("syslog-ng"); g_process_add_option_group(ctx); msg_add_option_group(ctx); -- 1.7.2.5
On Sun, 2011-05-08 at 12:26 +0200, Gergely Nagy wrote:
If cap_syslog exists, the kernel will complain (once) that we only have cap_sys_admin. Additionally, using cap_syslog instead of cap_sys_admin significantly lowers the unneeded privs we are using.
Upon startup, syslog-ng will detect whether CAP_SYSLOG is available, and use capabilities based on that finding. This detection will also have a side-effect, which will make it so that g_process_cap_modify(CAP_SYSLOG) will fall back to CAP_SYS_ADMIN, if CAP_SYSLOG support was not detected.
Thanks to Andrew Morgan for pointing out a nice way to detect whether the kernel has CAP_SYSLOG. Original code by Serge Hallyn, with minor changes based on Balazs Scheidler's review by Gergely Nagy.
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> Signed-off-by: Gergely Nagy <algernon@balabit.hu> ---
Applied with cosmetic changes (moved the function into the ENABLE_LINUX_CAPS block, the invocation of setup_caps() was changed a bit, so setup_caps() will call g_process_have_cap_syslog() on its own). Thanks -- Bazsi
Hi, Thanks, applied. On Sun, 2011-05-08 at 10:39 +0200, Gergely Nagy wrote:
During configuration, if the user specified value-pairs(), the driver would crash, because it tried to free a NULL structure.
Signed-off-by: Gergely Nagy <algernon@balabit.hu> --- modules/afmongodb/afmongodb.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
-- Bazsi
participants (2)
-
Balazs Scheidler
-
Gergely Nagy