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: <82c60f1b-a52a-4bc7-a635-06198dba0e13@gmail.com>
Date: Mon, 7 Oct 2024 15:56:49 +0200
From: Christian König <ckoenig.leichtzumerken@...il.com>
To: Advait Dhamorikar <advaitdhamorikar@...il.com>,
 "Sundararaju, Sathishkumar" <sasundar@....com>
Cc: Alex Deucher <alexdeucher@...il.com>, alexander.deucher@....com,
 christian.koenig@....com, Xinhui.Pan@....com, airlied@...il.com,
 simona@...ll.ch, leo.liu@....com, sathishkumar.sundararaju@....com,
 saleemkhan.jamadar@....com, Veerabadhran.Gopalakrishnan@....com,
 sonny.jiang@....com, amd-gfx@...ts.freedesktop.org,
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
 skhan@...uxfoundation.org, anupnewsmail@...il.com,
 "Lazar, Lijo" <lijo.lazar@....com>
Subject: Re: [PATCH-next] Fix unintentional integer overflow

Am 05.10.24 um 09:05 schrieb Advait Dhamorikar:
> Hi Sathish,
>
>> Please collate the changes together with Lijo's suggestion as well,
>> "1ULL <<" instead of typecast, there are 3 occurrences of the error in
>> f0b19b84d391.
> I could only observe two instances of this error in f0b19b84d391 at:
> 'mask = (1 << (adev->jpeg.num_jpeg_inst * adev->jpeg.num_jpeg_rings)) - 1;`
> and `mask |= 1 << ((i * adev->jpeg.num_jpeg_rings) + j);`
>
> There are a few instances where we can use 1U instead of int as
> harvest_config uses unsigned int
> (adev->jpeg.harvest_config & (1 << i)
> However I think they should be fixed in a separate patch?

No, all of this are numerical problems where not taken into account the 
size of the destination type.

Saying that all of that are basically just style cleanups which doesn't 
need to be back-ported in any way, so please drop the Fixes: tag.

And you should probably change the subject line to something like 
"drm/amdgpu: cleanup shift coding style".

Regards,
Christian.

>
> Thanks and regards,
> Advait
>
> On Sat, 5 Oct 2024 at 09:05, Sundararaju, Sathishkumar <sasundar@....com> wrote:
>>
>>
>> On 10/4/2024 11:30 PM, Alex Deucher wrote:
>>> On Fri, Oct 4, 2024 at 5:15 AM Sundararaju, Sathishkumar
>>> <sasundar@....com> wrote:
>>>> All occurrences of this error fix should have been together in a single patch both in _get and _set callbacks corresponding to f0b19b84d391, please avoid separate patch for each occurrence.
>>>>
>>>> Sorry Alex, I missed to note this yesterday.
>>> I've dropped the patch.  Please pick it up once it's fixed up appropriately.
>> Thanks Alex.
>>
>> Hi Advait,
>> Please collate the changes together with Lijo's suggestion as well,
>> "1ULL <<" instead of typecast, there are 3 occurrences of the error in
>> f0b19b84d391.
>>
>> Regards,
>> Sathish
>>> Thanks,
>>>
>>> Alex
>>>
>>>> Regards,
>>>> Sathish
>>>>
>>>>
>>>> On 10/4/2024 1:46 PM, Advait Dhamorikar wrote:
>>>>
>>>> Fix shift-count-overflow when creating mask.
>>>> The expression's value may not be what the
>>>> programmer intended, because the expression is
>>>> evaluated using a narrower integer type.
>>>>
>>>> Fixes: f0b19b84d391 ("drm/amdgpu: add amdgpu_jpeg_sched_mask debugfs")
>>>> Signed-off-by: Advait Dhamorikar <advaitdhamorikar@...il.com>
>>>> ---
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
>>>> index 95e2796919fc..7df402c45f40 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
>>>> @@ -388,7 +388,7 @@ static int amdgpu_debugfs_jpeg_sched_mask_get(void *data, u64 *val)
>>>>     for (j = 0; j < adev->jpeg.num_jpeg_rings; ++j) {
>>>>     ring = &adev->jpeg.inst[i].ring_dec[j];
>>>>     if (ring->sched.ready)
>>>> - mask |= 1 << ((i * adev->jpeg.num_jpeg_rings) + j);
>>>> + mask |= (u64)1 << ((i * adev->jpeg.num_jpeg_rings) + j);
>>>>     }
>>>>     }
>>>>     *val = mask;


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ