diff mbox series

lib: fwts_acpi_object_eval: avoid any division by zero

Message ID 20200407184532.948596-1-colin.king@canonical.com
State Accepted
Headers show
Series lib: fwts_acpi_object_eval: avoid any division by zero | expand

Commit Message

Colin Ian King April 7, 2020, 6:45 p.m. UTC
From: Colin Ian King <colin.king@canonical.com>

There are a couple of divisions by granularity + 1 and
granularity - 1 that may be overflow or underflowed to zero
causing a division by zero error. Add a sanity check and
flag an error if granularity is going to cause this division
error.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/lib/src/fwts_acpi_object_eval.c | 79 +++++++++++++++++++----------
 1 file changed, 51 insertions(+), 28 deletions(-)

Comments

Alex Hung April 7, 2020, 8:03 p.m. UTC | #1
On 2020-04-07 12:45 p.m., Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> There are a couple of divisions by granularity + 1 and
> granularity - 1 that may be overflow or underflowed to zero
> causing a division by zero error. Add a sanity check and
> flag an error if granularity is going to cause this division
> error.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/lib/src/fwts_acpi_object_eval.c | 79 +++++++++++++++++++----------
>  1 file changed, 51 insertions(+), 28 deletions(-)
> 
> diff --git a/src/lib/src/fwts_acpi_object_eval.c b/src/lib/src/fwts_acpi_object_eval.c
> index 692e4ec0..3845d424 100644
> --- a/src/lib/src/fwts_acpi_object_eval.c
> +++ b/src/lib/src/fwts_acpi_object_eval.c
> @@ -1304,37 +1304,60 @@ static void method_test_CRS_mif_maf(
>  			fwts_advice(fw, "%s", mif_maf_advice);
>  			*passed = false;
>  		}
> -		if ((mif == 1) && (min % (granularity + 1) != 0)) {
> -			snprintf(tmp, sizeof(tmp), "Method%s%sMinNotMultipleOfGran", objname, tag);
> -			if (fw->flags & FWTS_FLAG_TEST_SBBR)
> +
> +		if (mif == 1) {
> +			const uint64_t tmpgran = granularity + 1;
> +
> +			if (tmpgran == 0) {
> +				/* Avoid min % tmp division by zero, flag an error */
> +				snprintf(tmp, sizeof(tmp), "Method%s%sGranTooLarge", objname, tag);
>  				fwts_warning(fw, tmp,
> -					"%s %s _MIN address is not a multiple "
> -					"of the granularity when _MIF is 1.",
> -					name, type);
> -			else
> -				fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -					tmp,
> -					"%s %s _MIN address is not a multiple "
> -					"of the granularity when _MIF is 1.",
> -					name, type);
> -			fwts_advice(fw, "%s", mif_maf_advice);
> -			*passed = false;
> +					"%s %s granularity 0x%" PRIx64 " is too large when _MIF is 1.",
> +					name, type, granularity);
> +				*passed = false;
> +			} else if (min % tmpgran != 0) {
> +				snprintf(tmp, sizeof(tmp), "Method%s%sMinNotMultipleOfGran", objname, tag);
> +				if (fw->flags & FWTS_FLAG_TEST_SBBR)
> +					fwts_warning(fw, tmp,
> +						"%s %s _MIN address is not a multiple "
> +						"of the granularity when _MIF is 1.",
> +						name, type);
> +				else
> +					fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +						tmp,
> +						"%s %s _MIN address is not a multiple "
> +						"of the granularity when _MIF is 1.",
> +						name, type);
> +				fwts_advice(fw, "%s", mif_maf_advice);
> +				*passed = false;
> +			}
>  		}
> -		if ((maf == 1) && (max % (granularity - 1) != 0)) {
> -			snprintf(tmp, sizeof(tmp), "Method%s%sMaxNotMultipleOfGran", objname, tag);
> -			if (fw->flags & FWTS_FLAG_TEST_SBBR)
> +		if (maf == 1) {
> +			const uint64_t tmpgran = granularity - 1;
> +
> +			if (tmpgran == 0) {
> +				/* Avoid max % tmp division by zero, flag an error */
> +				snprintf(tmp, sizeof(tmp), "Method%s%sGranTooSmall", objname, tag);
>  				fwts_warning(fw, tmp,
> -					"%s %s _MAX address is not a multiple "
> -					"of the granularity when _MAF is 1.",
> -					name, type);
> -			else
> -				fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -					tmp,
> -					"%s %s _MAX address is not a multiple "
> -					"of the granularity when _MAF is 1.",
> -					name, type);
> -			fwts_advice(fw, "%s", mif_maf_advice);
> -			*passed = false;
> +					"%s %s granularity 0x%" PRIx64 " is too small when _MAF is 1.",
> +					name, type, granularity);
> +				*passed = false;
> +			} else if (max % tmpgran != 0) {
> +				snprintf(tmp, sizeof(tmp), "Method%s%sMaxNotMultipleOfGran", objname, tag);
> +				if (fw->flags & FWTS_FLAG_TEST_SBBR)
> +					fwts_warning(fw, tmp,
> +						"%s %s _MAX address is not a multiple "
> +						"of the granularity when _MAF is 1.",
> +						name, type);
> +				else
> +					fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +						tmp,
> +						"%s %s _MAX address is not a multiple "
> +						"of the granularity when _MAF is 1.",
> +						name, type);
> +				fwts_advice(fw, "%s", mif_maf_advice);
> +				*passed = false;
> +			}
>  		}
>  	} else {
>  		if ((mif == 0) && (maf == 0) &&
> 

Acked-by: Alex Hung <alex.hung@canonical.com>
Ivan Hu April 8, 2020, 7:29 a.m. UTC | #2
On 4/8/20 2:45 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> There are a couple of divisions by granularity + 1 and
> granularity - 1 that may be overflow or underflowed to zero
> causing a division by zero error. Add a sanity check and
> flag an error if granularity is going to cause this division
> error.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/lib/src/fwts_acpi_object_eval.c | 79 +++++++++++++++++++----------
>  1 file changed, 51 insertions(+), 28 deletions(-)
> 
> diff --git a/src/lib/src/fwts_acpi_object_eval.c b/src/lib/src/fwts_acpi_object_eval.c
> index 692e4ec0..3845d424 100644
> --- a/src/lib/src/fwts_acpi_object_eval.c
> +++ b/src/lib/src/fwts_acpi_object_eval.c
> @@ -1304,37 +1304,60 @@ static void method_test_CRS_mif_maf(
>  			fwts_advice(fw, "%s", mif_maf_advice);
>  			*passed = false;
>  		}
> -		if ((mif == 1) && (min % (granularity + 1) != 0)) {
> -			snprintf(tmp, sizeof(tmp), "Method%s%sMinNotMultipleOfGran", objname, tag);
> -			if (fw->flags & FWTS_FLAG_TEST_SBBR)
> +
> +		if (mif == 1) {
> +			const uint64_t tmpgran = granularity + 1;
> +
> +			if (tmpgran == 0) {
> +				/* Avoid min % tmp division by zero, flag an error */
> +				snprintf(tmp, sizeof(tmp), "Method%s%sGranTooLarge", objname, tag);
>  				fwts_warning(fw, tmp,
> -					"%s %s _MIN address is not a multiple "
> -					"of the granularity when _MIF is 1.",
> -					name, type);
> -			else
> -				fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -					tmp,
> -					"%s %s _MIN address is not a multiple "
> -					"of the granularity when _MIF is 1.",
> -					name, type);
> -			fwts_advice(fw, "%s", mif_maf_advice);
> -			*passed = false;
> +					"%s %s granularity 0x%" PRIx64 " is too large when _MIF is 1.",
> +					name, type, granularity);
> +				*passed = false;
> +			} else if (min % tmpgran != 0) {
> +				snprintf(tmp, sizeof(tmp), "Method%s%sMinNotMultipleOfGran", objname, tag);
> +				if (fw->flags & FWTS_FLAG_TEST_SBBR)
> +					fwts_warning(fw, tmp,
> +						"%s %s _MIN address is not a multiple "
> +						"of the granularity when _MIF is 1.",
> +						name, type);
> +				else
> +					fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +						tmp,
> +						"%s %s _MIN address is not a multiple "
> +						"of the granularity when _MIF is 1.",
> +						name, type);
> +				fwts_advice(fw, "%s", mif_maf_advice);
> +				*passed = false;
> +			}
>  		}
> -		if ((maf == 1) && (max % (granularity - 1) != 0)) {
> -			snprintf(tmp, sizeof(tmp), "Method%s%sMaxNotMultipleOfGran", objname, tag);
> -			if (fw->flags & FWTS_FLAG_TEST_SBBR)
> +		if (maf == 1) {
> +			const uint64_t tmpgran = granularity - 1;
> +
> +			if (tmpgran == 0) {
> +				/* Avoid max % tmp division by zero, flag an error */
> +				snprintf(tmp, sizeof(tmp), "Method%s%sGranTooSmall", objname, tag);
>  				fwts_warning(fw, tmp,
> -					"%s %s _MAX address is not a multiple "
> -					"of the granularity when _MAF is 1.",
> -					name, type);
> -			else
> -				fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -					tmp,
> -					"%s %s _MAX address is not a multiple "
> -					"of the granularity when _MAF is 1.",
> -					name, type);
> -			fwts_advice(fw, "%s", mif_maf_advice);
> -			*passed = false;
> +					"%s %s granularity 0x%" PRIx64 " is too small when _MAF is 1.",
> +					name, type, granularity);
> +				*passed = false;
> +			} else if (max % tmpgran != 0) {
> +				snprintf(tmp, sizeof(tmp), "Method%s%sMaxNotMultipleOfGran", objname, tag);
> +				if (fw->flags & FWTS_FLAG_TEST_SBBR)
> +					fwts_warning(fw, tmp,
> +						"%s %s _MAX address is not a multiple "
> +						"of the granularity when _MAF is 1.",
> +						name, type);
> +				else
> +					fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +						tmp,
> +						"%s %s _MAX address is not a multiple "
> +						"of the granularity when _MAF is 1.",
> +						name, type);
> +				fwts_advice(fw, "%s", mif_maf_advice);
> +				*passed = false;
> +			}
>  		}
>  	} else {
>  		if ((mif == 0) && (maf == 0) &&
> 

Acked-by: Ivan Hu <ivan.hu@canonical.com>
diff mbox series

Patch

diff --git a/src/lib/src/fwts_acpi_object_eval.c b/src/lib/src/fwts_acpi_object_eval.c
index 692e4ec0..3845d424 100644
--- a/src/lib/src/fwts_acpi_object_eval.c
+++ b/src/lib/src/fwts_acpi_object_eval.c
@@ -1304,37 +1304,60 @@  static void method_test_CRS_mif_maf(
 			fwts_advice(fw, "%s", mif_maf_advice);
 			*passed = false;
 		}
-		if ((mif == 1) && (min % (granularity + 1) != 0)) {
-			snprintf(tmp, sizeof(tmp), "Method%s%sMinNotMultipleOfGran", objname, tag);
-			if (fw->flags & FWTS_FLAG_TEST_SBBR)
+
+		if (mif == 1) {
+			const uint64_t tmpgran = granularity + 1;
+
+			if (tmpgran == 0) {
+				/* Avoid min % tmp division by zero, flag an error */
+				snprintf(tmp, sizeof(tmp), "Method%s%sGranTooLarge", objname, tag);
 				fwts_warning(fw, tmp,
-					"%s %s _MIN address is not a multiple "
-					"of the granularity when _MIF is 1.",
-					name, type);
-			else
-				fwts_failed(fw, LOG_LEVEL_MEDIUM,
-					tmp,
-					"%s %s _MIN address is not a multiple "
-					"of the granularity when _MIF is 1.",
-					name, type);
-			fwts_advice(fw, "%s", mif_maf_advice);
-			*passed = false;
+					"%s %s granularity 0x%" PRIx64 " is too large when _MIF is 1.",
+					name, type, granularity);
+				*passed = false;
+			} else if (min % tmpgran != 0) {
+				snprintf(tmp, sizeof(tmp), "Method%s%sMinNotMultipleOfGran", objname, tag);
+				if (fw->flags & FWTS_FLAG_TEST_SBBR)
+					fwts_warning(fw, tmp,
+						"%s %s _MIN address is not a multiple "
+						"of the granularity when _MIF is 1.",
+						name, type);
+				else
+					fwts_failed(fw, LOG_LEVEL_MEDIUM,
+						tmp,
+						"%s %s _MIN address is not a multiple "
+						"of the granularity when _MIF is 1.",
+						name, type);
+				fwts_advice(fw, "%s", mif_maf_advice);
+				*passed = false;
+			}
 		}
-		if ((maf == 1) && (max % (granularity - 1) != 0)) {
-			snprintf(tmp, sizeof(tmp), "Method%s%sMaxNotMultipleOfGran", objname, tag);
-			if (fw->flags & FWTS_FLAG_TEST_SBBR)
+		if (maf == 1) {
+			const uint64_t tmpgran = granularity - 1;
+
+			if (tmpgran == 0) {
+				/* Avoid max % tmp division by zero, flag an error */
+				snprintf(tmp, sizeof(tmp), "Method%s%sGranTooSmall", objname, tag);
 				fwts_warning(fw, tmp,
-					"%s %s _MAX address is not a multiple "
-					"of the granularity when _MAF is 1.",
-					name, type);
-			else
-				fwts_failed(fw, LOG_LEVEL_MEDIUM,
-					tmp,
-					"%s %s _MAX address is not a multiple "
-					"of the granularity when _MAF is 1.",
-					name, type);
-			fwts_advice(fw, "%s", mif_maf_advice);
-			*passed = false;
+					"%s %s granularity 0x%" PRIx64 " is too small when _MAF is 1.",
+					name, type, granularity);
+				*passed = false;
+			} else if (max % tmpgran != 0) {
+				snprintf(tmp, sizeof(tmp), "Method%s%sMaxNotMultipleOfGran", objname, tag);
+				if (fw->flags & FWTS_FLAG_TEST_SBBR)
+					fwts_warning(fw, tmp,
+						"%s %s _MAX address is not a multiple "
+						"of the granularity when _MAF is 1.",
+						name, type);
+				else
+					fwts_failed(fw, LOG_LEVEL_MEDIUM,
+						tmp,
+						"%s %s _MAX address is not a multiple "
+						"of the granularity when _MAF is 1.",
+						name, type);
+				fwts_advice(fw, "%s", mif_maf_advice);
+				*passed = false;
+			}
 		}
 	} else {
 		if ((mif == 0) && (maf == 0) &&