Skip to content

Start addressing unchecked snprintf() calls #3898

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 49 commits into from
Jul 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
b3a416b
Refactor: libcrmcommon: Avoid snprintf() in time_as_string_common()
nrwahl2 Jun 17, 2025
655b98e
Refactor: libcrmcommon: Support usec in pcmk__time_format_hr() test
nrwahl2 Jun 19, 2025
960d042
Test: libcrmcommon: Test pcmk__time_format_hr() with nonzero usec
nrwahl2 Jun 19, 2025
0fcc669
Refactor: libcrmcommon: Avoid snprintf() in pcmk__time_format_hr()
nrwahl2 Jun 17, 2025
8d47cea
Refactor: libcrmcommon: Clarify fractional seconds in time format
nrwahl2 Jun 19, 2025
335a2ac
Refactor: libcrmcommon: Avoid sprintf() in pcmk__time_format_hr()
nrwahl2 Jun 19, 2025
4b075d2
Refactor: libcrmcommon: Use g_strndup() in pcmk__time_format_hr()
nrwahl2 Jun 19, 2025
4d739a9
Doc: Pacemaker Explained: Clarify alert timestamp-format
nrwahl2 Jun 19, 2025
6bad182
Refactor: based: Error-check snprintf() in cib_digester_cb()
nrwahl2 Jun 20, 2025
b8316fa
Refactor: executor: Error-check snprintf() in read_events()
nrwahl2 Jun 20, 2025
d001021
Refactor: executor: Avoid snprintf() in get_address_info()
nrwahl2 Jun 20, 2025
8ed2743
Refactor: fencer: Avoid snprintf() in test_register_async_devices()
nrwahl2 Jun 20, 2025
cdeccf6
Refactor: fencer: Avoid snprintf() in get_action_timeout()
nrwahl2 Jun 20, 2025
e63bdba
Refactor: pacemakerd: Avoid snprintf() in pacemakerd_read_config()
nrwahl2 Jun 20, 2025
4c1024f
Refactor: libcrmcommon: Avoid snprintf() in no_pids()/has_pids()
nrwahl2 Jun 20, 2025
107a779
Refactor: libcrmcommon: Avoid snprintf() in pcmk__scan_double() tests
nrwahl2 Jun 20, 2025
5ebfd70
Refactor: libcrmcommon: Avoid snprintf() in log_list_item()
nrwahl2 Jun 20, 2025
c16887f
Refactor: libcrmcommon: Avoid snprintf() in pcmk__node_attr_target()
nrwahl2 Jun 20, 2025
a4d9c92
Refactor: libcrmcommon: Avoid snprintf() in set_format_string()
nrwahl2 Jun 20, 2025
249a3da
Refactor: libcrmcommon: Avoid snprintf() in pcmk__pid_active()
nrwahl2 Jun 20, 2025
d37b5e2
Refactor: libcrmcommon: Error-check snprintf() in pcmk_readable_score()
nrwahl2 Jun 20, 2025
3f848cb
Refactor: libcrmcommon: Avoid snprintf() in pcmk__set_env_option()
nrwahl2 Jun 20, 2025
4c17a60
Refactor: libcrmcommon: Allow '=' in pcmk__set_env_option()
nrwahl2 Jun 21, 2025
3702fcb
Refactor: libcrmcommon: Avoid snprintf() in pcmk__env_option()
nrwahl2 Jun 21, 2025
4ee10dc
Refactor: libcrmcommon: Return path from pcmk__procfs_pid2path()
nrwahl2 Jun 21, 2025
47741a9
Refactor: libcrmcommon: Avoid snprintf() in pcmk__procfs_pid2path()
nrwahl2 Jun 21, 2025
697422f
Refactor: libcrmcommon: Avoid snprintf() in crm_xml_add_ll()
nrwahl2 Jun 21, 2025
cf90a75
Refactor: libpe_status: Avoid snprintf in pe__bundle_replica_output_html
nrwahl2 Jun 21, 2025
b1f4cc9
Refactor: libpe_status: Avoid snprintf in pe__bundle_replica_output_text
nrwahl2 Jun 21, 2025
008efad
Refactor: libcrmcommon: Make pcmk__read_pidfile() static
nrwahl2 Jun 21, 2025
1315fd4
Refactor: libcrmcommon: Error-check snprintf() in pcmk__lock_pidfile()
nrwahl2 Jun 23, 2025
e75b485
Refactor: libcrmcommon: Avoid snprintf() in crm_write_blackbox()
nrwahl2 Jun 23, 2025
6af9ad6
Refactor: libcrmcommon: Avoid snprintf() in crm_log_filter()
nrwahl2 Jun 23, 2025
c076d5e
Refactor: libstonithd: Avoid snprintf() in stonith__action_create()
nrwahl2 Jun 23, 2025
906b56e
Refactor: liblrmd: Avoid snprintf() in exec_alert_list()
nrwahl2 Jun 23, 2025
d0a14b0
Refactor: libpacemaker: Error-check snprintf() in result_code_text()
nrwahl2 Jun 23, 2025
b31f2ab
Refactor: libpe_status: Avoid snprintf() in pe__resource_xml()
nrwahl2 Jun 23, 2025
5ed177a
Refactor: libpe_status: Avoid snprintf() in get_rscs_brief()
nrwahl2 Jun 23, 2025
566398f
Refactor: libcrmservice: Avoid snprintf() in services__get_lsb_metadata
nrwahl2 Jun 23, 2025
dd9e8f5
Refactor: libcrmservice: Avoid snprintf() in services_linux.c
nrwahl2 Jun 23, 2025
f07769d
Refactor: libcrmcommon: Avoid snprintf() in pcmk__readable_interval()
nrwahl2 Jun 23, 2025
c162dee
Test: libcrmcommon: Test y-m-d for pcmk__time_format_hr()
nrwahl2 Jun 26, 2025
ccbb956
Test: libcrmcommon: Use June for pcmk__time_format_hr() test date
nrwahl2 Jun 26, 2025
6a3c450
Refactor: libcrmcommon: Break up TIME_S for convenience
nrwahl2 Jun 27, 2025
3bb625e
Test: libcrmcommon: Test characters after %N
nrwahl2 Jun 27, 2025
ff0e441
Refactor: libcrmcommon: Simplify ha_get_tm_time()
nrwahl2 Jun 27, 2025
f0af304
Refactor: libcrmcommon: Drop pcmk__time_set_hr_dt()
nrwahl2 Jun 30, 2025
a5658f7
Refactor: libcrmcommon: Make pcmk__time_hr_convert() static
nrwahl2 Jun 30, 2025
5a783ff
Refactor: libcrmcommon: Use g_date_time_format() instead of strftime()
nrwahl2 Jun 27, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion daemons/based/based_callbacks.c
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,8 @@ cib_digester_cb(gpointer data)
free(ping_digest);
ping_digest = NULL;
ping_modified_since = FALSE;
snprintf(buffer, 32, "%" PRIu64, ping_seq);
pcmk__assert(snprintf(buffer, 32, "%" PRIu64, ping_seq) >= 0);

crm_trace("Requesting peer digests (%s)", buffer);

crm_xml_add(ping, PCMK__XA_T, PCMK__VALUE_CIB);
Expand Down
33 changes: 15 additions & 18 deletions daemons/execd/cts-exec-helper.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2024 the Pacemaker project contributors
* Copyright 2012-2025 the Pacemaker project contributors
*
* The version control history for this file may have further details.
*
Expand Down Expand Up @@ -160,8 +160,6 @@ static GOptionEntry api_call_entries[] = {
static GMainLoop *mainloop = NULL;
static lrmd_t *lrmd_conn = NULL;

static char event_buf_v0[1024];

static crm_exit_t
test_exit(crm_exit_t exit_code)
{
Expand All @@ -174,15 +172,6 @@ test_exit(crm_exit_t exit_code)
printf(fmt "\n" , ##args); \
}

#define report_event(event) \
snprintf(event_buf_v0, sizeof(event_buf_v0), "NEW_EVENT event_type:%s rsc_id:%s action:%s rc:%s op_status:%s", \
lrmd_event_type2str(event->type), \
event->rsc_id, \
event->op_type ? event->op_type : "none", \
crm_exit_str((crm_exit_t) event->rc), \
pcmk_exec_status_str(event->op_status)); \
crm_info("%s", event_buf_v0);

static void
test_shutdown(int nsig)
{
Expand All @@ -193,12 +182,20 @@ test_shutdown(int nsig)
static void
read_events(lrmd_event_data_t * event)
{
report_event(event);
if (options.listen) {
if (pcmk__str_eq(options.listen, event_buf_v0, pcmk__str_casei)) {
print_result("LISTEN EVENT SUCCESSFUL");
test_exit(CRM_EX_OK);
}
char buf[1024] = { '\0', };

pcmk__assert(snprintf(buf, sizeof(buf),
"NEW_EVENT event_type:%s rsc_id:%s action:%s rc:%s "
"op_status:%s",
lrmd_event_type2str(event->type), event->rsc_id,
pcmk__s(event->op_type, "none"),
crm_exit_str((crm_exit_t) event->rc),
pcmk_exec_status_str(event->op_status)) >= 0);
crm_info("%s", buf);

if (options.listen && pcmk__str_eq(options.listen, buf, pcmk__str_casei)) {
print_result("LISTEN EVENT SUCCESSFUL");
test_exit(CRM_EX_OK);
}

if (exec_call_id && (event->call_id == exec_call_id)) {
Expand Down
11 changes: 5 additions & 6 deletions daemons/execd/remoted_tls.c
Original file line number Diff line number Diff line change
Expand Up @@ -319,8 +319,8 @@ bind_and_listen(struct addrinfo *addr)
static int
get_address_info(const char *bind_name, int port, struct addrinfo **res)
{
int rc;
char port_str[6]; // at most "65535"
int rc = pcmk_rc_ok;
char *port_s = pcmk__itoa(port);
struct addrinfo hints;

memset(&hints, 0, sizeof(struct addrinfo));
Expand All @@ -329,17 +329,16 @@ get_address_info(const char *bind_name, int port, struct addrinfo **res)
hints.ai_socktype = SOCK_STREAM;
hints.ai_protocol = IPPROTO_TCP;

snprintf(port_str, sizeof(port_str), "%d", port);
rc = getaddrinfo(bind_name, port_str, &hints, res);
rc = getaddrinfo(bind_name, port_s, &hints, res);
rc = pcmk__gaierror2rc(rc);

if (rc != pcmk_rc_ok) {
crm_err("Unable to get IP address(es) for %s: %s",
(bind_name? bind_name : "local node"), pcmk_rc_str(rc));
return rc;
}

return pcmk_rc_ok;
free(port_s);
return rc;
}

int
Expand Down
7 changes: 4 additions & 3 deletions daemons/fenced/cts-fence-helper.c
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,8 @@ test_async_monitor(int check_event)
static void
test_register_async_devices(int check_event)
{
char buf[16] = { 0, };
char *off_timeout_s = pcmk__itoa(MAINLOOP_DEFAULT_TIMEOUT
+ CUSTOM_TIMEOUT_ADDITION);
stonith_key_value_t *params = NULL;

params = stonith__key_value_add(params, PCMK_STONITH_HOST_MAP,
Expand All @@ -518,12 +519,12 @@ test_register_async_devices(int check_event)
"custom_timeout_node1=1,2");
params = stonith__key_value_add(params, "mode", "fail");
params = stonith__key_value_add(params, "delay", "1000");
snprintf(buf, sizeof(buf) - 1, "%d", MAINLOOP_DEFAULT_TIMEOUT + CUSTOM_TIMEOUT_ADDITION);
params = stonith__key_value_add(params, "pcmk_off_timeout", buf);
params = stonith__key_value_add(params, "pcmk_off_timeout", off_timeout_s);
st->cmds->register_device(st, st_opts, "false_custom_timeout", "stonith-ng", "fence_dummy",
params);
stonith__key_value_freeall(params, true, true);

free(off_timeout_s);
mainloop_test_done(__func__, true);
}

Expand Down
8 changes: 5 additions & 3 deletions daemons/fenced/fenced_commands.c
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ get_action_timeout(const fenced_device_t *device, const char *action,
int default_timeout)
{
if (action && device && device->params) {
char buffer[64] = { 0, };
char *timeout_param = NULL;
const char *value = NULL;

/* If "reboot" was requested but the device does not support it,
Expand All @@ -286,8 +286,10 @@ get_action_timeout(const fenced_device_t *device, const char *action,
}

/* If the device config specified an action-specific timeout, use it */
snprintf(buffer, sizeof(buffer), "pcmk_%s_timeout", action);
value = g_hash_table_lookup(device->params, buffer);
timeout_param = crm_strdup_printf("pcmk_%s_timeout", action);
value = g_hash_table_lookup(device->params, timeout_param);
free(timeout_param);

if (value) {
long long timeout_ms = crm_get_msec(value);
return (int) QB_MIN(pcmk__timeout_ms2s(timeout_ms), INT_MAX);
Expand Down
8 changes: 5 additions & 3 deletions daemons/pacemakerd/pcmkd_corosync.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2010-2024 the Pacemaker project contributors
* Copyright 2010-2025 the Pacemaker project contributors
*
* The version control history for this file may have further details.
*
Expand Down Expand Up @@ -360,9 +360,11 @@ pacemakerd_read_config(void)
" No group found for user %s", CRM_DAEMON_USER);

} else {
char key[PATH_MAX];
snprintf(key, PATH_MAX, "uidgid.gid.%u", gid);
char *key = crm_strdup_printf("uidgid.gid.%lld", (long long) gid);

rc = cmap_set_uint8(local_handle, key, 1);
free(key);

if (rc != CS_OK) {
crm_warn("Could not authorize group with Corosync: %s " QB_XS
" group=%u rc=%d", pcmk__cs_err_str(rc), gid, rc);
Expand Down
17 changes: 15 additions & 2 deletions doc/sphinx/Pacemaker_Explained/alerts.rst
Original file line number Diff line number Diff line change
Expand Up @@ -114,14 +114,27 @@ whether and how Pacemaker calls them.
| | | recipient of that alert, that recipient will not be |
| | | used. *(since 2.1.6)* |
+------------------+---------------+-----------------------------------------------------+
| timestamp-format | %H:%M:%S.%06N | .. index:: |
| timestamp-format | %H:%M:%S.%6N | .. index:: |
| | | single: alert; meta-attribute, timestamp-format |
| | | single: meta-attribute; timestamp-format (alert) |
| | | single: timestamp-format; alert meta-attribute |
| | | |
| | | Format the cluster will use when sending the |
| | | event's timestamp to the agent. This is a string as |
| | | used with the ``date(1)`` command. |
| | | used with the ``date(1)`` command, with the |
| | | following extension. ``"%xN"``, where ``x`` is a |
| | | number with ``1 <= x <= 6``, prints the fractional |
| | | seconds component of the timestamp at ``10^(-x)`` |
| | | resolution, without a decimal point (``'.'``). |
| | | Values are truncated toward zero, not rounded. |
| | | |
| | | Note: This is implemented using ``strftime()`` with |
| | | a 128-character buffer. If any format specifier's |
| | | expansion requires more than 128 characters, or if |
| | | any specifier expands to an empty string, then the |
| | | timestamp is discarded. (Expanding to an empty |
| | | string is not an error, but there is no way to |
| | | distinguish this from a too-small buffer.) |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

......I looked hard for anything like this, in GLib or elsewhere, and never found this. Sigh. What I did find was https://docs.gtk.org/glib/type_func.Date.strftime.html, which doesn't help at all.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The '%f' could be interesting. Unfortunately it seems not to support limiting to less than 6 digits. And I guess we should stay compatible here with the timestamp-format introduced initially.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wouldn't mean we have to use g_date_time_format() to replace our parsing of %N. We could perhaps use it to replace the strftime() call, which could be nice.

Although even there, "The format strings understood by this function are a subset of the strftime() format language as specified by C99."

"Subset."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far, two unit tests fail because g_date_time_format() doesn't support a bare '%' or a width field (e.g., "%3s"). It returns NULL when it finds either of these.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was just thinking of a way to get rid of the %N implementation stating that "s/%.+N/%f/" is unfortunately not exactly the same :-(

Copy link
Contributor

@wenningerk wenningerk Jun 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A width field should be fine with the %N implementation but it would as well stumble over a bare '%' at least if there is another format to follow as it would eat the next '%'.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating to use g_date_time_format() with a fallback to strftime() for now

+------------------+---------------+-----------------------------------------------------+
| timeout | 30s | .. index:: |
| | | single: alert; meta-attribute, timeout |
Expand Down
3 changes: 1 addition & 2 deletions include/crm/common/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ char *pcmk__format_nvpair(const char *name, const char *value,

pid_t pcmk__procfs_pid_of(const char *name);
unsigned int pcmk__procfs_num_cores(void);
int pcmk__procfs_pid2path(pid_t pid, char path[], size_t path_size);
int pcmk__procfs_pid2path(pid_t pid, char **path);
bool pcmk__procfs_has_pids(void);
DIR *pcmk__procfs_fd_dir(void);
void pcmk__sysrq_trigger(char t);
Expand All @@ -101,7 +101,6 @@ bool pcmk__throttle_load_avg(float *load);
*/
int pcmk__pid_active(pid_t pid, const char *daemon);

int pcmk__read_pidfile(const char *filename, pid_t *pid);
int pcmk__pidfile_matches(const char *filename, pid_t expected_pid,
const char *expected_name, pid_t *pid);
int pcmk__lock_pidfile(const char *filename, const char *name);
Expand Down
5 changes: 1 addition & 4 deletions include/crm/common/iso8601_internal.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2015-2024 the Pacemaker project contributors
* Copyright 2015-2025 the Pacemaker project contributors
*
* The version control history for this file may have further details.
*
Expand All @@ -22,9 +22,6 @@ extern "C" {

typedef struct pcmk__time_us pcmk__time_hr_t;

pcmk__time_hr_t *pcmk__time_hr_convert(pcmk__time_hr_t *target,
const crm_time_t *dt);
void pcmk__time_set_hr_dt(crm_time_t *target, const pcmk__time_hr_t *hr_dt);
pcmk__time_hr_t *pcmk__time_hr_now(time_t *epoch);
pcmk__time_hr_t *pcmk__time_hr_new(const char *date_time);
void pcmk__time_hr_free(pcmk__time_hr_t *hr_dt);
Expand Down
16 changes: 9 additions & 7 deletions lib/common/attrs.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2011-2024 the Pacemaker project contributors
* Copyright 2011-2025 the Pacemaker project contributors
*
* The version control history for this file may have further details.
*
Expand Down Expand Up @@ -38,18 +38,22 @@ const char *
pcmk__node_attr_target(const char *name)
{
if (name == NULL || pcmk__strcase_any_of(name, "auto", "localhost", NULL)) {
char buf[128] = OCF_RESKEY_PREFIX;
size_t offset = sizeof(OCF_RESKEY_PREFIX) - 1;
char *buf = NULL;
char *target_var = crm_meta_name(PCMK_META_CONTAINER_ATTRIBUTE_TARGET);
char *phys_var = crm_meta_name(PCMK__META_PHYSICAL_HOST);
const char *target = NULL;
const char *host_physical = NULL;

snprintf(buf + offset, sizeof(buf) - offset, "%s", target_var);
buf = crm_strdup_printf(OCF_RESKEY_PREFIX "%s", target_var);
target = getenv(buf);
free(buf);

snprintf(buf + offset, sizeof(buf) - offset, "%s", phys_var);
buf = crm_strdup_printf(OCF_RESKEY_PREFIX "%s", phys_var);
host_physical = getenv(buf);
free(buf);

free(target_var);
free(phys_var);

// It is important to use the name by which the scheduler knows us
if (host_physical
Expand All @@ -63,8 +67,6 @@ pcmk__node_attr_target(const char *name)
name = host_pcmk;
}
}
free(target_var);
free(phys_var);

// TODO? Call pcmk__cluster_local_node_name() if name == NULL
// (currently would require linkage against libcrmcluster)
Expand Down
Loading