diff mbox

[v2,22/23] FADT: add safety checks for older versions of FADT

Message ID 1455925199-8587-23-git-send-email-al.stone@linaro.org
State Accepted
Headers show

Commit Message

Al Stone Feb. 19, 2016, 11:39 p.m. UTC
Not every set of FADT entries will make nice and clean ACPI 6.0/6.1
FADTs.  So, assume that we will also test older versions of the FADT
and be more paranoid about whether or not the field we're testing is
even defined for this version, skipping the test if is not.

Signed-off-by: Al Stone <al.stone@linaro.org>
---
 src/acpi/fadt/fadt.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

Comments

Alex Hung Feb. 23, 2016, 7:01 a.m. UTC | #1
On 02/20/2016 07:39 AM, Al Stone wrote:
> Not every set of FADT entries will make nice and clean ACPI 6.0/6.1
> FADTs.  So, assume that we will also test older versions of the FADT
> and be more paranoid about whether or not the field we're testing is
> even defined for this version, skipping the test if is not.
>
> Signed-off-by: Al Stone <al.stone@linaro.org>
> ---
>   src/acpi/fadt/fadt.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 55 insertions(+)
>
> diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c
> index 40a0028..6dc324e 100644
> --- a/src/acpi/fadt/fadt.c
> +++ b/src/acpi/fadt/fadt.c
> @@ -334,6 +334,8 @@ static void acpi_table_check_fadt_dsdt(fwts_framework *fw)
>   			fwts_failed(fw, LOG_LEVEL_MEDIUM,
>   				"FADTDSDTNull",
>   				"FADT DSDT address is null.");
> +		/* can't do much else */
> +		return;
>   	}
>
>   	/* if one field is being used, the other should be null */
> @@ -412,6 +414,9 @@ static void acpi_table_check_fadt_reserved(fwts_framework *fw)
>   			    "FADT first reserved field is not zero: 0x%02x",
>   			    fadt->reserved);
>
> +	if (fadt->header.length < 112)
> +		return;
> +
>   	if (fadt->reserved1 == (uint8_t)0)
>   		fwts_passed(fw, "FADT second reserved field is zero.");
>   	else
> @@ -914,6 +919,21 @@ static void acpi_table_check_fadt_pm1a_evt_blk(fwts_framework *fw)
>   	bool both_zero;
>   	bool both_nonzero;
>
> +	if (fadt->header.length < 160) {
> +		if (fadt->pm1a_evt_blk == 0)
> +			fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +				    "FADTPm1aEvtBlkNotSet",
> +				    "FADT PM1A_EVT_BLK is a required field "
> +				    "and must have a 32-bit address set.");
> +		else
> +			fwts_passed(fw,
> +				    "FADT required PM1A_EVT_BLK field is "
> +				    "non-zero.");
> +
> +		/* can't do much else */
> +		return;
> +	}
> +
>   	if (fadt->pm1a_evt_blk == 0 && fadt->x_pm1a_evt_blk.address == 0) {
>   		both_zero = true;
>   		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> @@ -968,6 +988,11 @@ static void acpi_table_check_fadt_pm1a_evt_blk(fwts_framework *fw)
>
>   static void acpi_table_check_fadt_pm1b_evt_blk(fwts_framework *fw)
>   {
> +	if (fadt->pm1b_evt_blk == 0 && fadt->header.length < 172) {
> +		fwts_skipped(fw, "FADT PM1B_EVT_BLK not being used.");
> +		return;
> +	}
> +
>   	if (fadt->pm1b_evt_blk == 0 && fadt->x_pm1b_evt_blk.address == 0) {
>   		fwts_skipped(fw, "FADT PM1B_EVT_BLK not being used.");
>   		return;
> @@ -1009,6 +1034,21 @@ static void acpi_table_check_fadt_pm1b_evt_blk(fwts_framework *fw)
>
>   static void acpi_table_check_fadt_pm1a_cnt_blk(fwts_framework *fw)
>   {
> +	if (fadt->header.length < 184) {
> +		if (fadt->pm1a_cnt_blk == 0)
> +			fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +				    "FADTPm1aCntBlkNotSet",
> +				    "FADT PM1A_CNT_BLK is a required field "
> +				    "and must have a 32-bit address set.");
> +		else
> +			fwts_passed(fw,
> +				    "FADT required PM1A_EVT_BLK field is "
> +				    "non-zero.");
> +
> +		/* can't do much else */
> +		return;
> +	}
> +
>   	if (fadt->pm1a_cnt_blk != 0 || fadt->x_pm1a_cnt_blk.address != 0)
>   		fwts_passed(fw,
>   			    "FADT required PM1A_CNT_BLK field is non-zero");
> @@ -1054,6 +1094,11 @@ static void acpi_table_check_fadt_pm1a_cnt_blk(fwts_framework *fw)
>
>   static void acpi_table_check_fadt_pm1b_cnt_blk(fwts_framework *fw)
>   {
> +	if (fadt->pm1b_cnt_blk == 0 && fadt->header.length < 196) {
> +		fwts_skipped(fw, "FADT PM1B_CNT_BLK not being used.");
> +		return;
> +	}
> +
>   	if (fadt->pm1b_cnt_blk == 0 && fadt->x_pm1b_cnt_blk.address == 0) {
>   		fwts_skipped(fw, "FADT PM1B_CNT_BLK not being used.");
>   		return;
> @@ -1095,6 +1140,11 @@ static void acpi_table_check_fadt_pm1b_cnt_blk(fwts_framework *fw)
>
>   static void acpi_table_check_fadt_pm2_cnt_blk(fwts_framework *fw)
>   {
> +	if (fadt->pm2_cnt_blk == 0 && fadt->header.length < 208) {
> +		fwts_skipped(fw, "FADT PM2_CNT_BLK not being used.");
> +		return;
> +	}
> +
>   	if (fadt->pm2_cnt_blk == 0 && fadt->x_pm2_cnt_blk.address == 0) {
>   		fwts_skipped(fw, "FADT PM2_CNT_BLK not being used.");
>   		return;
> @@ -1136,6 +1186,11 @@ static void acpi_table_check_fadt_pm2_cnt_blk(fwts_framework *fw)
>
>   static void acpi_table_check_fadt_pm_tmr_blk(fwts_framework *fw)
>   {
> +	if (fadt->pm_tmr_blk == 0 && fadt->header.length < 220) {
> +		fwts_skipped(fw, "FADT PM_TMR_BLK not being used.");
> +		return;
> +	}
> +
>   	if (fadt->pm_tmr_blk == 0 && fadt->x_pm_tmr_blk.address == 0) {
>   		fwts_skipped(fw, "FADT PM_TMR_BLK not being used.");
>   		return;
>


Acked-by: Alex Hung <alex.hung@canonical.com>
Ivan Hu Feb. 23, 2016, 9:09 a.m. UTC | #2
On 2016年02月20日 07:39, Al Stone wrote:
> Not every set of FADT entries will make nice and clean ACPI 6.0/6.1
> FADTs.  So, assume that we will also test older versions of the FADT
> and be more paranoid about whether or not the field we're testing is
> even defined for this version, skipping the test if is not.
>
> Signed-off-by: Al Stone <al.stone@linaro.org>
> ---
>   src/acpi/fadt/fadt.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 55 insertions(+)
>
> diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c
> index 40a0028..6dc324e 100644
> --- a/src/acpi/fadt/fadt.c
> +++ b/src/acpi/fadt/fadt.c
> @@ -334,6 +334,8 @@ static void acpi_table_check_fadt_dsdt(fwts_framework *fw)
>   			fwts_failed(fw, LOG_LEVEL_MEDIUM,
>   				"FADTDSDTNull",
>   				"FADT DSDT address is null.");
> +		/* can't do much else */
> +		return;
>   	}
>   
>   	/* if one field is being used, the other should be null */
> @@ -412,6 +414,9 @@ static void acpi_table_check_fadt_reserved(fwts_framework *fw)
>   			    "FADT first reserved field is not zero: 0x%02x",
>   			    fadt->reserved);
>   
> +	if (fadt->header.length < 112)
> +		return;
> +
>   	if (fadt->reserved1 == (uint8_t)0)
>   		fwts_passed(fw, "FADT second reserved field is zero.");
>   	else
> @@ -914,6 +919,21 @@ static void acpi_table_check_fadt_pm1a_evt_blk(fwts_framework *fw)
>   	bool both_zero;
>   	bool both_nonzero;
>   
> +	if (fadt->header.length < 160) {
> +		if (fadt->pm1a_evt_blk == 0)
> +			fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +				    "FADTPm1aEvtBlkNotSet",
> +				    "FADT PM1A_EVT_BLK is a required field "
> +				    "and must have a 32-bit address set.");
> +		else
> +			fwts_passed(fw,
> +				    "FADT required PM1A_EVT_BLK field is "
> +				    "non-zero.");
> +
> +		/* can't do much else */
> +		return;
> +	}
> +
>   	if (fadt->pm1a_evt_blk == 0 && fadt->x_pm1a_evt_blk.address == 0) {
>   		both_zero = true;
>   		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> @@ -968,6 +988,11 @@ static void acpi_table_check_fadt_pm1a_evt_blk(fwts_framework *fw)
>   
>   static void acpi_table_check_fadt_pm1b_evt_blk(fwts_framework *fw)
>   {
> +	if (fadt->pm1b_evt_blk == 0 && fadt->header.length < 172) {
> +		fwts_skipped(fw, "FADT PM1B_EVT_BLK not being used.");
> +		return;
> +	}
> +
>   	if (fadt->pm1b_evt_blk == 0 && fadt->x_pm1b_evt_blk.address == 0) {
>   		fwts_skipped(fw, "FADT PM1B_EVT_BLK not being used.");
>   		return;
> @@ -1009,6 +1034,21 @@ static void acpi_table_check_fadt_pm1b_evt_blk(fwts_framework *fw)
>   
>   static void acpi_table_check_fadt_pm1a_cnt_blk(fwts_framework *fw)
>   {
> +	if (fadt->header.length < 184) {
> +		if (fadt->pm1a_cnt_blk == 0)
> +			fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +				    "FADTPm1aCntBlkNotSet",
> +				    "FADT PM1A_CNT_BLK is a required field "
> +				    "and must have a 32-bit address set.");
> +		else
> +			fwts_passed(fw,
> +				    "FADT required PM1A_EVT_BLK field is "
> +				    "non-zero.");
> +
> +		/* can't do much else */
> +		return;
> +	}
> +
>   	if (fadt->pm1a_cnt_blk != 0 || fadt->x_pm1a_cnt_blk.address != 0)
>   		fwts_passed(fw,
>   			    "FADT required PM1A_CNT_BLK field is non-zero");
> @@ -1054,6 +1094,11 @@ static void acpi_table_check_fadt_pm1a_cnt_blk(fwts_framework *fw)
>   
>   static void acpi_table_check_fadt_pm1b_cnt_blk(fwts_framework *fw)
>   {
> +	if (fadt->pm1b_cnt_blk == 0 && fadt->header.length < 196) {
> +		fwts_skipped(fw, "FADT PM1B_CNT_BLK not being used.");
> +		return;
> +	}
> +
>   	if (fadt->pm1b_cnt_blk == 0 && fadt->x_pm1b_cnt_blk.address == 0) {
>   		fwts_skipped(fw, "FADT PM1B_CNT_BLK not being used.");
>   		return;
> @@ -1095,6 +1140,11 @@ static void acpi_table_check_fadt_pm1b_cnt_blk(fwts_framework *fw)
>   
>   static void acpi_table_check_fadt_pm2_cnt_blk(fwts_framework *fw)
>   {
> +	if (fadt->pm2_cnt_blk == 0 && fadt->header.length < 208) {
> +		fwts_skipped(fw, "FADT PM2_CNT_BLK not being used.");
> +		return;
> +	}
> +
>   	if (fadt->pm2_cnt_blk == 0 && fadt->x_pm2_cnt_blk.address == 0) {
>   		fwts_skipped(fw, "FADT PM2_CNT_BLK not being used.");
>   		return;
> @@ -1136,6 +1186,11 @@ static void acpi_table_check_fadt_pm2_cnt_blk(fwts_framework *fw)
>   
>   static void acpi_table_check_fadt_pm_tmr_blk(fwts_framework *fw)
>   {
> +	if (fadt->pm_tmr_blk == 0 && fadt->header.length < 220) {
> +		fwts_skipped(fw, "FADT PM_TMR_BLK not being used.");
> +		return;
> +	}
> +
>   	if (fadt->pm_tmr_blk == 0 && fadt->x_pm_tmr_blk.address == 0) {
>   		fwts_skipped(fw, "FADT PM_TMR_BLK not being used.");
>   		return;

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

Patch

diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c
index 40a0028..6dc324e 100644
--- a/src/acpi/fadt/fadt.c
+++ b/src/acpi/fadt/fadt.c
@@ -334,6 +334,8 @@  static void acpi_table_check_fadt_dsdt(fwts_framework *fw)
 			fwts_failed(fw, LOG_LEVEL_MEDIUM,
 				"FADTDSDTNull",
 				"FADT DSDT address is null.");
+		/* can't do much else */
+		return;
 	}
 
 	/* if one field is being used, the other should be null */
@@ -412,6 +414,9 @@  static void acpi_table_check_fadt_reserved(fwts_framework *fw)
 			    "FADT first reserved field is not zero: 0x%02x",
 			    fadt->reserved);
 
+	if (fadt->header.length < 112)
+		return;
+
 	if (fadt->reserved1 == (uint8_t)0)
 		fwts_passed(fw, "FADT second reserved field is zero.");
 	else
@@ -914,6 +919,21 @@  static void acpi_table_check_fadt_pm1a_evt_blk(fwts_framework *fw)
 	bool both_zero;
 	bool both_nonzero;
 
+	if (fadt->header.length < 160) {
+		if (fadt->pm1a_evt_blk == 0)
+			fwts_failed(fw, LOG_LEVEL_MEDIUM,
+				    "FADTPm1aEvtBlkNotSet",
+				    "FADT PM1A_EVT_BLK is a required field "
+				    "and must have a 32-bit address set.");
+		else
+			fwts_passed(fw,
+				    "FADT required PM1A_EVT_BLK field is "
+				    "non-zero.");
+
+		/* can't do much else */
+		return;
+	}
+
 	if (fadt->pm1a_evt_blk == 0 && fadt->x_pm1a_evt_blk.address == 0) {
 		both_zero = true;
 		fwts_failed(fw, LOG_LEVEL_MEDIUM,
@@ -968,6 +988,11 @@  static void acpi_table_check_fadt_pm1a_evt_blk(fwts_framework *fw)
 
 static void acpi_table_check_fadt_pm1b_evt_blk(fwts_framework *fw)
 {
+	if (fadt->pm1b_evt_blk == 0 && fadt->header.length < 172) {
+		fwts_skipped(fw, "FADT PM1B_EVT_BLK not being used.");
+		return;
+	}
+
 	if (fadt->pm1b_evt_blk == 0 && fadt->x_pm1b_evt_blk.address == 0) {
 		fwts_skipped(fw, "FADT PM1B_EVT_BLK not being used.");
 		return;
@@ -1009,6 +1034,21 @@  static void acpi_table_check_fadt_pm1b_evt_blk(fwts_framework *fw)
 
 static void acpi_table_check_fadt_pm1a_cnt_blk(fwts_framework *fw)
 {
+	if (fadt->header.length < 184) {
+		if (fadt->pm1a_cnt_blk == 0)
+			fwts_failed(fw, LOG_LEVEL_MEDIUM,
+				    "FADTPm1aCntBlkNotSet",
+				    "FADT PM1A_CNT_BLK is a required field "
+				    "and must have a 32-bit address set.");
+		else
+			fwts_passed(fw,
+				    "FADT required PM1A_EVT_BLK field is "
+				    "non-zero.");
+
+		/* can't do much else */
+		return;
+	}
+
 	if (fadt->pm1a_cnt_blk != 0 || fadt->x_pm1a_cnt_blk.address != 0)
 		fwts_passed(fw,
 			    "FADT required PM1A_CNT_BLK field is non-zero");
@@ -1054,6 +1094,11 @@  static void acpi_table_check_fadt_pm1a_cnt_blk(fwts_framework *fw)
 
 static void acpi_table_check_fadt_pm1b_cnt_blk(fwts_framework *fw)
 {
+	if (fadt->pm1b_cnt_blk == 0 && fadt->header.length < 196) {
+		fwts_skipped(fw, "FADT PM1B_CNT_BLK not being used.");
+		return;
+	}
+
 	if (fadt->pm1b_cnt_blk == 0 && fadt->x_pm1b_cnt_blk.address == 0) {
 		fwts_skipped(fw, "FADT PM1B_CNT_BLK not being used.");
 		return;
@@ -1095,6 +1140,11 @@  static void acpi_table_check_fadt_pm1b_cnt_blk(fwts_framework *fw)
 
 static void acpi_table_check_fadt_pm2_cnt_blk(fwts_framework *fw)
 {
+	if (fadt->pm2_cnt_blk == 0 && fadt->header.length < 208) {
+		fwts_skipped(fw, "FADT PM2_CNT_BLK not being used.");
+		return;
+	}
+
 	if (fadt->pm2_cnt_blk == 0 && fadt->x_pm2_cnt_blk.address == 0) {
 		fwts_skipped(fw, "FADT PM2_CNT_BLK not being used.");
 		return;
@@ -1136,6 +1186,11 @@  static void acpi_table_check_fadt_pm2_cnt_blk(fwts_framework *fw)
 
 static void acpi_table_check_fadt_pm_tmr_blk(fwts_framework *fw)
 {
+	if (fadt->pm_tmr_blk == 0 && fadt->header.length < 220) {
+		fwts_skipped(fw, "FADT PM_TMR_BLK not being used.");
+		return;
+	}
+
 	if (fadt->pm_tmr_blk == 0 && fadt->x_pm_tmr_blk.address == 0) {
 		fwts_skipped(fw, "FADT PM_TMR_BLK not being used.");
 		return;