Skip to content

Commit

Permalink
Fix integer overflow (#435)
Browse files Browse the repository at this point in the history
* Fix MAX_BUCKETS_MEM overflow

* Fix MAX_QUERY_BUF overflow

* Fix int overflow in IsBucketValid function

* Add missing newline

* Remove test for max value of pgsm_query_shared_buffer parameter

* Tune tests

* Cleanup

* Use int64 type instead of long long
  • Loading branch information
artemgavrilov authored Apr 5, 2024
1 parent 7ea569e commit 64c71f9
Show file tree
Hide file tree
Showing 10 changed files with 225 additions and 7 deletions.
2 changes: 1 addition & 1 deletion pg_stat_monitor.c
Original file line number Diff line number Diff line change
Expand Up @@ -1999,7 +1999,7 @@ IsBucketValid(uint64 bucketid)

TimestampDifference(pgsm->bucket_start_time[bucketid], current_tz, &secs, &microsecs);

if (secs > (pgsm_bucket_time * pgsm_max_buckets))
if (secs > ((int64)pgsm_bucket_time * pgsm_max_buckets))
return false;
return true;
}
Expand Down
4 changes: 2 additions & 2 deletions pg_stat_monitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@
/* the assumption of query max nested level */
#define DEFAULT_MAX_NESTED_LEVEL 10

#define MAX_QUERY_BUF (pgsm_query_shared_buffer * 1024 * 1024)
#define MAX_BUCKETS_MEM (pgsm_max * 1024 * 1024)
#define MAX_QUERY_BUF ((int64)pgsm_query_shared_buffer * 1024 * 1024)
#define MAX_BUCKETS_MEM ((int64)pgsm_max * 1024 * 1024)
#define BUCKETS_MEM_OVERFLOW() ((hash_get_num_entries(pgsm_hash) * sizeof(pgsmEntry)) >= MAX_BUCKETS_MEM)
#define MAX_BUCKET_ENTRIES (MAX_BUCKETS_MEM / sizeof(pgsmEntry))
#define QUERY_BUFFER_OVERFLOW(x,y) ((x + y + sizeof(uint64) + sizeof(uint64)) > MAX_QUERY_BUF)
Expand Down
25 changes: 24 additions & 1 deletion t/007_settings_pgsm_query_shared_buffer.pl
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
open my $conf, '>>', "$pgdata/postgresql.conf";
print $conf "shared_preload_libraries = 'pg_stat_monitor'\n";
print $conf "pg_stat_monitor.pgsm_bucket_time = 360000\n";
print $conf "pg_stat_monitor.pgsm_query_shared_buffer = 1\n";
print $conf "pg_stat_monitor.pgsm_query_shared_buffer = 1\n"; # Min possible value
print $conf "pg_stat_monitor.pgsm_normalized_query = 'yes'\n";
close $conf;

Expand Down Expand Up @@ -109,6 +109,29 @@
ok($cmdret == 0, "SELECT XXX FROM pg_stat_monitor");
PGSM::append_to_file($stdout);

$node->append_conf('postgresql.conf', "pg_stat_monitor.pgsm_query_shared_buffer = 2048\n");
$node->restart();

($cmdret, $stdout, $stderr) = $node->psql('postgres', 'SELECT pg_stat_monitor_reset();', extra_params => ['-a', '-Pformat=aligned','-Ptuples_only=off']);
ok($cmdret == 0, "Reset PGSM EXTENSION");
PGSM::append_to_file($stdout);

($cmdret, $stdout, $stderr) = $node->psql('postgres', "SELECT name, setting, unit, context, vartype, source, min_val, max_val, enumvals, boot_val, reset_val, pending_restart FROM pg_settings WHERE name='pg_stat_monitor.pgsm_query_shared_buffer';", extra_params => ['-a', '-Pformat=aligned','-Ptuples_only=off']);
ok($cmdret == 0, "Print PGSM EXTENSION Settings");
PGSM::append_to_file($stdout);

$out = system ("pgbench -i -s 10 -p $port example");
print " out: $out \n";
ok($cmdret == 0, "Perform pgbench init");

$out = system ("pgbench -c 10 -j 2 -t 1000 -p $port example");
print " out: $out \n";
ok($cmdret == 0, "Run pgbench");

($cmdret, $stdout, $stderr) = $node->psql('postgres', 'SELECT datname, substr(query,0,150) AS query, SUM(calls) AS calls FROM pg_stat_monitor GROUP BY datname, query ORDER BY datname, query, calls DESC Limit 20;', extra_params => ['-a', '-Pformat=aligned','-Ptuples_only=off']);
ok($cmdret == 0, "SELECT XXX FROM pg_stat_monitor");
PGSM::append_to_file($stdout);

# DROP EXTENSION
$stdout = $node->safe_psql('postgres', 'DROP EXTENSION pg_stat_monitor;', extra_params => ['-a']);
ok($cmdret == 0, "DROP PGSM EXTENSION");
Expand Down
4 changes: 2 additions & 2 deletions t/016_settings_pgsm_max.pl
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
# UPDATE postgresql.conf to include/load pg_stat_monitor library
open my $conf, '>>', "$pgdata/postgresql.conf";
print $conf "shared_preload_libraries = 'pg_stat_monitor'\n";
print $conf "pg_stat_monitor.pgsm_max = 1000\n";
print $conf "pg_stat_monitor.pgsm_max = 2048\n";
close $conf;

# Start server
Expand Down Expand Up @@ -62,7 +62,7 @@
ok($cmdret == 0, "Print PGSM EXTENSION Settings");
PGSM::append_to_file($stdout);

$node->append_conf('postgresql.conf', "pg_stat_monitor.pgsm_max = 10\n");
$node->append_conf('postgresql.conf', "pg_stat_monitor.pgsm_max = 10\n"); # Min possible value
$node->restart();

($cmdret, $stdout, $stderr) = $node->psql('postgres', 'SELECT pg_stat_monitor_reset();', extra_params => ['-a', '-Pformat=aligned','-Ptuples_only=off']);
Expand Down
63 changes: 63 additions & 0 deletions t/031_query_stat.pl
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
#!/usr/bin/perl

use strict;
use warnings;
use File::Basename;
use File::Compare;
use File::Copy;
use Test::More;
use lib 't';
use pgsm;

# Get file name and CREATE out file name and dirs WHERE requried
PGSM::setup_files_dir(basename($0));

# CREATE new PostgreSQL node and do initdb
my $node = PGSM->pgsm_init_pg();
my $pgdata = $node->data_dir;

# UPDATE postgresql.conf to include/load pg_stat_monitor library
open my $conf, '>>', "$pgdata/postgresql.conf";
print $conf "shared_preload_libraries = 'pg_stat_monitor'\n";
print $conf "pg_stat_monitor.pgsm_bucket_time = 2147483647\n"; # Max value for this parameter
print $conf "pg_stat_monitor.pgsm_max_buckets = 20000\n"; # Max value for this parameter
close $conf;

# Start server
my $rt_value = $node->start;
ok($rt_value == 1, "Start Server");

# CREATE EXTENSION and change out file permissions
my ($cmdret, $stdout, $stderr) = $node->psql('postgres', 'CREATE EXTENSION pg_stat_monitor;', extra_params => ['-a']);
ok($cmdret == 0, "CREATE PGSM EXTENSION");
PGSM::append_to_file($stdout);

# Run required commands/queries and dump output to out file.
($cmdret, $stdout, $stderr) = $node->psql('postgres', 'SELECT pg_stat_monitor_reset();', extra_params => ['-a', '-Pformat=aligned','-Ptuples_only=off']);
ok($cmdret == 0, "Reset PGSM EXTENSION");
PGSM::append_to_file($stdout);

($cmdret, $stdout, $stderr) = $node->psql('postgres', "SELECT 1 AS num;", extra_params => ['-a', '-Pformat=aligned','-Ptuples_only=off']);
ok($cmdret == 0, "Print results of a test query");
PGSM::append_to_file($stdout);

($cmdret, $stdout, $stderr) = $node->psql('postgres', 'SELECT query, comments FROM pg_stat_monitor ORDER BY query COLLATE "C";', extra_params => ['-a', '-Pformat=aligned','-Ptuples_only=off']);
ok($cmdret == 0, "Check query stats");
PGSM::append_to_file($stdout);

# DROP EXTENSION
$stdout = $node->safe_psql('postgres', 'DROP EXTENSION pg_stat_monitor;', extra_params => ['-a']);
ok($cmdret == 0, "DROP PGSM EXTENSION");
PGSM::append_to_file($stdout);

# Stop the server
$node->stop;

# compare the expected and out file
my $compare = PGSM->compare_results();

# Test/check if expected and result/out file match. If Yes, test passes.
is($compare,0,"Compare Files: $PGSM::expected_filename_with_path and $PGSM::out_filename_with_path files.");

# Done testing for this testcase file.
done_testing();
37 changes: 37 additions & 0 deletions t/expected/007_settings_pgsm_query_shared_buffer.out
Original file line number Diff line number Diff line change
Expand Up @@ -111,4 +111,41 @@ SELECT datname, substr(query,0,150) AS query, SUM(calls) AS calls FROM pg_stat_m
example | insert into pgbench_tellers(tid,bid,tbalance) values ($1,$2,$3) | 100
(20 rows)

SELECT pg_stat_monitor_reset();
pg_stat_monitor_reset
-----------------------

(1 row)

SELECT name, setting, unit, context, vartype, source, min_val, max_val, enumvals, boot_val, reset_val, pending_restart FROM pg_settings WHERE name='pg_stat_monitor.pgsm_query_shared_buffer';
name | setting | unit | context | vartype | source | min_val | max_val | enumvals | boot_val | reset_val | pending_restart
------------------------------------------+---------+------+------------+---------+--------------------+---------+---------+----------+----------+-----------+-----------------
pg_stat_monitor.pgsm_query_shared_buffer | 2048 | MB | postmaster | integer | configuration file | 1 | 10000 | | 20 | 2048 | f
(1 row)

SELECT datname, substr(query,0,150) AS query, SUM(calls) AS calls FROM pg_stat_monitor GROUP BY datname, query ORDER BY datname, query, calls DESC Limit 20;
datname | query | calls
---------+---------------------------------------------------------------------------------------------------------------+-------
example | BEGIN | 10000
example | END | 10000
example | INSERT INTO pgbench_history (tid, bid, aid, delta, mtime) VALUES ($1, $2, $3, $4, CURRENT_TIMESTAMP) | 10000
example | SELECT abalance FROM pgbench_accounts WHERE aid = $1 | 10000
example | UPDATE pgbench_accounts SET abalance = abalance + $1 WHERE aid = $2 | 10000
example | UPDATE pgbench_branches SET bbalance = bbalance + $1 WHERE bid = $2 | 10000
example | UPDATE pgbench_tellers SET tbalance = tbalance + $1 WHERE tid = $2 | 10000
example | alter table pgbench_accounts add primary key (aid) | 1
example | alter table pgbench_branches add primary key (bid) | 1
example | alter table pgbench_tellers add primary key (tid) | 1
example | begin | 1
example | commit | 1
example | copy pgbench_accounts from stdin | 1
example | create table pgbench_accounts(aid int not null,bid int,abalance int,filler char(84)) with (fillfactor=100) | 1
example | create table pgbench_branches(bid int not null,bbalance int,filler char(88)) with (fillfactor=100) | 1
example | create table pgbench_history(tid int,bid int,aid int,delta int,mtime timestamp,filler char(22)) | 1
example | create table pgbench_tellers(tid int not null,bid int,tbalance int,filler char(84)) with (fillfactor=100) | 1
example | drop table if exists pgbench_accounts, pgbench_branches, pgbench_history, pgbench_tellers | 1
example | insert into pgbench_branches(bid,bbalance) values($1,$2) | 10
example | insert into pgbench_tellers(tid,bid,tbalance) values ($1,$2,$3) | 100
(20 rows)

DROP EXTENSION pg_stat_monitor;
37 changes: 37 additions & 0 deletions t/expected/007_settings_pgsm_query_shared_buffer.out.15
Original file line number Diff line number Diff line change
Expand Up @@ -111,4 +111,41 @@ SELECT datname, substr(query,0,150) AS query, SUM(calls) AS calls FROM pg_stat_m
example | insert into pgbench_tellers(tid,bid,tbalance) values ($1,$2,$3) | 100
(20 rows)

SELECT pg_stat_monitor_reset();
pg_stat_monitor_reset
-----------------------

(1 row)

SELECT name, setting, unit, context, vartype, source, min_val, max_val, enumvals, boot_val, reset_val, pending_restart FROM pg_settings WHERE name='pg_stat_monitor.pgsm_query_shared_buffer';
name | setting | unit | context | vartype | source | min_val | max_val | enumvals | boot_val | reset_val | pending_restart
------------------------------------------+---------+------+------------+---------+--------------------+---------+---------+----------+----------+-----------+-----------------
pg_stat_monitor.pgsm_query_shared_buffer | 2048 | MB | postmaster | integer | configuration file | 1 | 10000 | | 20 | 2048 | f
(1 row)

SELECT datname, substr(query,0,150) AS query, SUM(calls) AS calls FROM pg_stat_monitor GROUP BY datname, query ORDER BY datname, query, calls DESC Limit 20;
datname | query | calls
---------+---------------------------------------------------------------------------------------------------------------+-------
example | BEGIN | 10000
example | END | 10000
example | INSERT INTO pgbench_history (tid, bid, aid, delta, mtime) VALUES ($1, $2, $3, $4, CURRENT_TIMESTAMP) | 10000
example | SELECT abalance FROM pgbench_accounts WHERE aid = $1 | 10000
example | UPDATE pgbench_accounts SET abalance = abalance + $1 WHERE aid = $2 | 10000
example | UPDATE pgbench_branches SET bbalance = bbalance + $1 WHERE bid = $2 | 10000
example | UPDATE pgbench_tellers SET tbalance = tbalance + $1 WHERE tid = $2 | 10000
example | alter table pgbench_accounts add primary key (aid) | 1
example | alter table pgbench_branches add primary key (bid) | 1
example | alter table pgbench_tellers add primary key (tid) | 1
example | begin | 1
example | commit | 1
example | copy pgbench_accounts from stdin with (freeze on) | 1
example | create table pgbench_accounts(aid int not null,bid int,abalance int,filler char(84)) with (fillfactor=100) | 1
example | create table pgbench_branches(bid int not null,bbalance int,filler char(88)) with (fillfactor=100) | 1
example | create table pgbench_history(tid int,bid int,aid int,delta int,mtime timestamp,filler char(22)) | 1
example | create table pgbench_tellers(tid int not null,bid int,tbalance int,filler char(84)) with (fillfactor=100) | 1
example | drop table if exists pgbench_accounts, pgbench_branches, pgbench_history, pgbench_tellers | 1
example | insert into pgbench_branches(bid,bbalance) values($1,$2) | 10
example | insert into pgbench_tellers(tid,bid,tbalance) values ($1,$2,$3) | 100
(20 rows)

DROP EXTENSION pg_stat_monitor;
37 changes: 37 additions & 0 deletions t/expected/007_settings_pgsm_query_shared_buffer.out.16
Original file line number Diff line number Diff line change
Expand Up @@ -111,4 +111,41 @@ SELECT datname, substr(query,0,150) AS query, SUM(calls) AS calls FROM pg_stat_m
example | select o.n, p.partstrat, pg_catalog.count(i.inhparent) from pg_catalog.pg_class as c join pg_catalog.pg_namespace as n on (n.oid = c.relnamespace) cr | 1
(20 rows)

SELECT pg_stat_monitor_reset();
pg_stat_monitor_reset
-----------------------

(1 row)

SELECT name, setting, unit, context, vartype, source, min_val, max_val, enumvals, boot_val, reset_val, pending_restart FROM pg_settings WHERE name='pg_stat_monitor.pgsm_query_shared_buffer';
name | setting | unit | context | vartype | source | min_val | max_val | enumvals | boot_val | reset_val | pending_restart
------------------------------------------+---------+------+------------+---------+--------------------+---------+---------+----------+----------+-----------+-----------------
pg_stat_monitor.pgsm_query_shared_buffer | 2048 | MB | postmaster | integer | configuration file | 1 | 10000 | | 20 | 2048 | f
(1 row)

SELECT datname, substr(query,0,150) AS query, SUM(calls) AS calls FROM pg_stat_monitor GROUP BY datname, query ORDER BY datname, query, calls DESC Limit 20;
datname | query | calls
---------+-------------------------------------------------------------------------------------------------------------------------------------------------------+-------
example | INSERT INTO pgbench_history (tid, bid, aid, delta, mtime) VALUES ($1, $2, $3, $4, CURRENT_TIMESTAMP) | 10000
example | SELECT abalance FROM pgbench_accounts WHERE aid = $1 | 10000
example | UPDATE pgbench_accounts SET abalance = abalance + $1 WHERE aid = $2 | 10000
example | UPDATE pgbench_branches SET bbalance = bbalance + $1 WHERE bid = $2 | 10000
example | UPDATE pgbench_tellers SET tbalance = tbalance + $1 WHERE tid = $2 | 10000
example | alter table pgbench_accounts add primary key (aid) | 1
example | alter table pgbench_branches add primary key (bid) | 1
example | alter table pgbench_tellers add primary key (tid) | 1
example | begin | 10001
example | commit | 10001
example | copy pgbench_accounts from stdin with (freeze on) | 1
example | create table pgbench_accounts(aid int not null,bid int,abalance int,filler char(84)) with (fillfactor=100) | 1
example | create table pgbench_branches(bid int not null,bbalance int,filler char(88)) with (fillfactor=100) | 1
example | create table pgbench_history(tid int,bid int,aid int,delta int,mtime timestamp,filler char(22)) | 1
example | create table pgbench_tellers(tid int not null,bid int,tbalance int,filler char(84)) with (fillfactor=100) | 1
example | drop table if exists pgbench_accounts, pgbench_branches, pgbench_history, pgbench_tellers | 1
example | insert into pgbench_branches(bid,bbalance) values($1,$2) | 10
example | insert into pgbench_tellers(tid,bid,tbalance) values ($1,$2,$3) | 100
example | select count(*) from pgbench_branches | 1
example | select o.n, p.partstrat, pg_catalog.count(i.inhparent) from pg_catalog.pg_class as c join pg_catalog.pg_namespace as n on (n.oid = c.relnamespace) cr | 1
(20 rows)

DROP EXTENSION pg_stat_monitor;
2 changes: 1 addition & 1 deletion t/expected/016_settings_pgsm_max.out
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ SELECT pg_stat_monitor_reset();
SELECT name, setting, unit, context, vartype, source, min_val, max_val, enumvals, boot_val, reset_val, pending_restart FROM pg_settings WHERE name='pg_stat_monitor.pgsm_max';
name | setting | unit | context | vartype | source | min_val | max_val | enumvals | boot_val | reset_val | pending_restart
--------------------------+---------+------+------------+---------+--------------------+---------+---------+----------+----------+-----------+-----------------
pg_stat_monitor.pgsm_max | 1000 | MB | postmaster | integer | configuration file | 10 | 10240 | | 256 | 1000 | f
pg_stat_monitor.pgsm_max | 2048 | MB | postmaster | integer | configuration file | 10 | 10240 | | 256 | 2048 | f
(1 row)

SELECT pg_stat_monitor_reset();
Expand Down
Loading

0 comments on commit 64c71f9

Please sign in to comment.