linux-kernel - Re: [PATCH] staging: media: atomisp: Fix premature setting of HMM_BO_DEVICE

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: <fbfbd0e5-2c27-4f32-a3d7-9cf57fde5098@kernel.org>
Date: Sun, 6 Jul 2025 20:25:52 +0200
From: Hans de Goede <hansg@...nel.org>
To: Abdelrahman Fekry <abdelrahmanfekry375@...il.com>
Cc: Andy Shevchenko <andriy.shevchenko@...el.com>,
 Andy Shevchenko <andy.shevchenko@...il.com>, andy@...nel.org,
 mchehab@...nel.org, sakari.ailus@...ux.intel.com,
 gregkh@...uxfoundation.org, linux-kernel-mentees@...ts.linux.dev,
 linux-kernel@...r.kernel.org, linux-media@...r.kernel.org,
 linux-staging@...ts.linux.dev, skhan@...uxfoundation.com,
 dan.carpenter@...aro.org
Subject: Re: [PATCH] staging: media: atomisp: Fix premature setting of
 HMM_BO_DEVICE_INITED flag

Hi, Abdelrahman

On 1-Jul-25 5:45 PM, Abdelrahman Fekry wrote:
> Hi Hans,
> 
> On Tue, Jul 1, 2025 at 4:54 PM Hans de Goede <hansg@...nel.org> wrote:

...

>> Actually I'm pretty sure that there will be quite a few
>> error-handling paths with bugs in the atomisp code given
>> its overall quality. But lets clean things up first, that
>> should make addressing any such cases easier.
>>
> I totally agree with this , i have submitted a patch that cleans the
> custom sysfs atrributes
> as you suggested as a beginning , the patch got reviewed by andy and dan
> here is the link
> https://lore.kernel.org/all/20250627100604.29061-1-abdelrahmanfekry375@gmail.com/
> 
> What do you think I should work on next after these two patches, do
> you have any suggestions?

The hmm_alloc code can use some more cleanups:

* hmm_get_mmu_base_addr() should be moved to drivers/staging/media/atomisp/pci/hmm/hmm.c
  and then the "struct hmm_bo_device bo_device;" in hmm.c can be made static

* hmm_init() sets hmm_initialized = true even on errors. It should
  immediately exit (return ret) on errors instead of continue-ing
  with calling hmm_alloc() even though hmm_bo_device_init() failed.

* I've checked the code and hmm_init() is called before any hmm_alloc()
  calls are made so the extra hmm_init() call in __hmm_alloc() can be
  dropped.

* After dropping the extra hmm_init() call in __hmm_alloc() the
  hmm_initialized flag can be removed since it is now no longer read
  anywhere.

* And maybe you'll find more possible cleanups while working on this

Regards,

Hans