Re: linux-next: build failure after merge of the kspp tree

From: Hans de Goede <hdegoede@redhat.com>
To: Kees Cook <keescook@chromium.org>,
	Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Mark Gross <markgross@kernel.org>,
	Mark Pearson <markpearson@lenovo.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux Next Mailing List <linux-next@vger.kernel.org>
Subject: Re: linux-next: build failure after merge of the kspp tree
Date: Wed, 9 Mar 2022 17:58:56 +0100	[thread overview]
Message-ID: <0d4cdbbb-993b-55f3-355a-e0826ab45f46@redhat.com> (raw)
In-Reply-To: <202203090847.81E8862@keescook>

Hi,

On 3/9/22 17:52, Kees Cook wrote:
> On Wed, Mar 09, 2022 at 09:10:29PM +1100, Stephen Rothwell wrote:
>> Hi all,
>>
>> After merging the kspp tree, today's linux-next build (x86_64
>> allmodconfig) failed like this:
>>
>> In file included from include/linux/bitops.h:33,
>>                  from include/linux/kernel.h:22,
>>                  from drivers/platform/x86/thinkpad_acpi.c:37:
>> drivers/platform/x86/thinkpad_acpi.c: In function 'tpacpi_dytc_profile_init':
>> arch/x86/include/asm/bitops.h:207:22: error: array subscript 'long unsigned int[0]' is partly outside array bounds of 'int[1]' [-Werror=array-bounds]
>>   207 |                 (addr[nr >> _BITOPS_LONG_SHIFT])) != 0;
>>       |                 ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/platform/x86/thinkpad_acpi.c:10385:18: note: while referencing 'output'
>> 10385 |         int err, output;
>>       |                  ^~~~~~
>>
>> Actually the test_bit() in line 10409.
>>
>> Caused by commit
>>
>>   e1c21608e3cf ("platform/x86: thinkpad_acpi: Add PSC mode support")
>>
>> from the drivers-x86 tree interacting with the enablement of
>> -Werror=array-bounds.
>>
>> I have reverted that commit for today.

Thank you.

> Looking at https://lore.kernel.org/all/20220225182505.7234-1-markpearson@lenovo.com/
> 
> The problem is with the cast of "int output" to "unsigned long". Past
> fixes have either made the variable unsigned long, or adding a new
> variable and removal of casts. e.g. this pseudo-patch:
> 
> +	unsigned long bits;
> 
> 	...
> +	bits = output;
> -	if (test_bit(DYTC_FC_MMC, (void *)&output)) { /* MMC MODE */
> +	if (test_bit(DYTC_FC_MMC, &bits)) { /* MMC MODE */
> 
> ...
> 
> -	} else if (test_bit(DYTC_FC_PSC, (void *)&output)) { /*PSC MODE */
> +	} else if (test_bit(DYTC_FC_PSC, &bits)) { /*PSC MODE */

Right I was just preparing a patch for this, I've gone with:

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 7016c7fc3440..c476a78599d6 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -10399,7 +10399,7 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
 	if (err)
 		return err;
 
-	if (test_bit(DYTC_FC_MMC, (void *)&output)) { /* MMC MODE */
+	if (output & BIT(DYTC_FC_MMC)) { /* MMC MODE */
 		dytc_profile_available = DYTC_FUNCMODE_MMC;
 
 		/*
@@ -10412,7 +10412,7 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
 			if (!err && ((output & DYTC_ERR_MASK) == DYTC_ERR_SUCCESS))
 				dytc_mmc_get_available = true;
 		}
-	} else if (test_bit(DYTC_FC_PSC, (void *)&output)) { /*PSC MODE */
+	} else if (output & BIT(DYTC_FC_PSC)) { /* PSC MODE */
 		dytc_profile_available = DYTC_FUNCMODE_PSC;
 	} else {
 		dbg_printk(TPACPI_DBG_INIT, "No DYTC support available\n");

I'll push this to the drivers-x86 tree for-next branch.

Regards,

Hans


