Fix two NULL pointer dereferences when reading custom pgstats from file

michaelpq · michaelpq · commit 341e9a05e7b4 · 2024-09-05T14:36:57.000+09:00
There were two spots in pgstat_read_statsfile() where is was possible to finish with a null-pointer-dereference crash for custom pgstats kinds: - When reading stats for a fixed-numbered stats entry. - When reading a variable stats entry with name serialization. For both cases, these issues were reachable by starting a server after changing shared_preload_libraries so as the stats written previously could not be loaded. The code is changed so as the stats are ignored in this case, like the other code paths doing similar sanity checks. Two WARNINGs are added to be able to debug these issues. A test is added for the case of fixed-numbered stats with the module injection_points. Oversights in 7949d95, spotted while looking at a different report. Discussion: https://postgr.es/m/Ztj0Jftsn4xXuXtl@paquier.xyz
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
@@ -1781,6 +1781,12 @@ pgstat_read_statsfile(XLogRecPtr redo)
 					}
 
 					info = pgstat_get_kind_info(kind);
+					if (!info)
+					{
+						elog(WARNING, "could not find information of kind %u for entry of type %c",
+							 kind, t);
+						goto error;
+					}
 
 					if (!info->fixed_amount)
 					{
@@ -1861,6 +1867,12 @@ pgstat_read_statsfile(XLogRecPtr redo)
 						}
 
 						kind_info = pgstat_get_kind_info(kind);
+						if (!kind_info)
+						{
+							elog(WARNING, "could not find information of kind %u for entry of type %c",
+								 kind, t);
+							goto error;
+						}
 
 						if (!kind_info->from_serialized_name)
 						{
diff --git a/src/test/modules/injection_points/t/001_stats.pl b/src/test/modules/injection_points/t/001_stats.pl
@@ -69,4 +69,10 @@
 	"SELECT * FROM injection_points_stats_fixed();");
 is($fixedstats, '0|0|0|0|0', 'fixed stats after crash');
 
+# Stop the server, disable the module, then restart.  The server
+# should be able to come up.
+$node->stop;
+$node->adjust_conf('postgresql.conf', 'shared_preload_libraries', "''");
+$node->start;
+
 done_testing();

Original file line number	Diff line number	Diff line change
`@@ -1781,6 +1781,12 @@ pgstat_read_statsfile(XLogRecPtr redo)`
`1781`	`1781`	`}`
`1782`	`1782`
`1783`	`1783`	`info = pgstat_get_kind_info(kind);`
	`1784`	`+ if (!info)`
	`1785`	`+ {`
	`1786`	`+ elog(WARNING, "could not find information of kind %u for entry of type %c",`
	`1787`	`+ kind, t);`
	`1788`	`+ goto error;`
	`1789`	`+ }`
`1784`	`1790`
`1785`	`1791`	`if (!info->fixed_amount)`
`1786`	`1792`	`{`
`@@ -1861,6 +1867,12 @@ pgstat_read_statsfile(XLogRecPtr redo)`
`1861`	`1867`	`}`
`1862`	`1868`
`1863`	`1869`	`kind_info = pgstat_get_kind_info(kind);`
	`1870`	`+ if (!kind_info)`
	`1871`	`+ {`
	`1872`	`+ elog(WARNING, "could not find information of kind %u for entry of type %c",`
	`1873`	`+ kind, t);`
	`1874`	`+ goto error;`
	`1875`	`+ }`
`1864`	`1876`
`1865`	`1877`	`if (!kind_info->from_serialized_name)`
`1866`	`1878`	`{`