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@xxxxxxxxxx/
>
> 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
