Skip to content

Commit 23fa9a3

Browse files
za-arthurza-arthur
committed
PGPRO-2032: Fix log-rotation-size and log-rotation-age integer overflow
1 parent ca43526 commit 23fa9a3

File tree

6 files changed

+56
-41
lines changed

6 files changed

+56
-41
lines changed

‎src/configure.c‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -161,12 +161,12 @@ writeBackupCatalogConfig(FILE *out, pgBackupConfig *config)
161161
if (config->master_user)
162162
fprintf(out, "master-user = %s\n", config->master_user);
163163

164-
convert_from_base_unit_u(config->replica_timeout, OPTION_UNIT_S,
164+
convert_from_base_unit_u(config->replica_timeout, OPTION_UNIT_MS,
165165
&res, &unit);
166166
fprintf(out, "replica-timeout = " UINT64_FORMAT "%s\n", res, unit);
167167

168168
fprintf(out, "#Archive parameters:\n");
169-
convert_from_base_unit_u(config->archive_timeout, OPTION_UNIT_S,
169+
convert_from_base_unit_u(config->archive_timeout, OPTION_UNIT_MS,
170170
&res, &unit);
171171
fprintf(out, "archive-timeout = " UINT64_FORMAT "%s\n", res, unit);
172172

@@ -183,11 +183,11 @@ writeBackupCatalogConfig(FILE *out, pgBackupConfig *config)
183183
fprintf(out, "log-directory = %s\n", config->log_directory);
184184
/* Convert values from base unit */
185185
convert_from_base_unit_u(config->log_rotation_size, OPTION_UNIT_KB,
186-
&res, &unit);
186+
&res, &unit);
187187
fprintf(out, "log-rotation-size = " UINT64_FORMAT "%s\n", res, (res)?unit:"KB");
188188

189-
convert_from_base_unit_u(config->log_rotation_age, OPTION_UNIT_S,
190-
&res, &unit);
189+
convert_from_base_unit_u(config->log_rotation_age, OPTION_UNIT_MS,
190+
&res, &unit);
191191
fprintf(out, "log-rotation-age = " UINT64_FORMAT "%s\n", res, (res)?unit:"min");
192192

193193
fprintf(out, "#Retention parameters:\n");
@@ -237,8 +237,8 @@ readBackupCatalogConfigFile(void)
237237
{ 's', 0, "log-filename", &(config->log_filename), SOURCE_CMDLINE },
238238
{ 's', 0, "error-log-filename", &(config->error_log_filename), SOURCE_CMDLINE },
239239
{ 's', 0, "log-directory", &(config->log_directory), SOURCE_CMDLINE },
240-
{ 'u', 0, "log-rotation-size", &(config->log_rotation_size), SOURCE_CMDLINE, SOURCE_DEFAULT, OPTION_UNIT_KB },
241-
{ 'u', 0, "log-rotation-age", &(config->log_rotation_age), SOURCE_CMDLINE, SOURCE_DEFAULT, OPTION_UNIT_S },
240+
{ 'U', 0, "log-rotation-size", &(config->log_rotation_size), SOURCE_CMDLINE, SOURCE_DEFAULT, OPTION_UNIT_KB },
241+
{ 'U', 0, "log-rotation-age", &(config->log_rotation_age), SOURCE_CMDLINE, SOURCE_DEFAULT, OPTION_UNIT_MS },
242242
/* connection options */
243243
{ 's', 0, "pgdata", &(config->pgdata), SOURCE_FILE_STRICT },
244244
{ 's', 0, "pgdatabase", &(config->pgdatabase), SOURCE_FILE_STRICT },
@@ -250,11 +250,11 @@ readBackupCatalogConfigFile(void)
250250
{ 's', 0, "master-port", &(config->master_port), SOURCE_FILE_STRICT },
251251
{ 's', 0, "master-db", &(config->master_db), SOURCE_FILE_STRICT },
252252
{ 's', 0, "master-user", &(config->master_user), SOURCE_FILE_STRICT },
253-
{ 'u', 0, "replica-timeout", &(config->replica_timeout), SOURCE_CMDLINE, SOURCE_DEFAULT, OPTION_UNIT_S },
253+
{ 'u', 0, "replica-timeout", &(config->replica_timeout), SOURCE_CMDLINE, SOURCE_DEFAULT, OPTION_UNIT_MS },
254254
/* other options */
255255
{ 'U', 0, "system-identifier", &(config->system_identifier), SOURCE_FILE_STRICT },
256256
/* archive options */
257-
{ 'u', 0, "archive-timeout", &(config->archive_timeout), SOURCE_CMDLINE, SOURCE_DEFAULT, OPTION_UNIT_S },
257+
{ 'u', 0, "archive-timeout", &(config->archive_timeout), SOURCE_CMDLINE, SOURCE_DEFAULT, OPTION_UNIT_MS },
258258
{0}
259259
};
260260

@@ -373,13 +373,13 @@ show_configure_json(pgBackupConfig *config)
373373
true);
374374

375375
json_add_key(buf, "replica-timeout", json_level, true);
376-
convert_from_base_unit_u(config->replica_timeout, OPTION_UNIT_S,
376+
convert_from_base_unit_u(config->replica_timeout, OPTION_UNIT_MS,
377377
&res, &unit);
378378
appendPQExpBuffer(buf, UINT64_FORMAT "%s", res, unit);
379379

380380
/* Archive parameters */
381381
json_add_key(buf, "archive-timeout", json_level, true);
382-
convert_from_base_unit_u(config->archive_timeout, OPTION_UNIT_S,
382+
convert_from_base_unit_u(config->archive_timeout, OPTION_UNIT_MS,
383383
&res, &unit);
384384
appendPQExpBuffer(buf, UINT64_FORMAT "%s", res, unit);
385385

@@ -416,7 +416,7 @@ show_configure_json(pgBackupConfig *config)
416416
appendPQExpBuffer(buf, UINT64_FORMAT "%s", res, (res)?unit:"KB");
417417

418418
json_add_key(buf, "log-rotation-age", json_level, true);
419-
convert_from_base_unit_u(config->log_rotation_age, OPTION_UNIT_S,
419+
convert_from_base_unit_u(config->log_rotation_age, OPTION_UNIT_MS,
420420
&res, &unit);
421421
appendPQExpBuffer(buf, UINT64_FORMAT "%s", res, (res)?unit:"min");
422422

‎src/pg_probackup.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,14 +131,14 @@ static pgut_option options[] =
131131
{ 'f', 'b', "backup-mode", opt_backup_mode, SOURCE_CMDLINE },
132132
{ 'b', 'C', "smooth-checkpoint", &smooth_checkpoint, SOURCE_CMDLINE },
133133
{ 's', 'S', "slot", &replication_slot, SOURCE_CMDLINE },
134-
{ 'u', 11, "archive-timeout", &archive_timeout, SOURCE_CMDLINE, SOURCE_DEFAULT, OPTION_UNIT_S },
134+
{ 'u', 11, "archive-timeout", &archive_timeout, SOURCE_CMDLINE, SOURCE_DEFAULT, OPTION_UNIT_MS },
135135
{ 'b', 12, "delete-wal", &delete_wal, SOURCE_CMDLINE },
136136
{ 'b', 13, "delete-expired", &delete_expired, SOURCE_CMDLINE },
137137
{ 's', 14, "master-db", &master_db, SOURCE_CMDLINE, },
138138
{ 's', 15, "master-host", &master_host, SOURCE_CMDLINE, },
139139
{ 's', 16, "master-port", &master_port, SOURCE_CMDLINE, },
140140
{ 's', 17, "master-user", &master_user, SOURCE_CMDLINE, },
141-
{ 'u', 18, "replica-timeout", &replica_timeout, SOURCE_CMDLINE, SOURCE_DEFAULT, OPTION_UNIT_S },
141+
{ 'u', 18, "replica-timeout", &replica_timeout, SOURCE_CMDLINE, SOURCE_DEFAULT, OPTION_UNIT_MS },
142142
/* TODO not completed feature. Make it unavailiable from user level
143143
{ 'b', 18, "remote", &is_remote_backup, SOURCE_CMDLINE, }, */
144144
/* restore options */
@@ -172,8 +172,8 @@ static pgut_option options[] =
172172
{ 's', 142, "log-filename", &log_filename, SOURCE_CMDLINE },
173173
{ 's', 143, "error-log-filename", &error_log_filename, SOURCE_CMDLINE },
174174
{ 's', 144, "log-directory", &log_directory, SOURCE_CMDLINE },
175-
{ 'u', 145, "log-rotation-size", &log_rotation_size, SOURCE_CMDLINE, SOURCE_DEFAULT, OPTION_UNIT_KB },
176-
{ 'u', 146, "log-rotation-age", &log_rotation_age, SOURCE_CMDLINE, SOURCE_DEFAULT, OPTION_UNIT_MIN },
175+
{ 'U', 145, "log-rotation-size", &log_rotation_size, SOURCE_CMDLINE, SOURCE_DEFAULT, OPTION_UNIT_KB },
176+
{ 'U', 146, "log-rotation-age", &log_rotation_age, SOURCE_CMDLINE, SOURCE_DEFAULT, OPTION_UNIT_MS },
177177
/* connection options */
178178
{ 's', 'd', "pgdatabase", &pgut_dbname, SOURCE_CMDLINE },
179179
{ 's', 'h', "pghost", &host, SOURCE_CMDLINE },

‎src/pg_probackup.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,8 @@ typedef struct pgBackupConfig
191191
char *log_filename;
192192
char *error_log_filename;
193193
char *log_directory;
194-
int log_rotation_size;
195-
int log_rotation_age;
194+
uint64 log_rotation_size;
195+
uint64 log_rotation_age;
196196

197197
uint32 retention_redundancy;
198198
uint32 retention_window;

‎src/utils/logger.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@ char *log_directory = NULL;
3232
char log_path[MAXPGPATH] = "";
3333

3434
/* Maximum size of an individual log file in kilobytes */
35-
int log_rotation_size = 0;
35+
uint64 log_rotation_size = 0;
3636
/* Maximum lifetime of an individual log file in minutes */
37-
int log_rotation_age = 0;
37+
uint64 log_rotation_age = 0;
3838

3939
/* Implementation for logging.h */
4040

‎src/utils/logger.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ extern char log_path[MAXPGPATH];
3636

3737
#define LOG_ROTATION_SIZE_DEFAULT 0
3838
#define LOG_ROTATION_AGE_DEFAULT 0
39-
extern int log_rotation_size;
40-
extern int log_rotation_age;
39+
extern uint64 log_rotation_size;
40+
extern uint64 log_rotation_age;
4141

4242
#define LOG_LEVEL_CONSOLE_DEFAULT INFO
4343
#define LOG_LEVEL_FILE_DEFAULT LOG_OFF

‎src/utils/pgut.c‎

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,6 @@
1919
#include "logger.h"
2020
#include "pgut.h"
2121

22-
/* old gcc doesn't have LLONG_MAX. */
23-
#ifndef LLONG_MAX
24-
#if defined(HAVE_LONG_INT_64) || !defined(HAVE_LONG_LONG_INT_64)
25-
#define LLONG_MAX LONG_MAX
26-
#else
27-
#define LLONG_MAX INT64CONST(0x7FFFFFFFFFFFFFFF)
28-
#endif
29-
#endif
30-
3122
#define MAX_TZDISP_HOUR 15 /* maximum allowed hour part */
3223
#define SECS_PER_MINUTE 60
3324
#define MINS_PER_HOUR 60
@@ -303,7 +294,13 @@ convert_to_base_unit(int64 value, const char *unit,
303294
if (table[i].multiplier < 0)
304295
*base_value = value / (-table[i].multiplier);
305296
else
297+
{
298+
/* Check for integer overflow first */
299+
if (value > PG_INT64_MAX / table[i].multiplier)
300+
return false;
301+
306302
*base_value = value * table[i].multiplier;
303+
}
307304
return true;
308305
}
309306
}
@@ -333,7 +330,13 @@ convert_to_base_unit_u(uint64 value, const char *unit,
333330
if (table[i].multiplier < 0)
334331
*base_value = value / (-table[i].multiplier);
335332
else
333+
{
334+
/* Check for integer overflow first */
335+
if (value > PG_UINT64_MAX / table[i].multiplier)
336+
return false;
337+
336338
*base_value = value * table[i].multiplier;
339+
}
337340
return true;
338341
}
339342
}
@@ -371,6 +374,10 @@ convert_from_base_unit(int64 base_value, int base_unit,
371374
*/
372375
if (table[i].multiplier < 0)
373376
{
377+
/* Check for integer overflow first */
378+
if (base_value > PG_INT64_MAX / (-table[i].multiplier))
379+
continue;
380+
374381
*value = base_value * (-table[i].multiplier);
375382
*unit = table[i].unit;
376383
break;
@@ -415,6 +422,10 @@ convert_from_base_unit_u(uint64 base_value, int base_unit,
415422
*/
416423
if (table[i].multiplier < 0)
417424
{
425+
/* Check for integer overflow first */
426+
if (base_value > PG_UINT64_MAX / (-table[i].multiplier))
427+
continue;
428+
418429
*value = base_value * (-table[i].multiplier);
419430
*unit = table[i].unit;
420431
break;
@@ -612,7 +623,7 @@ parse_int32(const char *value, int32 *result, int flags)
612623

613624
if (strcmp(value, INFINITE_STR) == 0)
614625
{
615-
*result = INT_MAX;
626+
*result = PG_INT32_MAX;
616627
return true;
617628
}
618629

@@ -621,12 +632,17 @@ parse_int32(const char *value, int32 *result, int flags)
621632
if (endptr == value || (*endptr && flags == 0))
622633
return false;
623634

635+
/* Check for integer overflow */
624636
if (errno == ERANGE || val != (int64) ((int32) val))
625637
return false;
626638

627639
if (!parse_unit(endptr, flags, val, &val))
628640
return false;
629641

642+
/* Check for integer overflow again */
643+
if (val != (int64) ((int32) val))
644+
return false;
645+
630646
*result = val;
631647

632648
return true;
@@ -644,7 +660,7 @@ parse_uint32(const char *value, uint32 *result, int flags)
644660

645661
if (strcmp(value, INFINITE_STR) == 0)
646662
{
647-
*result = UINT_MAX;
663+
*result = PG_UINT32_MAX;
648664
return true;
649665
}
650666

@@ -653,12 +669,17 @@ parse_uint32(const char *value, uint32 *result, int flags)
653669
if (endptr == value || (*endptr && flags == 0))
654670
return false;
655671

672+
/* Check for integer overflow */
656673
if (errno == ERANGE || val != (uint64) ((uint32) val))
657674
return false;
658675

659676
if (!parse_unit_u(endptr, flags, val, &val))
660677
return false;
661678

679+
/* Check for integer overflow again */
680+
if (val != (uint64) ((uint32) val))
681+
return false;
682+
662683
*result = val;
663684

664685
return true;
@@ -676,7 +697,7 @@ parse_int64(const char *value, int64 *result, int flags)
676697

677698
if (strcmp(value, INFINITE_STR) == 0)
678699
{
679-
*result = LLONG_MAX;
700+
*result = PG_INT64_MAX;
680701
return true;
681702
}
682703

@@ -714,13 +735,7 @@ parse_uint64(const char *value, uint64 *result, int flags)
714735

715736
if (strcmp(value, INFINITE_STR) == 0)
716737
{
717-
#if defined(HAVE_LONG_INT_64)
718-
*result = ULONG_MAX;
719-
#elif defined(HAVE_LONG_LONG_INT_64)
720-
*result = ULLONG_MAX;
721-
#else
722-
*result = ULONG_MAX;
723-
#endif
738+
*result = PG_UINT64_MAX;
724739
return true;
725740
}
726741

0 commit comments

Comments
 (0)