Hi Arnd,

Thanks for the review feedback.

On Thu, 20 Jun 2024 at 12:40, Arnd Bergmann [off-list ref] wrote:

On Thu, Jun 20, 2024, at 13:24, Peter Griffin wrote:

quoted

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
---
Changes in v2:
- Keep syscon lock held between checking and adding entry (Krzysztof)

Unfortunately you now have a different bug:

quoted

+     /* check if syscon entry already exists */
+     spin_lock(&syscon_list_slock);
+
+     list_for_each_entry(entry, &syscon_list, list)
+             if (entry->np == np) {
+                     syscon = entry;
+                     break;
+             }
+
+     if (syscon) {
+             ret = -EEXIST;
+             goto err_unlock;
+     }
+
+     syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
+     if (!syscon) {
+             ret = -ENOMEM;
+             goto err_unlock;
+     }

You can't use GFP_KERNEL while holding a spinlock.

I think the correct way to do this is to move the allocation
before the locked section, and then free it in the failure case.

Thanks for spotting this! I'll update as you suggest in v3.

Peter.