!30 pacemaker: Check snprintf return values
From: @bizhiyuan Reviewed-by: @bixiaoyan1 Signed-off-by: @bixiaoyan1
This commit is contained in:
commit
bdc2de7135
@ -24,7 +24,7 @@
|
||||
%bcond_with run_build_tests
|
||||
%bcond_with include_unit_test
|
||||
|
||||
%global release 4
|
||||
%global release 5
|
||||
|
||||
## User and group to use for nonprivileged services (should be in sync with pacemaker)
|
||||
%global uname hacluster
|
||||
@ -51,6 +51,7 @@ Source0: https://github.com/%{github_owner}/%{name}/releases/download/v%{
|
||||
Patch0: Remove-const-warning.patch
|
||||
Patch1: pacemaker-Don-t-add-explicit-error-prefix-in-log.patch
|
||||
Patch2: pacemaker-Use-long-format-for-crm_ticket-v.patch
|
||||
Patch3: pacemaker-Check-snprintf-return-values.patch
|
||||
|
||||
# direct build process dependencies
|
||||
BuildRequires: autoconf
|
||||
@ -299,6 +300,9 @@ VERBOSE=1 make check
|
||||
%{_usr}/lib/ocf/resource.d/booth/sharedrsc
|
||||
|
||||
%changelog
|
||||
* Sun Apr 28 2024 bizhiyuan <bizhiyuan@kylinos.cn> - 1.1-5
|
||||
- pacemaker Check snprintf return values
|
||||
|
||||
* Thu Feb 29 2024 bizhiyuan <bizhiyuan@kylinos.cn> - 1.1-4
|
||||
- pacemaker: Use long format for crm_ticket -v
|
||||
|
||||
|
||||
124
pacemaker-Check-snprintf-return-values.patch
Normal file
124
pacemaker-Check-snprintf-return-values.patch
Normal file
@ -0,0 +1,124 @@
|
||||
From 7e33a45d6898e06119dbe9dfd487f6c4923b48cb Mon Sep 17 00:00:00 2001
|
||||
From: Jan Friesse <jfriesse@redhat.com>
|
||||
Date: Tue, 14 Nov 2023 17:21:49 +0100
|
||||
Subject: [PATCH 2/7] pacemaker: Check snprintf return values
|
||||
|
||||
crm_ticket command string is stored into static buffer and not checked
|
||||
so it can be truncated without notice.
|
||||
|
||||
Solution would be to use dynamic buffer, but for now at least check
|
||||
snprintf return value and return error when string was truncated.
|
||||
|
||||
Signed-off-by: Jan Friesse <jfriesse@redhat.com>
|
||||
---
|
||||
src/pacemaker.c | 39 ++++++++++++++++++++++++++++++++++-----
|
||||
1 file changed, 34 insertions(+), 5 deletions(-)
|
||||
|
||||
diff --git a/src/pacemaker.c b/src/pacemaker.c
|
||||
index 8ad3c69..80aa1a3 100644
|
||||
--- a/src/pacemaker.c
|
||||
+++ b/src/pacemaker.c
|
||||
@@ -128,7 +128,7 @@ static int pcmk_write_ticket_atomic(struct ticket_config *tk, int grant)
|
||||
|
||||
/* The long format (--attr-value=) for attribute value is used instead of "-v",
|
||||
* so that NO_ONE (which is -1) isn't seen as another option. */
|
||||
- snprintf(cmd, COMMAND_MAX,
|
||||
+ rv = snprintf(cmd, COMMAND_MAX,
|
||||
"crm_ticket -t '%s' "
|
||||
"%s --force "
|
||||
"-S owner --attr-value=%" PRIi32 " "
|
||||
@@ -142,6 +142,10 @@ static int pcmk_write_ticket_atomic(struct ticket_config *tk, int grant)
|
||||
(int64_t)wall_ts(&tk->term_expires),
|
||||
(int64_t)tk->current_term);
|
||||
|
||||
+ if (rv < 0 || rv >= COMMAND_MAX) {
|
||||
+ log_error("pcmk_write_ticket_atomic: cannot format crm_ticket cmdline (probably too long)");
|
||||
+ return -1;
|
||||
+ }
|
||||
rv = system(cmd);
|
||||
log_debug("command: '%s' was executed", cmd);
|
||||
if (rv != 0)
|
||||
@@ -230,20 +234,34 @@ static int crm_ticket_set_int(const struct ticket_config *tk, const char *attr,
|
||||
static int pcmk_set_attr(struct ticket_config *tk, const char *attr, const char *val)
|
||||
{
|
||||
char cmd[COMMAND_MAX];
|
||||
+ int rv;
|
||||
|
||||
- snprintf(cmd, COMMAND_MAX,
|
||||
+ rv = snprintf(cmd, COMMAND_MAX,
|
||||
"crm_ticket -t '%s' -S '%s' --attr-value='%s'",
|
||||
tk->name, attr, val);
|
||||
+
|
||||
+ if (rv < 0 || rv >= COMMAND_MAX) {
|
||||
+ log_error("pcmk_set_attr: cannot format crm_ticket cmdline (probably too long)");
|
||||
+ return -1;
|
||||
+ }
|
||||
+
|
||||
return _run_crm_ticket(cmd);
|
||||
}
|
||||
|
||||
static int pcmk_del_attr(struct ticket_config *tk, const char *attr)
|
||||
{
|
||||
char cmd[COMMAND_MAX];
|
||||
+ int rv;
|
||||
|
||||
- snprintf(cmd, COMMAND_MAX,
|
||||
+ rv = snprintf(cmd, COMMAND_MAX,
|
||||
"crm_ticket -t '%s' -D '%s'",
|
||||
tk->name, attr);
|
||||
+
|
||||
+ if (rv < 0 || rv >= COMMAND_MAX) {
|
||||
+ log_error("pcmk_del_attr: cannot format crm_ticket cmdline (probably too long)");
|
||||
+ return -1;
|
||||
+ }
|
||||
+
|
||||
return _run_crm_ticket(cmd);
|
||||
}
|
||||
|
||||
@@ -352,13 +370,18 @@ static int pcmk_get_attr(struct ticket_config *tk, const char *attr, const char
|
||||
char cmd[COMMAND_MAX];
|
||||
char line[BOOTH_ATTRVAL_LEN+1];
|
||||
int rv = 0, pipe_rv;
|
||||
+ int res;
|
||||
FILE *p;
|
||||
|
||||
|
||||
*vp = NULL;
|
||||
- snprintf(cmd, COMMAND_MAX,
|
||||
+ res = snprintf(cmd, COMMAND_MAX,
|
||||
"crm_ticket -t '%s' -G '%s' --quiet",
|
||||
tk->name, attr);
|
||||
+ if (res < 0 || res >= COMMAND_MAX) {
|
||||
+ log_error("pcmk_get_attr: cannot format crm_ticket cmdline (probably too long)");
|
||||
+ return -1;
|
||||
+ }
|
||||
|
||||
p = popen(cmd, "r");
|
||||
if (p == NULL) {
|
||||
@@ -483,16 +506,22 @@ static int pcmk_load_ticket(struct ticket_config *tk)
|
||||
{
|
||||
char cmd[COMMAND_MAX];
|
||||
int rv = 0, pipe_rv;
|
||||
+ int res;
|
||||
FILE *p;
|
||||
|
||||
/* This here gets run during startup; testing that here means that
|
||||
* normal operation won't be interrupted with that test. */
|
||||
test_atomicity();
|
||||
|
||||
- snprintf(cmd, COMMAND_MAX,
|
||||
+ res = snprintf(cmd, COMMAND_MAX,
|
||||
"crm_ticket -t '%s' -q",
|
||||
tk->name);
|
||||
|
||||
+ if (res < 0 || res >= COMMAND_MAX) {
|
||||
+ log_error("pcmk_load_ticket: cannot format crm_ticket cmdline (probably too long)");
|
||||
+ return -1;
|
||||
+ }
|
||||
+
|
||||
p = popen(cmd, "r");
|
||||
if (p == NULL) {
|
||||
pipe_rv = errno;
|
||||
--
|
||||
2.33.0
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user