[v2,1/2] xfs_db: btheight should check geometry more carefully
diff mbox series

Message ID 20190926190911.GG9916@magnolia
State New
Headers show
Series
  • [v2,1/2] xfs_db: btheight should check geometry more carefully
Related show

Commit Message

Darrick J. Wong Sept. 26, 2019, 7:09 p.m. UTC
From: Darrick J. Wong <darrick.wong@oracle.com>

The btheight command needs to check user-supplied geometry more
carefully so that we don't hit floating point exceptions.

Coverity-id: 1453661, 1453659
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 db/btheight.c |   88 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 82 insertions(+), 6 deletions(-)

Comments

Carlos Maiolino Oct. 1, 2019, 6:40 a.m. UTC | #1
On Thu, Sep 26, 2019 at 12:09:11PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> The btheight command needs to check user-supplied geometry more
> carefully so that we don't hit floating point exceptions.
> 
> Coverity-id: 1453661, 1453659
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks good to me

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> ---
>  db/btheight.c |   88 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 82 insertions(+), 6 deletions(-)
> 
> diff --git a/db/btheight.c b/db/btheight.c
> index 289e5d84..8aa17c89 100644
> --- a/db/btheight.c
> +++ b/db/btheight.c
> @@ -138,6 +138,10 @@ construct_records_per_block(
>  		perror(p);
>  		goto out;
>  	}
> +	if (record_size == 0) {
> +		fprintf(stderr, _("%s: record size cannot be zero.\n"), tag);
> +		goto out;
> +	}
>  
>  	p = strtok(NULL, ":");
>  	if (!p) {
> @@ -149,6 +153,10 @@ construct_records_per_block(
>  		perror(p);
>  		goto out;
>  	}
> +	if (key_size == 0) {
> +		fprintf(stderr, _("%s: key size cannot be zero.\n"), tag);
> +		goto out;
> +	}
>  
>  	p = strtok(NULL, ":");
>  	if (!p) {
> @@ -160,6 +168,10 @@ construct_records_per_block(
>  		perror(p);
>  		goto out;
>  	}
> +	if (ptr_size == 0) {
> +		fprintf(stderr, _("%s: pointer size cannot be zero.\n"), tag);
> +		goto out;
> +	}
>  
>  	p = strtok(NULL, ":");
>  	if (!p) {
> @@ -180,6 +192,27 @@ construct_records_per_block(
>  		goto out;
>  	}
>  
> +	if (record_size > blocksize) {
> +		fprintf(stderr,
> +_("%s: record size must be less than selected block size (%u bytes).\n"),
> +			tag, blocksize);
> +		goto out;
> +	}
> +
> +	if (key_size > blocksize) {
> +		fprintf(stderr,
> +_("%s: key size must be less than selected block size (%u bytes).\n"),
> +			tag, blocksize);
> +		goto out;
> +	}
> +
> +	if (ptr_size > blocksize) {
> +		fprintf(stderr,
> +_("%s: pointer size must be less than selected block size (%u bytes).\n"),
> +			tag, blocksize);
> +		goto out;
> +	}
> +
>  	p = strtok(NULL, ":");
>  	if (p) {
>  		fprintf(stderr,
> @@ -211,13 +244,24 @@ report(
>  	int			ret;
>  
>  	ret = construct_records_per_block(tag, blocksize, records_per_block);
> -	if (ret) {
> -		printf(_("%s: Unable to determine records per block.\n"),
> -				tag);
> +	if (ret)
>  		return;
> -	}
>  
>  	if (report_what & REPORT_MAX) {
> +		if (records_per_block[0] < 2) {
> +			fprintf(stderr,
> +_("%s: cannot calculate best case scenario due to leaf geometry underflow.\n"),
> +				tag);
> +			return;
> +		}
> +
> +		if (records_per_block[1] < 4) {
> +			fprintf(stderr,
> +_("%s: cannot calculate best case scenario due to node geometry underflow.\n"),
> +				tag);
> +			return;
> +		}
> +
>  		printf(
>  _("%s: best case per %u-byte block: %u records (leaf) / %u keyptrs (node)\n"),
>  				tag, blocksize, records_per_block[0],
> @@ -230,6 +274,20 @@ _("%s: best case per %u-byte block: %u records (leaf) / %u keyptrs (node)\n"),
>  		records_per_block[0] /= 2;
>  		records_per_block[1] /= 2;
>  
> +		if (records_per_block[0] < 1) {
> +			fprintf(stderr,
> +_("%s: cannot calculate worst case scenario due to leaf geometry underflow.\n"),
> +				tag);
> +			return;
> +		}
> +
> +		if (records_per_block[1] < 2) {
> +			fprintf(stderr,
> +_("%s: cannot calculate worst case scenario due to node geometry underflow.\n"),
> +				tag);
> +			return;
> +		}
> +
>  		printf(
>  _("%s: worst case per %u-byte block: %u records (leaf) / %u keyptrs (node)\n"),
>  				tag, blocksize, records_per_block[0],
> @@ -284,8 +342,26 @@ btheight_f(
>  		}
>  	}
>  
> -	if (argc == optind || blocksize <= 0 || blocksize > INT_MAX ||
> -	    nr_records == 0) {
> +	if (nr_records == 0) {
> +		fprintf(stderr,
> +_("Number of records must be greater than zero.\n"));
> +		return 0;
> +	}
> +
> +	if (blocksize > INT_MAX) {
> +		fprintf(stderr,
> +_("The largest block size this command will consider is %u bytes.\n"),
> +			INT_MAX);
> +		return 0;
> +	}
> +
> +	if (blocksize < 128) {
> +		fprintf(stderr,
> +_("The smallest block size this command will consider is 128 bytes.\n"));
> +		return 0;
> +	}
> +
> +	if (argc == optind) {
>  		btheight_help();
>  		return 0;
>  	}

Patch
diff mbox series

diff --git a/db/btheight.c b/db/btheight.c
index 289e5d84..8aa17c89 100644
--- a/db/btheight.c
+++ b/db/btheight.c
@@ -138,6 +138,10 @@  construct_records_per_block(
 		perror(p);
 		goto out;
 	}
+	if (record_size == 0) {
+		fprintf(stderr, _("%s: record size cannot be zero.\n"), tag);
+		goto out;
+	}
 
 	p = strtok(NULL, ":");
 	if (!p) {
@@ -149,6 +153,10 @@  construct_records_per_block(
 		perror(p);
 		goto out;
 	}
+	if (key_size == 0) {
+		fprintf(stderr, _("%s: key size cannot be zero.\n"), tag);
+		goto out;
+	}
 
 	p = strtok(NULL, ":");
 	if (!p) {
@@ -160,6 +168,10 @@  construct_records_per_block(
 		perror(p);
 		goto out;
 	}
+	if (ptr_size == 0) {
+		fprintf(stderr, _("%s: pointer size cannot be zero.\n"), tag);
+		goto out;
+	}
 
 	p = strtok(NULL, ":");
 	if (!p) {
@@ -180,6 +192,27 @@  construct_records_per_block(
 		goto out;
 	}
 
+	if (record_size > blocksize) {
+		fprintf(stderr,
+_("%s: record size must be less than selected block size (%u bytes).\n"),
+			tag, blocksize);
+		goto out;
+	}
+
+	if (key_size > blocksize) {
+		fprintf(stderr,
+_("%s: key size must be less than selected block size (%u bytes).\n"),
+			tag, blocksize);
+		goto out;
+	}
+
+	if (ptr_size > blocksize) {
+		fprintf(stderr,
+_("%s: pointer size must be less than selected block size (%u bytes).\n"),
+			tag, blocksize);
+		goto out;
+	}
+
 	p = strtok(NULL, ":");
 	if (p) {
 		fprintf(stderr,
@@ -211,13 +244,24 @@  report(
 	int			ret;
 
 	ret = construct_records_per_block(tag, blocksize, records_per_block);
-	if (ret) {
-		printf(_("%s: Unable to determine records per block.\n"),
-				tag);
+	if (ret)
 		return;
-	}
 
 	if (report_what & REPORT_MAX) {
+		if (records_per_block[0] < 2) {
+			fprintf(stderr,
+_("%s: cannot calculate best case scenario due to leaf geometry underflow.\n"),
+				tag);
+			return;
+		}
+
+		if (records_per_block[1] < 4) {
+			fprintf(stderr,
+_("%s: cannot calculate best case scenario due to node geometry underflow.\n"),
+				tag);
+			return;
+		}
+
 		printf(
 _("%s: best case per %u-byte block: %u records (leaf) / %u keyptrs (node)\n"),
 				tag, blocksize, records_per_block[0],
@@ -230,6 +274,20 @@  _("%s: best case per %u-byte block: %u records (leaf) / %u keyptrs (node)\n"),
 		records_per_block[0] /= 2;
 		records_per_block[1] /= 2;
 
+		if (records_per_block[0] < 1) {
+			fprintf(stderr,
+_("%s: cannot calculate worst case scenario due to leaf geometry underflow.\n"),
+				tag);
+			return;
+		}
+
+		if (records_per_block[1] < 2) {
+			fprintf(stderr,
+_("%s: cannot calculate worst case scenario due to node geometry underflow.\n"),
+				tag);
+			return;
+		}
+
 		printf(
 _("%s: worst case per %u-byte block: %u records (leaf) / %u keyptrs (node)\n"),
 				tag, blocksize, records_per_block[0],
@@ -284,8 +342,26 @@  btheight_f(
 		}
 	}
 
-	if (argc == optind || blocksize <= 0 || blocksize > INT_MAX ||
-	    nr_records == 0) {
+	if (nr_records == 0) {
+		fprintf(stderr,
+_("Number of records must be greater than zero.\n"));
+		return 0;
+	}
+
+	if (blocksize > INT_MAX) {
+		fprintf(stderr,
+_("The largest block size this command will consider is %u bytes.\n"),
+			INT_MAX);
+		return 0;
+	}
+
+	if (blocksize < 128) {
+		fprintf(stderr,
+_("The smallest block size this command will consider is 128 bytes.\n"));
+		return 0;
+	}
+
+	if (argc == optind) {
 		btheight_help();
 		return 0;
 	}