linux-kernel - Re: [PATCH] phy: sun4i-usb: Fix a W=1 compilation failure

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: <858ca657-3845-f65e-b9d8-9d7700e4a069@wanadoo.fr>
Date:   Mon, 4 Sep 2023 19:57:33 +0200
From:   Christophe JAILLET <christophe.jaillet@...adoo.fr>
To:     Andre Przywara <andre.przywara@....com>
Cc:     Vinod Koul <vkoul@...nel.org>,
        Kishon Vijay Abraham I <kishon@...nel.org>,
        Chen-Yu Tsai <wens@...e.org>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        Samuel Holland <samuel@...lland.org>,
        linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org,
        linux-phy@...ts.infradead.org,
        linux-arm-kernel@...ts.infradead.org, linux-sunxi@...ts.linux.dev
Subject: Re: [PATCH] phy: sun4i-usb: Fix a W=1 compilation failure

Le 04/09/2023 à 01:58, Andre Przywara a écrit :
> On Sun,  3 Sep 2023 12:11:06 +0200
> Christophe JAILLET <christophe.jaillet@...adoo.fr> wrote:
> 
>> With gcc 12.3.0, when this file is built, we get errors such as:
>>
>> drivers/phy/allwinner/phy-sun4i-usb.c: In function ‘sun4i_usb_phy_probe’:
>> drivers/phy/allwinner/phy-sun4i-usb.c:790:52: error: ‘_vbus’ directive output may be truncated writing 5 bytes into a region of size between 2 and 12 [-Werror=format-truncation=]
>>    790 |                 snprintf(name, sizeof(name), "usb%d_vbus", i);
>>        |                                                    ^~~~~
>> drivers/phy/allwinner/phy-sun4i-usb.c:790:17: note: ‘snprintf’ output between 10 and 20 bytes into a destination of size 16
>>    790 |                 snprintf(name, sizeof(name), "usb%d_vbus", i);
>>        |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Because of the possible value of 'i', this can't be an issue in real world
> 
> Would using "u8 i;" help? After all currently there are only 4 PHYs
> max, and in general this isn't expected to be more than a "handful", so
> 8 bits should be plenty. An unsigned is better anyway.
> It leaves a bit of a bitter taste, though, as we shouldn't do this kind
> type tweaking, especially not to work around the compiler trying to be
> clever, but then not seeing the whole picture (that "i" is bounded by
> compile time constants not exceeding "4").

data->cfg->num_phys is also an int, and having 'i' as an char is really 
unusual.

So, if changing the size of name (only to waste some stack in order to 
silence a compiler warning) is not acceptable, I think that the best is 
to leave things as-is.

CJ


> 
> Cheers,
> Andre
> 
>> application, but in order to have "make W=1" work correctly, give more
>> space for 'name'.
>>
>> Signed-off-by: Christophe JAILLET <christophe.jaillet@...adoo.fr>
>> ---
>>   drivers/phy/allwinner/phy-sun4i-usb.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
>> index ec551464dd4f..e53a9a9317bc 100644
>> --- a/drivers/phy/allwinner/phy-sun4i-usb.c
>> +++ b/drivers/phy/allwinner/phy-sun4i-usb.c
>> @@ -782,7 +782,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
>>   
>>   	for (i = 0; i < data->cfg->num_phys; i++) {
>>   		struct sun4i_usb_phy *phy = data->phys + i;
>> -		char name[16];
>> +		char name[32];
>>   
>>   		if (data->cfg->missing_phys & BIT(i))
>>   			continue;
> 
>