Linux PCI subsystem development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Pramod Maurya" <pramod.nexgen@gmail.com>
Cc: linux-pci@vger.kernel.org
Subject: Re: [PATCH v1 1/1] PCI: ibmphp: Simplify ibmphp_init_devno() to take struct slot *
Date: Fri, 15 May 2026 11:28:43 +0000	[thread overview]
Message-ID: <20260515112844.7BAADC2BCB0@smtp.kernel.org> (raw)
In-Reply-To: <20260515110944.216993-2-pramod.nexgen@gmail.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unbalanced Lifecycle Operations / NULL Pointer Dereference on module initialization failure.
- [Low] The patch resolves a documented TODO item by simplifying the signature of ibmphp_init_devno(), but fails to remove the corresponding entry from the drivers/pci/hotplug/TODO file.
--

Note: The standard inline review format has been altered to a free-form
summary due to recitation restrictions. No direct quotes from the patch are
included.

commit 10dfdd5d4dd69d8f479f459382630a4bba2b9d73
Author: Pramod Maurya <pramod.nexgen@gmail.com>

PCI: ibmphp: Simplify ibmphp_init_devno() to take struct slot *

This commit simplifies the ibmphp_init_devno() function by changing its
parameter from a double pointer to a single pointer. It updates all
internal accesses and the call site in ebda_rsrc_controller() to pass the
pointer directly.

This isn't a bug, but since this patch successfully simplifies the function
signature, should the corresponding task also be removed from the
drivers/pci/hotplug/TODO file? Leaving the resolved item in the list might
cause some confusion for future cleanups.

Additionally, while reviewing the call site in ebda_rsrc_controller(), I
noticed an existing issue in the error handling path. If
ibmphp_init_devno() succeeds for one slot but fails for a subsequent slot,
the code takes the goto error path. The successfully processed slot remains
in the global ibmphp_slot_head list, but the pci_hp_register() loop is
bypassed.

Does this cause a panic during module initialization failure? When the
error cascades up to ibmphp_init(), it appears to invoke ibmphp_unload()
and free_slots(). Since pci_hp_register() was never called for the first
slot, its hotplug_slot.pci_slot remains null and its slot_list remains
uninitialized. Calling pci_hp_del() and pci_hp_destroy() on this slot
looks like it would dereference these invalid pointers.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260515110944.216993-2-pramod.nexgen@gmail.com?part=1

      reply	other threads:[~2026-05-15 11:28 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-15 11:09 [PATCH v1 0/1] PCI: ibmphp: Simplify ibmphp_init_devno() parameter Pramod Maurya
2026-05-15 11:09 ` [PATCH v1 1/1] PCI: ibmphp: Simplify ibmphp_init_devno() to take struct slot * Pramod Maurya
2026-05-15 11:28   ` sashiko-bot [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260515112844.7BAADC2BCB0@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=pramod.nexgen@gmail.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox