c/mirror_zfs
1
0
Fork 0
mirror of https://git.proxmox.com/git/mirror_zfs.git synced 2024-12-26 03:09:34 +03:00
Code Issues Packages Projects Releases Wiki Activity

Address theoretical uninitialized variable usage in zstream

Browse Source 
Coverity has long complained about the checksum being uninitialized if
an END record is processed before its BEGIN record. This should not
happen, but there was no code to check for it. I had left this unfixed
since it was a low priority issue, but then
9f4ede63d2 added another instance of this.

I am making an effort to "hold the line" to keep new coverity defect
reports from going unaddressed, so I find myself forced to fix this much
earlier than I had originally planned to address it.

The solution is to maintain a counter and a flag. Then use VERIFY
statements to verify the following runtime constraints:

 * Every record either has a corresponding BEGIN record, is a BEGIN
   record or is the end of stream END record for replication streams.
 * BEGIN records cannot be nested. i.e. There must be an END record
   before another BEGIN record may be seen.

Failure to meet these constraints will cause the program to exit.

This is sufficient to ensure that the checksum is never accessed when
uninitialized.

Reported-by: Coverity (CID 1524578)
Reported-by: Coverity (CID 1524633)
Reported-by: Coverity (CID 1527295)
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Damian Szuberski <szuberskidamian@gmail.com>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14176
This commit is contained in:
Richard Yao 2022-12-12 13:40:05 -05:00 committed by GitHub
parent 786ff6a6cb
commit d31a7cb4fa
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 51 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
cmd/zstream/zstream_decompress.c
View File

@ -158,6 +158,8 @@ zstream_do_decompress(int argc, char *argv[])
}
fletcher_4_init();
int begin = 0;
boolean_t seen = B_FALSE;
while (sfread(drr, sizeof (*drr), stdin) != 0) {
struct drr_write *drrw;
uint64_t payload_size = 0;
@ -174,6 +176,8 @@ zstream_do_decompress(int argc, char *argv[])
case DRR_BEGIN:
{
ZIO_SET_CHECKSUM(&stream_cksum, 0, 0, 0, 0);
VERIFY0(begin++);
seen = B_TRUE;
int sz = drr->drr_payloadlen;
if (sz != 0) {
@ -191,6 +195,13 @@ zstream_do_decompress(int argc, char *argv[])
case DRR_END:
{
struct drr_end *drre = &drr->drr_u.drr_end;
/*
* We would prefer to just check --begin == 0, but
* replication streams have an end of stream END
* record, so we must avoid tripping it.
*/
VERIFY3B(seen, ==, B_TRUE);
begin--;
/*
* Use the recalculated checksum, unless this is
* the END record of a stream package, which has
@ -204,6 +215,7 @@ zstream_do_decompress(int argc, char *argv[])
case DRR_OBJECT:
{
struct drr_object *drro = &drr->drr_u.drr_object;
VERIFY3S(begin, ==, 1);
if (drro->drr_bonuslen > 0) {
payload_size = DRR_OBJECT_PAYLOAD_SIZE(drro);
@ -215,12 +227,14 @@ zstream_do_decompress(int argc, char *argv[])
case DRR_SPILL:
{
struct drr_spill *drrs = &drr->drr_u.drr_spill;
VERIFY3S(begin, ==, 1);
payload_size = DRR_SPILL_PAYLOAD_SIZE(drrs);
(void) sfread(buf, payload_size, stdin);
break;
}
case DRR_WRITE_BYREF:
VERIFY3S(begin, ==, 1);
fprintf(stderr,
"Deduplicated streams are not supported\n");
exit(1);
@ -228,6 +242,7 @@ zstream_do_decompress(int argc, char *argv[])
case DRR_WRITE:
{
VERIFY3S(begin, ==, 1);
drrw = &thedrr.drr_u.drr_write;
payload_size = DRR_WRITE_PAYLOAD_SIZE(drrw);
ENTRY *p;
@ -321,6 +336,7 @@ zstream_do_decompress(int argc, char *argv[])
case DRR_WRITE_EMBEDDED:
{
VERIFY3S(begin, ==, 1);
struct drr_write_embedded *drrwe =
&drr->drr_u.drr_write_embedded;
payload_size =
@ -332,6 +348,7 @@ zstream_do_decompress(int argc, char *argv[])
case DRR_FREEOBJECTS:
case DRR_FREE:
case DRR_OBJECT_RANGE:
VERIFY3S(begin, ==, 1);
break;
default:

17
cmd/zstream/zstream_recompress.c
View File

@ -138,6 +138,8 @@ zstream_do_recompress(int argc, char *argv[])
fletcher_4_init();
zio_init();
zstd_init();
int begin = 0;
boolean_t seen = B_FALSE;
while (sfread(drr, sizeof (*drr), stdin) != 0) {
struct drr_write *drrw;
uint64_t payload_size = 0;
@ -155,6 +157,8 @@ zstream_do_recompress(int argc, char *argv[])
case DRR_BEGIN:
{
ZIO_SET_CHECKSUM(&stream_cksum, 0, 0, 0, 0);
VERIFY0(begin++);
seen = B_TRUE;
int sz = drr->drr_payloadlen;
if (sz != 0) {
@ -172,6 +176,13 @@ zstream_do_recompress(int argc, char *argv[])
case DRR_END:
{
struct drr_end *drre = &drr->drr_u.drr_end;
/*
* We would prefer to just check --begin == 0, but
* replication streams have an end of stream END
* record, so we must avoid tripping it.
*/
VERIFY3B(seen, ==, B_TRUE);
begin--;
/*
* Use the recalculated checksum, unless this is
* the END record of a stream package, which has
@ -185,6 +196,7 @@ zstream_do_recompress(int argc, char *argv[])
case DRR_OBJECT:
{
struct drr_object *drro = &drr->drr_u.drr_object;
VERIFY3S(begin, ==, 1);
if (drro->drr_bonuslen > 0) {
payload_size = DRR_OBJECT_PAYLOAD_SIZE(drro);
@ -196,12 +208,14 @@ zstream_do_recompress(int argc, char *argv[])
case DRR_SPILL:
{
struct drr_spill *drrs = &drr->drr_u.drr_spill;
VERIFY3S(begin, ==, 1);
payload_size = DRR_SPILL_PAYLOAD_SIZE(drrs);
(void) sfread(buf, payload_size, stdin);
break;
}
case DRR_WRITE_BYREF:
VERIFY3S(begin, ==, 1);
fprintf(stderr,
"Deduplicated streams are not supported\n");
exit(1);
@ -209,6 +223,7 @@ zstream_do_recompress(int argc, char *argv[])
case DRR_WRITE:
{
VERIFY3S(begin, ==, 1);
drrw = &thedrr.drr_u.drr_write;
payload_size = DRR_WRITE_PAYLOAD_SIZE(drrw);
/*
@ -295,6 +310,7 @@ zstream_do_recompress(int argc, char *argv[])
{
struct drr_write_embedded *drrwe =
&drr->drr_u.drr_write_embedded;
VERIFY3S(begin, ==, 1);
payload_size =
P2ROUNDUP((uint64_t)drrwe->drr_psize, 8);
(void) sfread(buf, payload_size, stdin);
@ -304,6 +320,7 @@ zstream_do_recompress(int argc, char *argv[])
case DRR_FREEOBJECTS:
case DRR_FREE:
case DRR_OBJECT_RANGE:
VERIFY3S(begin, ==, 1);
break;
default:

17
cmd/zstream/zstream_redup.c
View File

@ -222,6 +222,8 @@ zfs_redup_stream(int infd, int outfd, boolean_t verbose)
char *buf = safe_calloc(bufsz);
FILE *ofp = fdopen(infd, "r");
long offset = ftell(ofp);
int begin = 0;
boolean_t seen = B_FALSE;
while (sfread(drr, sizeof (*drr), ofp) != 0) {
num_records++;
@ -240,6 +242,8 @@ zfs_redup_stream(int infd, int outfd, boolean_t verbose)
struct drr_begin *drrb = &drr->drr_u.drr_begin;
int fflags;
ZIO_SET_CHECKSUM(&stream_cksum, 0, 0, 0, 0);
VERIFY0(begin++);
seen = B_TRUE;
assert(drrb->drr_magic == DMU_BACKUP_MAGIC);
@ -266,6 +270,13 @@ zfs_redup_stream(int infd, int outfd, boolean_t verbose)
case DRR_END:
{
struct drr_end *drre = &drr->drr_u.drr_end;
/*
* We would prefer to just check --begin == 0, but
* replication streams have an end of stream END
* record, so we must avoid tripping it.
*/
VERIFY3B(seen, ==, B_TRUE);
begin--;
/*
* Use the recalculated checksum, unless this is
* the END record of a stream package, which has
@ -279,6 +290,7 @@ zfs_redup_stream(int infd, int outfd, boolean_t verbose)
case DRR_OBJECT:
{
struct drr_object *drro = &drr->drr_u.drr_object;
VERIFY3S(begin, ==, 1);
if (drro->drr_bonuslen > 0) {
payload_size = DRR_OBJECT_PAYLOAD_SIZE(drro);
@ -290,6 +302,7 @@ zfs_redup_stream(int infd, int outfd, boolean_t verbose)
case DRR_SPILL:
{
struct drr_spill *drrs = &drr->drr_u.drr_spill;
VERIFY3S(begin, ==, 1);
payload_size = DRR_SPILL_PAYLOAD_SIZE(drrs);
(void) sfread(buf, payload_size, ofp);
break;
@ -299,6 +312,7 @@ zfs_redup_stream(int infd, int outfd, boolean_t verbose)
{
struct drr_write_byref drrwb =
drr->drr_u.drr_write_byref;
VERIFY3S(begin, ==, 1);
num_write_byref_records++;
@ -334,6 +348,7 @@ zfs_redup_stream(int infd, int outfd, boolean_t verbose)
case DRR_WRITE:
{
struct drr_write *drrw = &drr->drr_u.drr_write;
VERIFY3S(begin, ==, 1);
payload_size = DRR_WRITE_PAYLOAD_SIZE(drrw);
(void) sfread(buf, payload_size, ofp);
@ -346,6 +361,7 @@ zfs_redup_stream(int infd, int outfd, boolean_t verbose)
{
struct drr_write_embedded *drrwe =
&drr->drr_u.drr_write_embedded;
VERIFY3S(begin, ==, 1);
payload_size =
P2ROUNDUP((uint64_t)drrwe->drr_psize, 8);
(void) sfread(buf, payload_size, ofp);
@ -355,6 +371,7 @@ zfs_redup_stream(int infd, int outfd, boolean_t verbose)
case DRR_FREEOBJECTS:
case DRR_FREE:
case DRR_OBJECT_RANGE:
VERIFY3S(begin, ==, 1);
break;
default: