Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitcc4f6b7

Browse files
committed
Clean up assorted misuses of snprintf()'s result value.
Fix a small number of places that were testing the result of snprintf()but doing so incorrectly. The right test for buffer overrun, per C99,is "result >= bufsize" not "result > bufsize". Some places were alsochecking for failure with "result == -1", but the standard only saysthat a negative value is delivered on failure.(Note that this only makes these places correct if snprintf() deliversC99-compliant results. But at least now these places are consistentwith all the other places where we assume that.)Also, make psql_start_test() and isolation_start_test() check forbuffer overrun while constructing their shell commands. There seemslike a higher risk of overrun, with more severe consequences, herethan there is for the individual file paths that are made elsewherein the same functions, so this seemed like a worthwhile change.Also fix guc.c's do_serialize() to initialize errno = 0 beforecalling vsnprintf. In principle, this should be unnecessary becausevsnprintf should have set errno if it returns a failure indication ...but the other two places this coding pattern is cribbed from don'tassume that, so let's be consistent.These errors are all very old, so back-patch as appropriate. I thinkthat only the shell command overrun cases are even theoreticallyreachable in practice, but there's not much point in erroneous errorchecks.Discussion:https://postgr.es/m/17245.1534289329@sss.pgh.pa.us
1 parent805889d commitcc4f6b7

File tree

8 files changed

+47
-21
lines changed

8 files changed

+47
-21
lines changed

‎src/backend/postmaster/pgstat.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4810,7 +4810,7 @@ get_dbstat_filename(bool permanent, bool tempname, Oid databaseid,
48104810
pgstat_stat_directory,
48114811
databaseid,
48124812
tempname ?"tmp" :"stat");
4813-
if (printed>len)
4813+
if (printed >=len)
48144814
elog(ERROR,"overlength pgstat path");
48154815
}
48164816

‎src/backend/utils/misc/guc.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9441,6 +9441,8 @@ do_serialize(char **destptr, Size *maxbytes, const char *fmt,...)
94419441
if (*maxbytes <=0)
94429442
elog(ERROR,"not enough space to serialize GUC state");
94439443

9444+
errno=0;
9445+
94449446
va_start(vargs,fmt);
94459447
n=vsnprintf(*destptr,*maxbytes,fmt,vargs);
94469448
va_end(vargs);

‎src/common/ip.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ getnameinfo_unix(const struct sockaddr_un *sa, int salen,
233233
char*service,intservicelen,
234234
intflags)
235235
{
236-
intret=-1;
236+
intret;
237237

238238
/* Invalid arguments. */
239239
if (sa==NULL||sa->sun_family!=AF_UNIX||
@@ -243,14 +243,14 @@ getnameinfo_unix(const struct sockaddr_un *sa, int salen,
243243
if (node)
244244
{
245245
ret=snprintf(node,nodelen,"%s","[local]");
246-
if (ret==-1||ret>nodelen)
246+
if (ret<0||ret >=nodelen)
247247
returnEAI_MEMORY;
248248
}
249249

250250
if (service)
251251
{
252252
ret=snprintf(service,servicelen,"%s",sa->sun_path);
253-
if (ret==-1||ret>servicelen)
253+
if (ret<0||ret >=servicelen)
254254
returnEAI_MEMORY;
255255
}
256256

‎src/interfaces/ecpg/pgtypeslib/common.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ pgtypes_fmt_replace(union un_fmt_comb replace_val, int replace_type, char **outp
110110
break;
111111
}
112112

113-
if (i<0)
113+
if (i<0||i >=PGTYPES_FMT_NUM_MAX_DIGITS)
114114
{
115115
free(t);
116116
return-1;

‎src/port/getaddrinfo.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ getnameinfo(const struct sockaddr *sa, int salen,
405405
ret=snprintf(service,servicelen,"%d",
406406
pg_ntoh16(((structsockaddr_in*)sa)->sin_port));
407407
}
408-
if (ret==-1||ret >=servicelen)
408+
if (ret<0||ret >=servicelen)
409409
returnEAI_MEMORY;
410410
}
411411

‎src/test/isolation/isolation_main.c

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,15 +75,27 @@ isolation_start_test(const char *testname,
7575
add_stringlist_item(expectfiles,expectfile);
7676

7777
if (launcher)
78+
{
7879
offset+=snprintf(psql_cmd+offset,sizeof(psql_cmd)-offset,
7980
"%s ",launcher);
81+
if (offset >=sizeof(psql_cmd))
82+
{
83+
fprintf(stderr,_("command too long\n"));
84+
exit(2);
85+
}
86+
}
8087

81-
snprintf(psql_cmd+offset,sizeof(psql_cmd)-offset,
82-
"\"%s\" \"dbname=%s\" < \"%s\" > \"%s\" 2>&1",
83-
isolation_exec,
84-
dblist->str,
85-
infile,
86-
outfile);
88+
offset+=snprintf(psql_cmd+offset,sizeof(psql_cmd)-offset,
89+
"\"%s\" \"dbname=%s\" < \"%s\" > \"%s\" 2>&1",
90+
isolation_exec,
91+
dblist->str,
92+
infile,
93+
outfile);
94+
if (offset >=sizeof(psql_cmd))
95+
{
96+
fprintf(stderr,_("command too long\n"));
97+
exit(2);
98+
}
8799

88100
pid=spawn_process(psql_cmd);
89101

‎src/test/regress/pg_regress.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1024,7 +1024,7 @@ config_sspi_auth(const char *pgdata)
10241024
} while (0)
10251025

10261026
res=snprintf(fname,sizeof(fname),"%s/pg_hba.conf",pgdata);
1027-
if (res<0||res >=sizeof(fname)-1)
1027+
if (res<0||res >=sizeof(fname))
10281028
{
10291029
/*
10301030
* Truncating this name is a fatal error, because we must not fail to

‎src/test/regress/pg_regress_main.c

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -63,20 +63,32 @@ psql_start_test(const char *testname,
6363
add_stringlist_item(expectfiles,expectfile);
6464

6565
if (launcher)
66+
{
6667
offset+=snprintf(psql_cmd+offset,sizeof(psql_cmd)-offset,
6768
"%s ",launcher);
69+
if (offset >=sizeof(psql_cmd))
70+
{
71+
fprintf(stderr,_("command too long\n"));
72+
exit(2);
73+
}
74+
}
75+
76+
offset+=snprintf(psql_cmd+offset,sizeof(psql_cmd)-offset,
77+
"\"%s%spsql\" -X -a -q -d \"%s\" < \"%s\" > \"%s\" 2>&1",
78+
bindir ?bindir :"",
79+
bindir ?"/" :"",
80+
dblist->str,
81+
infile,
82+
outfile);
83+
if (offset >=sizeof(psql_cmd))
84+
{
85+
fprintf(stderr,_("command too long\n"));
86+
exit(2);
87+
}
6888

6989
appnameenv=psprintf("PGAPPNAME=pg_regress/%s",testname);
7090
putenv(appnameenv);
7191

72-
snprintf(psql_cmd+offset,sizeof(psql_cmd)-offset,
73-
"\"%s%spsql\" -X -a -q -d \"%s\" < \"%s\" > \"%s\" 2>&1",
74-
bindir ?bindir :"",
75-
bindir ?"/" :"",
76-
dblist->str,
77-
infile,
78-
outfile);
79-
8092
pid=spawn_process(psql_cmd);
8193

8294
if (pid==INVALID_PID)

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp