Jim,
thanks for pointing bugs. Attached patch should be fine (I hope).
Regards,
Honza
Jim Meyering wrote:
> Jan Friesse wrote:
>> Attached patch solves possible problems of overflow buffer, which can
>> happened with sprintf. I tried to change only files where it makes sense
>> (so basically, where in format is something like %s) and I hope I catch
>> all cases.
>>
>>
>> Index: test/sa_error.c
> ...
>> - sprintf (test_result,
>> + sprintf (test_result, sizeof(test_result),
> ...
>> Index: lcr/lcr_ifact.c
> ...
>> - sprintf (filename_cat, "%s/%s", path, filename);
>> + sprintf (filename_cat, sizeof(filename_cat), "%s/%s", path, filename);
>
> Oops. you meant to convert to snprintf.
Index: test/sa_error.c
===================================================================
--- test/sa_error.c (revision 1987)
+++ test/sa_error.c (working copy)
@@ -59,7 +59,7 @@
if (result == expected) {
return ("PASSED");
} else {
- sprintf (test_result,
+ snprintf (test_result, sizeof(test_result),
"FAILED expected %s got %s",
get_sa_error_b(expected), get_sa_error_b(result));
return (test_result);
Index: test/logsysbench.c
===================================================================
--- test/logsysbench.c (revision 1987)
+++ test/logsysbench.c (working copy)
@@ -131,11 +131,11 @@
*/
printf ("heating up cache with sprintf functionality\n");
for (i = 0; i < ITERATIONS; i++) {
- sprintf (buf, "Some logging information %s", "recordA");
+ snprintf (buf, sizeof(buf), "Some logging information %s", "recordA");
}
bm_start();
for (i = 0; i < ITERATIONS; i++) {
- sprintf (buf, "Some logging information %s", "recordA");
+ snprintf (buf, sizeof(buf), "Some logging information %s", "recordA");
}
bm_finish ("sprintf 1 argument:");
bm_start();
Index: lcr/lcr_ifact.c
===================================================================
--- lcr/lcr_ifact.c (revision 1987)
+++ lcr/lcr_ifact.c (working copy)
@@ -224,7 +224,7 @@
struct dirent **scandir_list;
unsigned int scandir_entries;
- sprintf (filename_cat, "%s/%s", path, filename);
+ snprintf (filename_cat, sizeof(filename_cat), "%s/%s", path, filename);
if (filename[0] == '*') {
scandir_entries = scandir (
path,
@@ -375,7 +375,8 @@
/*
* Load objects, scan them, unload them if they are not a match
*/
- sprintf (dl_name, "%s/%s", path, scandir_list[libs_to_scan]->d_name);
+ snprintf (dl_name, sizeof(dl_name), "%s/%s",
+ path, scandir_list[libs_to_scan]->d_name);
/*
* Don't reload already loaded libraries
*/
Index: exec/coroparse.c
===================================================================
--- exec/coroparse.c (revision 1987)
+++ exec/coroparse.c (working copy)
@@ -171,7 +171,8 @@
fp = fopen (filename, "r");
if (fp == NULL) {
- sprintf (error_reason, "Can't read file %s reason = (%s)\n",
+ snprintf (error_reason, sizeof(error_string_response),
+ "Can't read file %s reason = (%s)\n",
filename, strerror (errno));
*error_string = error_reason;
return -1;
@@ -181,7 +182,8 @@
fclose(fp);
- sprintf (error_reason, "Successfully read main configuration file '%s'.\n", filename);
+ snprintf (error_reason, sizeof(error_string_response),
+ "Successfully read main configuration file '%s'.\n", filename);
*error_string = error_reason;
return res;
Index: exec/mainconfig.c
===================================================================
--- exec/mainconfig.c (revision 1987)
+++ exec/mainconfig.c (working copy)
@@ -375,7 +375,7 @@
return 0;
parse_error:
- sprintf (error_string_response,
+ snprintf (error_string_response, sizeof(error_string_response),
"parse error in config: %s.\n",
error_reason);
@@ -466,7 +466,7 @@
return 0;
parse_error:
- sprintf (error_string_response,
+ snprintf (error_string_response, sizeof(error_string_response),
"parse error in config: %s.\n",
error_reason);
Index: exec/totemsrp.c
===================================================================
--- exec/totemsrp.c (revision 1987)
+++ exec/totemsrp.c (working copy)
@@ -2939,7 +2939,7 @@
int res;
char filename[256];
- sprintf (filename, "%s/ringid_%s",
+ snprintf (filename, sizeof(filename), "%s/ringid_%s",
rundir, totemip_print (&instance->my_id.addr[0]));
fd = open (filename, O_RDONLY, 0700);
if (fd > 0) {
@@ -2978,7 +2978,7 @@
memcpy (&instance->my_ring_id, ring_id, sizeof (struct memb_ring_id));
- sprintf (filename, "%s/ringid_%s",
+ snprintf (filename, sizeof(filename), "%s/ringid_%s",
rundir, totemip_print (&instance->my_id.addr[0]));
fd = open (filename, O_WRONLY, 0777);
Index: exec/totemconfig.c
===================================================================
--- exec/totemconfig.c (revision 1987)
+++ exec/totemconfig.c (working copy)
@@ -410,7 +410,8 @@
}
if (totem_config->max_network_delay < MINIMUM_TIMEOUT) {
- sprintf (local_error_reason, "The max_network_delay parameter (%d ms) may not be less then (%d ms).",
+ snprintf (local_error_reason, sizeof(local_error_reason),
+ "The max_network_delay parameter (%d ms) may not be less then (%d ms).",
totem_config->max_network_delay, MINIMUM_TIMEOUT);
goto parse_error;
}
@@ -424,7 +425,8 @@
}
if (totem_config->token_timeout < MINIMUM_TIMEOUT) {
- sprintf (local_error_reason, "The token timeout parameter (%d ms) may not be less then (%d ms).",
+ snprintf (local_error_reason, sizeof(local_error_reason),
+ "The token timeout parameter (%d ms) may not be less then (%d ms).",
totem_config->token_timeout, MINIMUM_TIMEOUT);
goto parse_error;
}
@@ -440,7 +442,8 @@
(1000/HZ));
}
if (totem_config->token_retransmit_timeout < MINIMUM_TIMEOUT) {
- sprintf (local_error_reason, "The token retransmit timeout parameter (%d ms) may not be less then (%d ms).",
+ snprintf (local_error_reason, sizeof(local_error_reason),
+ "The token retransmit timeout parameter (%d ms) may not be less then (%d ms).",
totem_config->token_retransmit_timeout, MINIMUM_TIMEOUT);
goto parse_error;
}
@@ -450,7 +453,8 @@
}
if (totem_config->token_hold_timeout < MINIMUM_TIMEOUT) {
- sprintf (local_error_reason, "The token hold timeout parameter (%d ms) may not be less then (%d ms).",
+ snprintf (local_error_reason, sizeof(local_error_reason),
+ "The token hold timeout parameter (%d ms) may not be less then (%d ms).",
totem_config->token_hold_timeout, MINIMUM_TIMEOUT);
goto parse_error;
}
@@ -460,7 +464,8 @@
}
if (totem_config->join_timeout < MINIMUM_TIMEOUT) {
- sprintf (local_error_reason, "The join timeout parameter (%d ms) may not be less then (%d ms).",
+ snprintf (local_error_reason, sizeof(local_error_reason),
+ "The join timeout parameter (%d ms) may not be less then (%d ms).",
totem_config->join_timeout, MINIMUM_TIMEOUT);
goto parse_error;
}
@@ -470,7 +475,8 @@
}
if (totem_config->consensus_timeout < MINIMUM_TIMEOUT) {
- sprintf (local_error_reason, "The consensus timeout parameter (%d ms) may not be less then (%d ms).",
+ snprintf (local_error_reason, sizeof(local_error_reason),
+ "The consensus timeout parameter (%d ms) may not be less then (%d ms).",
totem_config->consensus_timeout, MINIMUM_TIMEOUT);
goto parse_error;
}
@@ -480,7 +486,8 @@
}
if (totem_config->merge_timeout < MINIMUM_TIMEOUT) {
- sprintf (local_error_reason, "The merge timeout parameter (%d ms) may not be less then (%d ms).",
+ snprintf (local_error_reason, sizeof(local_error_reason),
+ "The merge timeout parameter (%d ms) may not be less then (%d ms).",
totem_config->merge_timeout, MINIMUM_TIMEOUT);
goto parse_error;
}
@@ -490,7 +497,8 @@
}
if (totem_config->downcheck_timeout < MINIMUM_TIMEOUT) {
- sprintf (local_error_reason, "The downcheck timeout parameter (%d ms) may not be less then (%d ms).",
+ snprintf (local_error_reason, sizeof(local_error_reason),
+ "The downcheck timeout parameter (%d ms) may not be less then (%d ms).",
totem_config->downcheck_timeout, MINIMUM_TIMEOUT);
goto parse_error;
}
@@ -501,14 +509,16 @@
if (strcmp (totem_config->rrp_mode, "none") &&
strcmp (totem_config->rrp_mode, "active") &&
strcmp (totem_config->rrp_mode, "passive")) {
- sprintf (local_error_reason, "The RRP mode \"%s\" specified is invalid. It must be none, active, or passive.\n", totem_config->rrp_mode);
+ snprintf (local_error_reason, sizeof(local_error_reason),
+ "The RRP mode \"%s\" specified is invalid. It must be none, active, or passive.\n", totem_config->rrp_mode);
goto parse_error;
}
if (totem_config->rrp_problem_count_timeout == 0) {
totem_config->rrp_problem_count_timeout = RRP_PROBLEM_COUNT_TIMEOUT;
}
if (totem_config->rrp_problem_count_timeout < MINIMUM_TIMEOUT) {
- sprintf (local_error_reason, "The RRP problem count timeout parameter (%d ms) may not be less then (%d ms).",
+ snprintf (local_error_reason, sizeof(local_error_reason),
+ "The RRP problem count timeout parameter (%d ms) may not be less then (%d ms).",
totem_config->rrp_problem_count_timeout, MINIMUM_TIMEOUT);
goto parse_error;
}
@@ -516,7 +526,8 @@
totem_config->rrp_problem_count_threshold = RRP_PROBLEM_COUNT_THRESHOLD_DEFAULT;
}
if (totem_config->rrp_problem_count_threshold < RRP_PROBLEM_COUNT_THRESHOLD_MIN) {
- sprintf (local_error_reason, "The RRP problem count threshold (%d problem count) may not be less then (%d problem count).",
+ snprintf (local_error_reason, sizeof(local_error_reason),
+ "The RRP problem count threshold (%d problem count) may not be less then (%d problem count).",
totem_config->rrp_problem_count_threshold, RRP_PROBLEM_COUNT_THRESHOLD_MIN);
goto parse_error;
}
@@ -526,7 +537,8 @@
}
if (totem_config->rrp_token_expired_timeout < MINIMUM_TIMEOUT) {
- sprintf (local_error_reason, "The RRP token expired timeout parameter (%d ms) may not be less then (%d ms).",
+ snprintf (local_error_reason, sizeof(local_error_reason),
+ "The RRP token expired timeout parameter (%d ms) may not be less then (%d ms).",
totem_config->rrp_token_expired_timeout, MINIMUM_TIMEOUT);
goto parse_error;
}
@@ -535,7 +547,7 @@
interface_max = 1;
}
if (interface_max < totem_config->interface_count) {
- sprintf (parse_error,
+ snprintf (parse_error, sizeof(parse_error),
"%d is too many configured interfaces for the rrp_mode setting %s.",
totem_config->interface_count,
totem_config->rrp_mode);
@@ -555,7 +567,8 @@
}
if ((MESSAGE_QUEUE_MAX) < totem_config->max_messages) {
- sprintf (local_error_reason, "The max_messages parameter (%d messages) may not be greater then (%d messages).",
+ snprintf (local_error_reason, sizeof(local_error_reason),
+ "The max_messages parameter (%d messages) may not be greater then (%d messages).",
totem_config->max_messages, MESSAGE_QUEUE_MAX);
goto parse_error;
}
@@ -577,7 +590,7 @@
return (0);
parse_error:
- sprintf (error_string_response,
+ snprintf (error_string_response, sizeof(error_string_response),
"parse error in config: %s\n", error_reason);
*error_string = error_string_response;
return (-1);
@@ -593,7 +606,8 @@
fd = open (key_location, O_RDONLY);
if (fd == -1) {
- sprintf (error_string_response, "Could not open %s: %s\n",
+ snprintf (error_string_response, sizeof(error_string_response),
+ "Could not open %s: %s\n",
key_location, strerror (errno));
goto parse_error;
}
@@ -601,7 +615,8 @@
res = read (fd, totem_config->private_key, 128);
if (res == -1) {
close (fd);
- sprintf (error_string_response, "Could not read %s: %s\n",
+ snprintf (error_string_response, sizeof(error_string_response),
+ "Could not read %s: %s\n",
key_location, strerror (errno));
goto parse_error;
}
@@ -610,7 +625,8 @@
if (res != 128) {
close (fd);
- sprintf (error_string_response, "Could only read %d bits of 1024 bits from %s.\n",
+ snprintf (error_string_response, sizeof(error_string_response),
+ "Could only read %d bits of 1024 bits from %s.\n",
res * 8, key_location);
goto parse_error;
}
Index: exec/logsys.c
===================================================================
--- exec/logsys.c (revision 1987)
+++ exec/logsys.c (working copy)
@@ -907,7 +907,8 @@
logsys_close_logfile();
logsys_file_fp = fopen (file, "a+");
if (logsys_file_fp == 0) {
- sprintf (error_string_response,
+ snprintf (error_string_response,
+ sizeof(error_string_response),
"Can't open logfile '%s' for reason (%s).\n",
file, strerror (errno));
*error_string = error_string_response;
_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais
