From 6bd9731c7c3f265f4b0ee06ac8c6dfed4a039719 Mon Sep 17 00:00:00 2001 From: Christoph Wurm Date: Fri, 8 Feb 2019 14:48:33 +0000 Subject: [PATCH 1/7] Disable librpm signal traps. --- .../module/system/package/rpm_linux.go | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/x-pack/auditbeat/module/system/package/rpm_linux.go b/x-pack/auditbeat/module/system/package/rpm_linux.go index a1d395f54b1f..b7bdb5305e6f 100644 --- a/x-pack/auditbeat/module/system/package/rpm_linux.go +++ b/x-pack/auditbeat/module/system/package/rpm_linux.go @@ -22,6 +22,7 @@ import ( #include #include #include +#include rpmts my_rpmtsCreate(void *f) { @@ -100,7 +101,31 @@ my_rpmtsFree(void *f, rpmts ts) { rpmtsFree = (rpmts (*)(rpmts))f; rpmtsFree(ts); -}*/ +} + +// By default, librpm is going to trap SIGINT and SIGTERM, +// which will prevent Beats from shutting down correctly. +// +// This disables that behavior. We should be very dilligent in +// cleaning up in our use of librpm. +// +// More recent versions of librpm have a new function rpmsqSetInterruptSafety() +// to do this. +// +// See also: +// - librpm traps signals and calls exit(1) to terminate the whole process incl. our Go code: https://github.com/rpm-software-management/rpm/blob/rpm-4.11.3-release/lib/rpmdb.c#L640 +// - has caused problems for gdb before, calling rpmsqEnable(_, NULL) is the workaround they also use: https://bugzilla.redhat.com/show_bug.cgi?id=643031 +// - the new rpmsqSetInterruptSafety(), unfortunately only available in librpm>=4.14.2.1 (CentOS 7 has 4.11.3): https://github.com/rpm-software-management/rpm/commit/56f49d7f5af7c1c8a3eb478431356195adbfdd25 +void +my_disableLibrpmSignalTraps(void *f) { + int (*rpmsqEnable)(int, rpmsqAction_t); + rpmsqEnable = (int (*)(int, rpmsqAction_t))f; + + // Disable traps + rpmsqEnable(-SIGTERM, NULL); + rpmsqEnable(-SIGINT, NULL); +} +*/ import "C" // Constants in sync with /usr/include/rpm/rpmtag.h @@ -126,6 +151,7 @@ type cFunctions struct { headerFree unsafe.Pointer rpmdbFreeIterator unsafe.Pointer rpmtsFree unsafe.Pointer + rpmsqEnable unsafe.Pointer } var cFun *cFunctions @@ -141,6 +167,11 @@ func dlopenCFunctions() (*cFunctions, error) { return nil, err } + cFun.rpmsqEnable, err = librpm.GetSymbolPointer("rpmsqEnable") + if err != nil { + return nil, err + } + cFun.rpmtsCreate, err = librpm.GetSymbolPointer("rpmtsCreate") if err != nil { return nil, err @@ -212,6 +243,7 @@ func listRPMPackages() ([]*Package, error) { if mi == nil { return nil, fmt.Errorf("Failed to get match iterator") } + C.my_disableLibrpmSignalTraps(cFun.rpmsqEnable) defer C.my_rpmdbFreeIterator(cFun.rpmdbFreeIterator, mi) var packages []*Package From 1ac0f2ce64f2c3e816a44f997a7d97bded92d26f Mon Sep 17 00:00:00 2001 From: Christoph Wurm Date: Sat, 9 Feb 2019 11:50:04 +0000 Subject: [PATCH 2/7] Use rpmsqSetInterruptSafety() when possible. --- .../module/system/package/rpm_linux.go | 47 ++++++++++++++----- 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/x-pack/auditbeat/module/system/package/rpm_linux.go b/x-pack/auditbeat/module/system/package/rpm_linux.go index b7bdb5305e6f..f870932ce49f 100644 --- a/x-pack/auditbeat/module/system/package/rpm_linux.go +++ b/x-pack/auditbeat/module/system/package/rpm_linux.go @@ -12,6 +12,7 @@ import ( "unsafe" "github.com/coreos/pkg/dlopen" + "github.com/joeshaw/multierror" ) /* @@ -103,27 +104,38 @@ my_rpmtsFree(void *f, rpmts ts) { rpmtsFree(ts); } -// By default, librpm is going to trap SIGINT and SIGTERM, +// By default, librpm is going to trap various UNIX signals including SIGINT and SIGTERM // which will prevent Beats from shutting down correctly. // // This disables that behavior. We should be very dilligent in // cleaning up in our use of librpm. // // More recent versions of librpm have a new function rpmsqSetInterruptSafety() -// to do this. +// to do this, see below. // // See also: // - librpm traps signals and calls exit(1) to terminate the whole process incl. our Go code: https://github.com/rpm-software-management/rpm/blob/rpm-4.11.3-release/lib/rpmdb.c#L640 // - has caused problems for gdb before, calling rpmsqEnable(_, NULL) is the workaround they also use: https://bugzilla.redhat.com/show_bug.cgi?id=643031 -// - the new rpmsqSetInterruptSafety(), unfortunately only available in librpm>=4.14.2.1 (CentOS 7 has 4.11.3): https://github.com/rpm-software-management/rpm/commit/56f49d7f5af7c1c8a3eb478431356195adbfdd25 +// - the new rpmsqSetInterruptSafety(), unfortunately only available in librpm>=4.14.0 (CentOS 7 has 4.11.3): https://github.com/rpm-software-management/rpm/commit/56f49d7f5af7c1c8a3eb478431356195adbfdd25 void my_disableLibrpmSignalTraps(void *f) { int (*rpmsqEnable)(int, rpmsqAction_t); rpmsqEnable = (int (*)(int, rpmsqAction_t))f; - // Disable traps - rpmsqEnable(-SIGTERM, NULL); + // Disable all traps + rpmsqEnable(-SIGHUP, NULL); rpmsqEnable(-SIGINT, NULL); + rpmsqEnable(-SIGTERM, NULL); + rpmsqEnable(-SIGQUIT, NULL); + rpmsqEnable(-SIGPIPE, NULL); +} + +void +my_rpmsqSetInterruptSafety(void *f, int on) { + void (*rpmsqSetInterruptSafety)(int); + rpmsqSetInterruptSafety = (void (*)(int))f; + + rpmsqSetInterruptSafety(on); } */ import "C" @@ -167,11 +179,6 @@ func dlopenCFunctions() (*cFunctions, error) { return nil, err } - cFun.rpmsqEnable, err = librpm.GetSymbolPointer("rpmsqEnable") - if err != nil { - return nil, err - } - cFun.rpmtsCreate, err = librpm.GetSymbolPointer("rpmtsCreate") if err != nil { return nil, err @@ -217,6 +224,22 @@ func dlopenCFunctions() (*cFunctions, error) { return nil, err } + // Only available in librpm>=4.13.0 + rpmsqSetInterruptSafety, err := librpm.GetSymbolPointer("rpmsqSetInterruptSafety") + if err != nil { + var err2 error + // Only available in librpm<4.14.0 + cFun.rpmsqEnable, err2 = librpm.GetSymbolPointer("rpmsqEnable") + if err2 != nil { + var errs multierror.Errors + errs = append(errs, err) + errs = append(errs, err2) + return nil, errs.Err() + } + } else { + C.my_rpmsqSetInterruptSafety(rpmsqSetInterruptSafety, 1) + } + return &cFun, nil } @@ -243,7 +266,9 @@ func listRPMPackages() ([]*Package, error) { if mi == nil { return nil, fmt.Errorf("Failed to get match iterator") } - C.my_disableLibrpmSignalTraps(cFun.rpmsqEnable) + if cFun.rpmsqEnable != nil { + C.my_disableLibrpmSignalTraps(cFun.rpmsqEnable) + } defer C.my_rpmdbFreeIterator(cFun.rpmdbFreeIterator, mi) var packages []*Package From 66b9de82092eff43681a998f47b17c4015750f4c Mon Sep 17 00:00:00 2001 From: Christoph Wurm Date: Sat, 9 Feb 2019 11:56:51 +0000 Subject: [PATCH 3/7] Re-enable package system test. --- x-pack/auditbeat/tests/system/test_metricsets.py | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/auditbeat/tests/system/test_metricsets.py b/x-pack/auditbeat/tests/system/test_metricsets.py index 965f7ee77c7c..47b54b2f49d9 100644 --- a/x-pack/auditbeat/tests/system/test_metricsets.py +++ b/x-pack/auditbeat/tests/system/test_metricsets.py @@ -41,7 +41,6 @@ def test_metricset_login(self): # Metricset is experimental and that generates a warning, TODO: remove later self.check_metricset("system", "login", COMMON_FIELDS + fields, config, warnings_allowed=True) - @unittest.skip("Flaky test, see https://github.com/elastic/beats/issues/10633") @unittest.skipIf(sys.platform == "win32", "Not implemented for Windows") @unittest.skipIf(sys.platform == "linux2" and not (os.path.isdir("/var/lib/dpkg") or os.path.isdir("/var/lib/rpm")), "Only implemented for dpkg and rpm") From 8a62562e53484903a2897e1e49c6bcb1c3150d78 Mon Sep 17 00:00:00 2001 From: Christoph Wurm Date: Tue, 12 Feb 2019 12:56:05 +0000 Subject: [PATCH 4/7] Small improvement. --- x-pack/auditbeat/module/system/package/rpm_linux.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x-pack/auditbeat/module/system/package/rpm_linux.go b/x-pack/auditbeat/module/system/package/rpm_linux.go index f870932ce49f..3cb735a32446 100644 --- a/x-pack/auditbeat/module/system/package/rpm_linux.go +++ b/x-pack/auditbeat/module/system/package/rpm_linux.go @@ -232,8 +232,7 @@ func dlopenCFunctions() (*cFunctions, error) { cFun.rpmsqEnable, err2 = librpm.GetSymbolPointer("rpmsqEnable") if err2 != nil { var errs multierror.Errors - errs = append(errs, err) - errs = append(errs, err2) + errs = append(errs, err, err2) return nil, errs.Err() } } else { From 77a855b8ec73ed568ae2d067e8314b034325b6ae Mon Sep 17 00:00:00 2001 From: Christoph Wurm Date: Tue, 12 Feb 2019 13:06:54 +0000 Subject: [PATCH 5/7] Changelog --- CHANGELOG.next.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 8d3a4998af95..2711aa5026b7 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -172,6 +172,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d *Auditbeat* - Enable System module config on Windows. {pull}10237[10237] +- Package: Disable librpm signal handlers. {pull}10694[10694] *Filebeat* From ae5006ead596d651dc0b8e36cb7b67e0e9551e72 Mon Sep 17 00:00:00 2001 From: Christoph Wurm Date: Thu, 14 Feb 2019 14:20:16 +0000 Subject: [PATCH 6/7] Disable, not enable, interrupt safety. --- x-pack/auditbeat/module/system/package/rpm_linux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/auditbeat/module/system/package/rpm_linux.go b/x-pack/auditbeat/module/system/package/rpm_linux.go index 3cb735a32446..704a0edac018 100644 --- a/x-pack/auditbeat/module/system/package/rpm_linux.go +++ b/x-pack/auditbeat/module/system/package/rpm_linux.go @@ -236,7 +236,7 @@ func dlopenCFunctions() (*cFunctions, error) { return nil, errs.Err() } } else { - C.my_rpmsqSetInterruptSafety(rpmsqSetInterruptSafety, 1) + C.my_rpmsqSetInterruptSafety(rpmsqSetInterruptSafety, 0) } return &cFun, nil From 58620647d63dfee854423ed4ec2b3f1ecbee82c3 Mon Sep 17 00:00:00 2001 From: Christoph Wurm Date: Sat, 16 Feb 2019 16:53:09 +0000 Subject: [PATCH 7/7] Handle librpm's thread-local storage. --- .../module/system/package/rpm_linux.go | 38 ++++++++++++------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/x-pack/auditbeat/module/system/package/rpm_linux.go b/x-pack/auditbeat/module/system/package/rpm_linux.go index 704a0edac018..292db1d5ed6f 100644 --- a/x-pack/auditbeat/module/system/package/rpm_linux.go +++ b/x-pack/auditbeat/module/system/package/rpm_linux.go @@ -8,6 +8,7 @@ package pkg import ( "fmt" + "runtime" "time" "unsafe" @@ -154,16 +155,17 @@ const ( ) type cFunctions struct { - rpmtsCreate unsafe.Pointer - rpmReadConfigFiles unsafe.Pointer - rpmtsInitIterator unsafe.Pointer - rpmdbNextIterator unsafe.Pointer - headerLink unsafe.Pointer - headerGetEntry unsafe.Pointer - headerFree unsafe.Pointer - rpmdbFreeIterator unsafe.Pointer - rpmtsFree unsafe.Pointer - rpmsqEnable unsafe.Pointer + rpmtsCreate unsafe.Pointer + rpmReadConfigFiles unsafe.Pointer + rpmtsInitIterator unsafe.Pointer + rpmdbNextIterator unsafe.Pointer + headerLink unsafe.Pointer + headerGetEntry unsafe.Pointer + headerFree unsafe.Pointer + rpmdbFreeIterator unsafe.Pointer + rpmtsFree unsafe.Pointer + rpmsqEnable unsafe.Pointer + rpmsqSetInterruptSafety unsafe.Pointer } var cFun *cFunctions @@ -225,7 +227,7 @@ func dlopenCFunctions() (*cFunctions, error) { } // Only available in librpm>=4.13.0 - rpmsqSetInterruptSafety, err := librpm.GetSymbolPointer("rpmsqSetInterruptSafety") + cFun.rpmsqSetInterruptSafety, err = librpm.GetSymbolPointer("rpmsqSetInterruptSafety") if err != nil { var err2 error // Only available in librpm<4.14.0 @@ -235,14 +237,20 @@ func dlopenCFunctions() (*cFunctions, error) { errs = append(errs, err, err2) return nil, errs.Err() } - } else { - C.my_rpmsqSetInterruptSafety(rpmsqSetInterruptSafety, 0) } return &cFun, nil } func listRPMPackages() ([]*Package, error) { + // In newer versions, librpm is using the thread-local variable + // `disableInterruptSafety` in rpmio/rpmsq.c to disable signal + // traps. To make sure our settings remain in effect throughout + // our function calls we have to lock the OS thread here, since + // Golang can otherwise use any thread it likes for each C.* call. + runtime.LockOSThread() + defer runtime.UnlockOSThread() + if cFun == nil { var err error cFun, err = dlopenCFunctions() @@ -251,6 +259,10 @@ func listRPMPackages() ([]*Package, error) { } } + if cFun.rpmsqSetInterruptSafety != nil { + C.my_rpmsqSetInterruptSafety(cFun.rpmsqSetInterruptSafety, 0) + } + rpmts := C.my_rpmtsCreate(cFun.rpmtsCreate) if rpmts == nil { return nil, fmt.Errorf("Failed to get rpmts")