diff --git a/SOURCES/0012-Fix-sbd-common-don-t-follow-symlinks-outside-dev-for.patch b/SOURCES/0012-Fix-sbd-common-don-t-follow-symlinks-outside-dev-for.patch new file mode 100644 index 0000000..0de1f14 --- /dev/null +++ b/SOURCES/0012-Fix-sbd-common-don-t-follow-symlinks-outside-dev-for.patch @@ -0,0 +1,96 @@ +From 5d52fa8c3c903df4be0e4e954fbca9b3b15285c6 Mon Sep 17 00:00:00 2001 +From: Klaus Wenninger +Date: Fri, 14 Sep 2018 17:51:50 +0200 +Subject: [PATCH] Fix: sbd-common: don't follow symlinks outside /dev for + watchdog + +This makes it easier to define a SELinux-policy that keeps +avc-log clean on /dev traversal triggered by query-watchdog. +--- + src/sbd-common.c | 42 ++++++++++++++++++++++++++++++++++++++---- + 1 file changed, 38 insertions(+), 4 deletions(-) + +diff --git a/src/sbd-common.c b/src/sbd-common.c +index 0ce6478..fcb7a31 100644 +--- a/src/sbd-common.c ++++ b/src/sbd-common.c +@@ -251,7 +251,8 @@ watchdog_close(bool disarm) + #define MAX_WATCHDOGS 64 + #define SYS_CLASS_WATCHDOG "/sys/class/watchdog" + #define SYS_CHAR_DEV_DIR "/sys/dev/char" +-#define WATCHDOG_NODEDIR "/dev" ++#define WATCHDOG_NODEDIR "/dev/" ++#define WATCHDOG_NODEDIR_LEN 5 + + struct watchdog_list_item { + dev_t dev; +@@ -273,7 +274,7 @@ watchdog_populate_list(void) + struct dirent *entry; + char entry_name[280]; + DIR *dp; +- char buf[256] = ""; ++ char buf[280] = ""; + + if (watchdog_list != NULL) { + return; +@@ -313,7 +314,38 @@ watchdog_populate_list(void) + struct stat statbuf; + + snprintf(entry_name, sizeof(entry_name), +- WATCHDOG_NODEDIR "/%s", entry->d_name); ++ WATCHDOG_NODEDIR "%s", entry->d_name); ++ if (entry->d_type == DT_LNK) { ++ int len; ++ ++ /* !realpath(entry_name, buf) unfortunately does a stat on ++ * target so we can't really use it to check if links stay ++ * within /dev without triggering e.g. AVC-logs (with ++ * SELinux policy that just allows stat within /dev). ++ * Without canonicalization that doesn't actually touch the ++ * filesystem easily available introduce some limitations ++ * for simplicity: ++ * - just simple path without '..' ++ * - just one level of symlinks (avoid e.g. loop-checking) ++ */ ++ len = readlink(entry_name, buf, sizeof(buf) - 1); ++ if ((len < 1) || ++ (len > sizeof(buf) - WATCHDOG_NODEDIR_LEN - 1)) { ++ continue; ++ } ++ buf[len] = '\0'; ++ if (buf[0] != '/') { ++ memmove(&buf[WATCHDOG_NODEDIR_LEN], buf, len+1); ++ memcpy(buf, WATCHDOG_NODEDIR, WATCHDOG_NODEDIR_LEN); ++ len += WATCHDOG_NODEDIR_LEN; ++ } ++ if (strstr(buf, "/../") || ++ strncmp(WATCHDOG_NODEDIR, buf, WATCHDOG_NODEDIR_LEN) || ++ lstat(buf, &statbuf) || ++ !S_ISCHR(statbuf.st_mode)) { ++ continue; ++ } ++ } + if(!stat(entry_name, &statbuf) && S_ISCHR(statbuf.st_mode)) { + int i; + +@@ -322,6 +354,7 @@ watchdog_populate_list(void) + int wdfd = watchdog_init_fd(entry_name, -1); + struct watchdog_list_item *wdg = + calloc(1, sizeof(struct watchdog_list_item)); ++ int len; + + wdg->dev = watchdogs[i]; + wdg->dev_node = strdup(entry_name); +@@ -343,7 +376,8 @@ watchdog_populate_list(void) + snprintf(entry_name, sizeof(entry_name), + SYS_CHAR_DEV_DIR "/%d:%d/device/driver", + major(watchdogs[i]), minor(watchdogs[i])); +- if (readlink(entry_name, buf, sizeof(buf)) > 0) { ++ if ((len = readlink(entry_name, buf, sizeof(buf) - 1)) > 0) { ++ buf[len] = '\0'; + wdg->dev_driver = strdup(basename(buf)); + } else if ((wdg->dev_ident) && + (strcmp(wdg->dev_ident, +-- +1.8.3.1 + diff --git a/SOURCES/0013-Refactor-sbd-common-separate-assignment-and-comparis.patch b/SOURCES/0013-Refactor-sbd-common-separate-assignment-and-comparis.patch new file mode 100644 index 0000000..2108e37 --- /dev/null +++ b/SOURCES/0013-Refactor-sbd-common-separate-assignment-and-comparis.patch @@ -0,0 +1,33 @@ +From e13297f45b4c5868800b1d3fc359bfd0723fcc5f Mon Sep 17 00:00:00 2001 +From: Klaus Wenninger +Date: Mon, 17 Sep 2018 23:13:37 +0200 +Subject: [PATCH] Refactor: sbd-common: separate assignment and comparison + +--- + src/sbd-common.c | 7 ++++--- + 1 file changed, 4 insertions(+), 3 deletions(-) + +diff --git a/src/sbd-common.c b/src/sbd-common.c +index fcb7a31..679f946 100644 +--- a/src/sbd-common.c ++++ b/src/sbd-common.c +@@ -376,12 +376,13 @@ watchdog_populate_list(void) + snprintf(entry_name, sizeof(entry_name), + SYS_CHAR_DEV_DIR "/%d:%d/device/driver", + major(watchdogs[i]), minor(watchdogs[i])); +- if ((len = readlink(entry_name, buf, sizeof(buf) - 1)) > 0) { ++ len = readlink(entry_name, buf, sizeof(buf) - 1); ++ if (len > 0) { + buf[len] = '\0'; + wdg->dev_driver = strdup(basename(buf)); + } else if ((wdg->dev_ident) && +- (strcmp(wdg->dev_ident, +- "Software Watchdog") == 0)) { ++ (strcmp(wdg->dev_ident, ++ "Software Watchdog") == 0)) { + wdg->dev_driver = strdup("softdog"); + } + break; +-- +1.8.3.1 + diff --git a/SOURCES/0014-Fix-sbd-common-avoid-statting-potential-links.patch b/SOURCES/0014-Fix-sbd-common-avoid-statting-potential-links.patch new file mode 100644 index 0000000..1e61c36 --- /dev/null +++ b/SOURCES/0014-Fix-sbd-common-avoid-statting-potential-links.patch @@ -0,0 +1,214 @@ +From 5b4c866f7c0b4ef8061e131a1ee0d1c608d35054 Mon Sep 17 00:00:00 2001 +From: Klaus Wenninger +Date: Wed, 19 Sep 2018 16:15:27 +0200 +Subject: [PATCH] Fix: sbd-common: avoid statting potential links + +These potential links might be anything and statting - if just +allowed to stat chr-nodes (e.g. SELinux) - them would lead +to avc-logs in the SELinux case. +--- + src/sbd-common.c | 133 +++++++++++++++++++++++++++++++++++++++---------------- + 1 file changed, 96 insertions(+), 37 deletions(-) + +diff --git a/src/sbd-common.c b/src/sbd-common.c +index 679f946..cc84cd0 100644 +--- a/src/sbd-common.c ++++ b/src/sbd-common.c +@@ -262,6 +262,12 @@ struct watchdog_list_item { + struct watchdog_list_item *next; + }; + ++struct link_list_item { ++ char *dev_node; ++ char *link_name; ++ struct link_list_item *next; ++}; ++ + static struct watchdog_list_item *watchdog_list = NULL; + static int watchdog_list_items = 0; + +@@ -275,6 +281,7 @@ watchdog_populate_list(void) + char entry_name[280]; + DIR *dp; + char buf[280] = ""; ++ struct link_list_item *link_list = NULL; + + if (watchdog_list != NULL) { + return; +@@ -288,7 +295,7 @@ watchdog_populate_list(void) + FILE *file; + + snprintf(entry_name, sizeof(entry_name), +- SYS_CLASS_WATCHDOG "/%s/dev", entry->d_name); ++ SYS_CLASS_WATCHDOG "/%s/dev", entry->d_name); + file = fopen(entry_name, "r"); + if (file) { + int major, minor; +@@ -309,43 +316,59 @@ watchdog_populate_list(void) + /* search for watchdog nodes in /dev */ + dp = opendir(WATCHDOG_NODEDIR); + if (dp) { ++ /* first go for links and memorize them */ + while ((entry = readdir(dp))) { +- if ((entry->d_type == DT_CHR) || (entry->d_type == DT_LNK)) { +- struct stat statbuf; ++ if (entry->d_type == DT_LNK) { ++ int len; + + snprintf(entry_name, sizeof(entry_name), +- WATCHDOG_NODEDIR "%s", entry->d_name); +- if (entry->d_type == DT_LNK) { +- int len; +- +- /* !realpath(entry_name, buf) unfortunately does a stat on +- * target so we can't really use it to check if links stay +- * within /dev without triggering e.g. AVC-logs (with +- * SELinux policy that just allows stat within /dev). +- * Without canonicalization that doesn't actually touch the +- * filesystem easily available introduce some limitations +- * for simplicity: +- * - just simple path without '..' +- * - just one level of symlinks (avoid e.g. loop-checking) +- */ +- len = readlink(entry_name, buf, sizeof(buf) - 1); +- if ((len < 1) || +- (len > sizeof(buf) - WATCHDOG_NODEDIR_LEN - 1)) { +- continue; +- } +- buf[len] = '\0'; +- if (buf[0] != '/') { +- memmove(&buf[WATCHDOG_NODEDIR_LEN], buf, len+1); +- memcpy(buf, WATCHDOG_NODEDIR, WATCHDOG_NODEDIR_LEN); +- len += WATCHDOG_NODEDIR_LEN; +- } +- if (strstr(buf, "/../") || +- strncmp(WATCHDOG_NODEDIR, buf, WATCHDOG_NODEDIR_LEN) || +- lstat(buf, &statbuf) || +- !S_ISCHR(statbuf.st_mode)) { +- continue; +- } ++ WATCHDOG_NODEDIR "%s", entry->d_name); ++ ++ /* !realpath(entry_name, buf) unfortunately does a stat on ++ * target so we can't really use it to check if links stay ++ * within /dev without triggering e.g. AVC-logs (with ++ * SELinux policy that just allows stat within /dev). ++ * Without canonicalization that doesn't actually touch the ++ * filesystem easily available introduce some limitations ++ * for simplicity: ++ * - just simple path without '..' ++ * - just one level of symlinks (avoid e.g. loop-checking) ++ */ ++ len = readlink(entry_name, buf, sizeof(buf) - 1); ++ if ((len < 1) || ++ (len > sizeof(buf) - WATCHDOG_NODEDIR_LEN - 1)) { ++ continue; ++ } ++ buf[len] = '\0'; ++ if (buf[0] != '/') { ++ memmove(&buf[WATCHDOG_NODEDIR_LEN], buf, len+1); ++ memcpy(buf, WATCHDOG_NODEDIR, WATCHDOG_NODEDIR_LEN); ++ len += WATCHDOG_NODEDIR_LEN; ++ } ++ if (strstr(buf, "/../") || ++ strncmp(WATCHDOG_NODEDIR, buf, WATCHDOG_NODEDIR_LEN)) { ++ continue; ++ } else { ++ /* just memorize to avoid statting the target - SELinux */ ++ struct link_list_item *lli = ++ calloc(1, sizeof(struct link_list_item)); ++ ++ lli->dev_node = strdup(buf); ++ lli->link_name = strdup(entry_name); ++ lli->next = link_list; ++ link_list = lli; + } ++ } ++ } ++ ++ rewinddir(dp); ++ ++ while ((entry = readdir(dp))) { ++ if (entry->d_type == DT_CHR) { ++ struct stat statbuf; ++ ++ snprintf(entry_name, sizeof(entry_name), ++ WATCHDOG_NODEDIR "%s", entry->d_name); + if(!stat(entry_name, &statbuf) && S_ISCHR(statbuf.st_mode)) { + int i; + +@@ -353,8 +376,9 @@ watchdog_populate_list(void) + if (statbuf.st_rdev == watchdogs[i]) { + int wdfd = watchdog_init_fd(entry_name, -1); + struct watchdog_list_item *wdg = +- calloc(1, sizeof(struct watchdog_list_item)); ++ calloc(1, sizeof(struct watchdog_list_item)); + int len; ++ struct link_list_item *tmp_list = NULL; + + wdg->dev = watchdogs[i]; + wdg->dev_node = strdup(entry_name); +@@ -374,8 +398,8 @@ watchdog_populate_list(void) + } + + snprintf(entry_name, sizeof(entry_name), +- SYS_CHAR_DEV_DIR "/%d:%d/device/driver", +- major(watchdogs[i]), minor(watchdogs[i])); ++ SYS_CHAR_DEV_DIR "/%d:%d/device/driver", ++ major(watchdogs[i]), minor(watchdogs[i])); + len = readlink(entry_name, buf, sizeof(buf) - 1); + if (len > 0) { + buf[len] = '\0'; +@@ -385,14 +409,49 @@ watchdog_populate_list(void) + "Software Watchdog") == 0)) { + wdg->dev_driver = strdup("softdog"); + } ++ ++ /* create dupes if we have memorized links ++ * to this node ++ */ ++ for (tmp_list = link_list; tmp_list; ++ tmp_list = tmp_list->next) { ++ if (!strcmp(tmp_list->dev_node, ++ wdg->dev_node)) { ++ struct watchdog_list_item *dupe_wdg = ++ calloc(1, sizeof(struct watchdog_list_item)); ++ ++ /* as long as we never purge watchdog_list ++ * there is no need to dupe strings ++ */ ++ *dupe_wdg = *wdg; ++ dupe_wdg->dev_node = strdup(tmp_list->link_name); ++ dupe_wdg->next = watchdog_list; ++ watchdog_list = dupe_wdg; ++ watchdog_list_items++; ++ } ++ /* for performance reasons we could remove ++ * the link_list entry ++ */ ++ } + break; + } + } + } + } + } ++ + closedir(dp); + } ++ ++ /* cleanup link list */ ++ while (link_list) { ++ struct link_list_item *tmp_list = link_list; ++ ++ link_list = link_list->next; ++ free(tmp_list->dev_node); ++ free(tmp_list->link_name); ++ free(tmp_list); ++ } + } + + int watchdog_info(void) +-- +1.8.3.1 + diff --git a/SPECS/sbd.spec b/SPECS/sbd.spec index d5a0deb..1a18cbf 100644 --- a/SPECS/sbd.spec +++ b/SPECS/sbd.spec @@ -18,7 +18,7 @@ %global commit a74b4d25a3eb93fe1abbe6e3ebfd2b16cf48873f %global shortcommit %(c=%{commit}; echo ${c:0:7}) %global github_owner Clusterlabs -%global buildnum 7 +%global buildnum 8.2 Name: sbd Summary: Storage-based death @@ -31,6 +31,9 @@ Source0: https://github.com/%{github_owner}/%{name}/archive/%{commit}/%{n Patch0: 0001-make-pacemaker-dlm-wait-for-sbd-start.patch Patch1: 0002-mention-timeout-caveat-with-SBD_DELAY_START.patch Patch2: 0003-Doc-sbd.8.pod-add-query-test-watchdog.patch +Patch11: 0012-Fix-sbd-common-don-t-follow-symlinks-outside-dev-for.patch +Patch12: 0013-Refactor-sbd-common-separate-assignment-and-comparis.patch +Patch13: 0014-Fix-sbd-common-avoid-statting-potential-links.patch BuildRoot: %{_tmppath}/%{name}-%{version}-build BuildRequires: autoconf BuildRequires: automake @@ -46,7 +49,7 @@ BuildRequires: pkgconfig BuildRequires: python-devel %if 0%{?rhel} > 0 -ExclusiveArch: i686 x86_64 s390x ppc64le +ExclusiveArch: i686 x86_64 s390x ppc64le aarch64 %endif %if %{defined systemd_requires} @@ -128,6 +131,21 @@ fi %doc COPYING %changelog +* Wed Sep 19 2018 Klaus Wenninger - 1.3.1-8.2 +- avoid statting potential symlink-targets in /dev + + Resolves: rhbz#1628988 + +* Fri Sep 14 2018 Klaus Wenninger - 1.3.1-8.1 +- skip symlinks pointing to dev-nodes outside of /dev + + Resolves: rhbz#1628988 + +* Mon Apr 16 2018 - 1.3.1-8 +- Added aarch64 target + + Resolves: rhbz#1568029 + * Mon Jan 15 2018 - 1.3.1-7 - reenable sbd on upgrade so that additional links to make pacemaker properly depend on