linux-kernel - Re: [Patch v3 2/2] memory: tegra: make sid and broadcast regions optional

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: <6506b2e8-c7f2-460d-b17d-55b731fac1ac@linaro.org>
Date: Tue, 23 Apr 2024 16:41:36 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Sumit Gupta <sumitg@...dia.com>, robh@...nel.org, conor+dt@...nel.org,
 maz@...nel.org, mark.rutland@....com, treding@...dia.com,
 jonathanh@...dia.com
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-tegra@...r.kernel.org, amhetre@...dia.com, bbasu@...dia.com
Subject: Re: [Patch v3 2/2] memory: tegra: make sid and broadcast regions
 optional

On 22/04/2024 16:36, Sumit Gupta wrote:
> 
>> On 12/04/2024 15:05, Sumit Gupta wrote:
>>> MC SID and Broadbast channel register access is restricted for Guest VM.
>>
>> Same typo
>>
> Thank you for catching. Will correct in v4.
> 
>>> In Tegra MC driver, consider both the regions as optional and skip
>>> access to restricted registers from Guest if a region is not present
>>> in Guest DT.
>>>
>>
>> ...
>>
>>>
>>>   static inline u32 mc_readl(const struct tegra_mc *mc, unsigned long offset)
>>> diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
>>> index 1b3183951bfe..716582255eeb 100644
>>> --- a/drivers/memory/tegra/tegra186.c
>>> +++ b/drivers/memory/tegra/tegra186.c
>>> @@ -26,20 +26,16 @@
>>>   static int tegra186_mc_probe(struct tegra_mc *mc)
>>>   {
>>>        struct platform_device *pdev = to_platform_device(mc->dev);
>>> +     struct resource *res;
>>>        unsigned int i;
>>> -     char name[8];
>>> +     char name[14];
>>
>> How is it relevant? I don't see this being used in your diff.
>>
>>
>> Best regards,
>> Krzysztof
>>
> 
> Did this change for below warning coming with 'W=1'.
> 
> ../drivers/memory/tegra/tegra186.c: In function tegra186_mc_probe:
> ../drivers/memory/tegra/tegra186.c:51:49: warning: %u directive output 
> may be truncated writing between 1 and 10 bytes into a region of size 6 
> [8;;https://gc
> c.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat-truncation=-Wformat-truncation=8;;]
>     51 |                 snprintf(name, sizeof(name), "ch%u", i);
>        |                                                 ^~
> ../drivers/memory/tegra/tegra186.c:51:46: note: directive argument in 
> the range [0, 4294967294]
>     51 |                 snprintf(name, sizeof(name), "ch%u", i);
>        |                                              ^~~~~~
> ../drivers/memory/tegra/tegra186.c:51:17: note: snprintf output between 
> 4 and 13 bytes into a destination of size 8
>     51 |                 snprintf(name, sizeof(name), "ch%u", i);
>        |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I asked how this is relevant to this change and you answer there is a
warning. If the warning was there, your answer is really just deflecting
the topic, so obviously this is new warning. Which part of code uses
longer name?

BTW, really, such answers do not make review of your code smoother.

Best regards,
Krzysztof