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

Commit6101bc2

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 parentefc4b48 commit6101bc2

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
@@ -4748,7 +4748,7 @@ get_dbstat_filename(bool permanent, bool tempname, Oid databaseid,
47484748
pgstat_stat_directory,
47494749
databaseid,
47504750
tempname ?"tmp" :"stat");
4751-
if (printed>len)
4751+
if (printed >=len)
47524752
elog(ERROR,"overlength pgstat path");
47534753
}
47544754

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

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

9164+
errno=0;
9165+
91649166
va_start(vargs,fmt);
91659167
n=vsnprintf(*destptr,*maxbytes,fmt,vargs);
91669168
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
@@ -404,7 +404,7 @@ getnameinfo(const struct sockaddr *sa, int salen,
404404
ret=snprintf(service,servicelen,"%d",
405405
ntohs(((structsockaddr_in*)sa)->sin_port));
406406
}
407-
if (ret==-1||ret >=servicelen)
407+
if (ret<0||ret >=servicelen)
408408
returnEAI_MEMORY;
409409
}
410410

‎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
@@ -1023,7 +1023,7 @@ config_sspi_auth(const char *pgdata)
10231023
} while (0)
10241024

10251025
res=snprintf(fname,sizeof(fname),"%s/pg_hba.conf",pgdata);
1026-
if (res<0||res >=sizeof(fname)-1)
1026+
if (res<0||res >=sizeof(fname))
10271027
{
10281028
/*
10291029
* 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