linux-kernel - Re: [PATCH RFC RESEND] soc: pxa: ssp: Cast to enum pxa_ssp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <2295d860-44cf-4816-8a1a-54274974302f@app.fastmail.com>
Date: Thu, 04 Jan 2024 11:03:41 +0100
From: "Arnd Bergmann" <arnd@...db.de>
To: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
 Duje Mihanović <duje.mihanovic@...le.hr>
Cc: "Robert Jarzmik" <robert.jarzmik@...e.fr>,
 "Lubomir Rintel" <lkundrak@...sk>, "zhang.songyi" <zhang.songyi@....com.cn>,
 soc@...nel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH RFC RESEND] soc: pxa: ssp: Cast to enum pxa_ssp_type instead of int

On Thu, Jan 4, 2024, at 10:08, Uwe Kleine-König wrote:
> [adding lakml to Cc for wider audience]
>
> On Wed, Jan 03, 2024 at 10:06:03PM +0100, Duje Mihanović wrote:
>> On ARM64 platforms, id->data is a 64-bit value and casting it to a
>> 32-bit integer causes build errors. Cast it to the corresponding enum
>> instead.
>> 
>> Signed-off-by: Duje Mihanović <duje.mihanovic@...le.hr>
>> ---
>> This patch is necessary for my Marvell PXA1908 series to compile successfully
>> with allyesconfig:
>> https://lore.kernel.org/all/20231102-pxa1908-lkml-v7-0-cabb1a0cb52b@skole.hr/
>> ---
>>  drivers/soc/pxa/ssp.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/soc/pxa/ssp.c b/drivers/soc/pxa/ssp.c
>> index a1e8a07f7275..e2ffd8fd7e13 100644
>> --- a/drivers/soc/pxa/ssp.c
>> +++ b/drivers/soc/pxa/ssp.c
>> @@ -152,11 +152,11 @@ static int pxa_ssp_probe(struct platform_device *pdev)
>>  	if (dev->of_node) {
>>  		const struct of_device_id *id =
>>  			of_match_device(of_match_ptr(pxa_ssp_of_ids), dev);
>> -		ssp->type = (int) id->data;
>> +		ssp->type = (enum pxa_ssp_type) id->data;
>
> I wonder if this is a long-term fix. The error that the compiler throws
> is:
>
> 	drivers/soc/pxa/ssp.c:155:29: error: cast from pointer to integer of 
> different size [-Werror=pointer-to-int-cast]
> 	  155 |                 ssp->type = (int) id->data;
> 	      |                             ^
>
> enum pxa_ssp_type is an integer type, too, and its width could be
> smaller than 64 bit, too.

I would just change the cast to (uintptr_t), which is what we
have elsewhere.

> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index f458469c5ce5..fbe16089e4bb 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -283,7 +283,10 @@ struct of_device_id {
>  	char	name[32];
>  	char	type[32];
>  	char	compatible[128];
> -	const void *data;
> +	union {
> +		const void *data;
> +		kernel_ulong_t driver_data;
> +	};
>  };
> 
>  /* VIO */
>
> For this driver the change would be nice, as several casts can be
> dropped.

I think this is a nice idea in general, but I would keep
it separate from the bugfix, as we might want to do this on
a wider scale, or run into problems.

In particular, removing tons of casts to (kernel_ulong_t)
in other subsystems is probably more valuable than removing
casts to (void *) for of_device_id, since kernel_ulong_t
is particularly confusing, with a definition that is
completely unrelated to the similarly named __kernel_ulong_t.

      Arnd