From: Tom Lane <tgl@sss.pgh.pa.us>
To: Robert Haas <robertmhaas@gmail.com>
cc: Amul Sul <sulamul@gmail.com>, Sravan Kumar <sravanvcybage@gmail.com>,
        PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: pg_verifybackup: TAR format backup verification
In-reply-to: <1115881.1727564342@sss.pgh.pa.us>
References: 
 <CAAJ_b94StvLWrc_p4q-f7n3OPfr6GhL8_XuAg2aAaYZp1tF-nw@mail.gmail.com>
 <CA+=NbjjkFADANQoyvgTQfKH8ZerxpmsJD70R17V8ZtfaoyRYwg@mail.gmail.com>
 <CAAJ_b96xU5fOOvWhJyMvvDCirJEZZdjieZZbyf0PnYukFQCy+Q@mail.gmail.com>
 <CA+Tgmoa0FVRbC9EG5GDaXORqMsoj8AnEDV4FVXwbLENKqwQqaA@mail.gmail.com>
 <CAAJ_b96Gwh=6zwpqsD-sVNOeYcU=9rfBygVtuXmU3qFAiNr2Hw@mail.gmail.com>
 <CA+Tgmobmi7qiu8HDQECwuy-JrAH0zcYbzGnhnKjurqv44DHtxg@mail.gmail.com>
 <CAAJ_b94sxUN73kNZ_Z5sP0GDEZGAxKgW0PvS5CBJXiH212+Fww@mail.gmail.com>
 <CA+Tgmoboyugvh0C-QWfzQK0nNgOHrxGnQQiOROjaa--+Aqr2xg@mail.gmail.com>
 <CAAJ_b975Pyo53P-zvkTPjVFErEANuRP9+K=2Qnifiy6BJBtwfw@mail.gmail.com>
 <CA+Tgmobt6=v-jE4iJ2kBGQ=jfubX_7fdOe4Uf9TWhKn+cfjGfw@mail.gmail.com>
 <CAAJ_b95rXAsyuetre9Wc-W6uHW-=ivVj56F8-w4gfrTGsnRRSA@mail.gmail.com>
 <CA+TgmoZw9dc6w-FFFJvuFMEfB11c+tDRyW+BjWD89+NG8bV=iA@mail.gmail.com>
 <CAAJ_b95mcGjkfAf1qduOR97CokW8-_i-dWLm3v6x1w2-OW9M+A@mail.gmail.com>
 <CA+TgmoZTJzUyzDkdo6K7TTM2ePP0EMXTwxGhJN1Ymx3qNHvevQ@mail.gmail.com>
 <CAAJ_b94MR!
 ! uQbNRa0DSgQM1r8sZO+S6nw58fheUORrVy76T=cCA@mail.gmail.com>
 <CA+TgmoYOQex-MuAfh+Fe1dRKT+O=nH+fPYJ6bKpHfUxgZpumew@mail.gmail.com>
 <CA+TgmoZnG5DT1jo=9xgFztiLX4tU7+wr3D9HbhH_vPc+C5M0iA@mail.gmail.com>
 <CA+TgmoYkXG1mvc4kc=fid8Sn-k4M1a4T_m4nN1_1Ed9afKg2KQ@mail.gmail.com>
 <CAAJ_b95e9oNpQ+EdLgKX+aLJXwpWAy2EZsLBFSB9=XSPs4hhFg@mail.gmail.com>
 <CAAJ_b972EmWT+a63wtTAwfGF3QpYZkzwov5UPh=JPmgJY=dW8A@mail.gmail.com>
 <CA+TgmobUiaquaz7CTzDDpJu5e6XYuvxEc4e+HXEvUqn7PD4byw@mail.gmail.com>
 <CAAJ_b976LwbmzJLDLTw6ScAVbH7mtxWhj12MWtyRYSkCSjOcfw@mail.gmail.com>
 <CA+TgmoZ-fwQ0aWUs_40oXkEpF7JAn4D9SnKj8dj=qGLtO0VSRw@mail.gmail.com>
 <CA+Tgmobs-gbRUMGPpzVZnPPP=cgeEO7hnzV2yKmM0BQ=MGQSMg@mail.gmail.com>
 <CAAJ_b955y7NrkTUGtiyoGkPBs-OgYMv32iW0WTSpVfpFpezxdQ@mail.gmail.com>
 <CA+Tgmob3zhdDM=ggw-NrV-jrvLP7a-otWFn5F+5oOputiFdhmQ@mail.gmail.com>
 <CAAJ_b96RBwBMkgK3gzzm+94m6jvdbMi--8RzxaQLu-2+52Nj6g@mail.gmail.com>
 <CA+TgmoZBPKaonc=rE0d7YUm3fzeQTftP6o=8vqFDe1AY2nmbrw@mail.gmail.com>
 <1115881.1727564342@sss.pgh.pa.!
 us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Sat, 28 Sep 2024 18:59:02 -0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1240822.1727629418.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Sun, 29 Sep 2024 13:03:38 -0400
Message-ID: <1240823.1727629418@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1240823.1727629418%40sss.pgh.pa.us>
Precedence: bulk

Piling on a bit ... Coverity reported the following issues in
this new code.  I have not analyzed them to see if they're
real problems.

__________________________________________________________________________=
______________________________
*** CID 1620458:  Resource leaks  (RESOURCE_LEAK)
/srv/coverity/git/pgsql-git/postgresql/src/bin/pg_verifybackup/pg_verifyba=
ckup.c: 1025 in verify_tar_file()
1019     							relpath);
1020     =

1021     	/* Close the file. */
1022     	if (close(fd) !=3D 0)
1023     		report_backup_error(context, "could not close file \"%s\": %m",
1024     							relpath);
>>>     CID 1620458:  Resource leaks  (RESOURCE_LEAK)
>>>     Variable "buffer" going out of scope leaks the storage it points t=
o.
1025     }
1026     =

1027     /*
1028      * Scan the hash table for entries where the 'matched' flag is no=
t set; report
1029      * that such files are present in the manifest but not on disk.
1030      */

__________________________________________________________________________=
______________________________
*** CID 1620457:  Memory - illegal accesses  (OVERRUN)
/srv/coverity/git/pgsql-git/postgresql/src/bin/pg_verifybackup/astreamer_v=
erify.c: 349 in member_copy_control_data()
343     	 */
344     	if (mystreamer->control_file_bytes <=3D sizeof(ControlFileData))
345     	{
346     		int			remaining;
347     =

348     		remaining =3D sizeof(ControlFileData) - mystreamer->control_file=
_bytes;
>>>     CID 1620457:  Memory - illegal accesses  (OVERRUN)
>>>     Overrunning array of 296 bytes at byte offset 296 by dereferencing=
 pointer "(char *)&mystreamer->control_file + mystreamer->control_file_byt=
es".
349     		memcpy(((char *) &mystreamer->control_file)
350     			   + mystreamer->control_file_bytes,
351     			   data, Min(len, remaining));
352     	}
353     =

354     	/* Remember how many bytes we saw, even if we didn't buffer them.=
 */

__________________________________________________________________________=
______________________________
*** CID 1620456:  Null pointer dereferences  (FORWARD_NULL)
/srv/coverity/git/pgsql-git/postgresql/src/bin/pg_verifybackup/pg_verifyba=
ckup.c: 939 in precheck_tar_backup_file()
933     								"file \"%s\" is not expected in a tar format backup",
934     								relpath);
935     		tblspc_oid =3D (Oid) num;
936     	}
937     =

938     	/* Now, check the compression type of the tar */
>>>     CID 1620456:  Null pointer dereferences  (FORWARD_NULL)
>>>     Passing null pointer "suffix" to "strcmp", which dereferences it.
939     	if (strcmp(suffix, ".tar") =3D=3D 0)
940     		compress_algorithm =3D PG_COMPRESSION_NONE;
941     	else if (strcmp(suffix, ".tgz") =3D=3D 0)
942     		compress_algorithm =3D PG_COMPRESSION_GZIP;
943     	else if (strcmp(suffix, ".tar.gz") =3D=3D 0)
944     		compress_algorithm =3D PG_COMPRESSION_GZIP;


			regards, tom lane