Linux PCI subsystem development
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Ryder Lee <ryder.lee@mediatek.com>
Cc: "Bjorn Helgaas" <bhelgaas@google.com>,
	"Rob Herring" <robh@kernel.org>,
	"Jianjun Wang" <jianjun.wang@mediatek.com>,
	"Manivannan Sadhasivam" <mani@kernel.org>,
	"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
	"Lorenzo Pieralisi" <lpieralisi@kernel.org>,
	linux-mediatek@lists.infradead.org, linux-pci@vger.kernel.org
Subject: Re: [PATCH] PCI: mediatek: fix W=1 snpringf warnings
Date: Fri, 2 Oct 2026 18:39:45 -0500	[thread overview]
Message-ID: <20261002233945.GA406594@bhelgaas> (raw)
In-Reply-To: <b835e360b42c5e0994f9301a34dbdf140a8d3ef5.1772493898.git.ryder.lee@mediatek.com>

On Mon, Mar 02, 2026 at 05:46:48PM -0800, Ryder Lee wrote:
> Fix the following errors in W=1 builds.
> 
>   $ make W=1 drivers/pci/controller/pcie-mediatek.o
>     CALL    scripts/checksyscalls.sh
>     DESCEND objtool
>     INSTALL libsubcmd_headers
>     CC      drivers/pci/controller/pcie-mediatek.o
>   drivers/pci/controller/pcie-mediatek.c: In function ‘mtk_pcie_parse_port’:
>   drivers/pci/controller/pcie-mediatek.c:963:43: error: ‘%d’ directive output may be truncated writing between 1 and 10 bytes into a region of size 6 [-Werror=format-truncation=]
>     963 |         snprintf(name, sizeof(name), "port%d", slot);
> 	|                                           ^~
>   drivers/pci/controller/pcie-mediatek.c:963:38: note: directive argument in the range [0, 2147483647]
>     963 |         snprintf(name, sizeof(name), "port%d", slot);
> 	|                                      ^~~~~~~~
>   drivers/pci/controller/pcie-mediatek.c:963:9: note: ‘snprintf’ output between 6 and 15 bytes into a destination of size 10
>     963 |         snprintf(name, sizeof(name), "port%d", slot);
> 	|         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ...

> +++ b/drivers/pci/controller/pcie-mediatek.c
> @@ -953,7 +953,7 @@ static int mtk_pcie_parse_port(struct mtk_pcie *pcie,
>  	struct mtk_pcie_port *port;
>  	struct device *dev = pcie->dev;
>  	struct platform_device *pdev = to_platform_device(dev);
> -	char name[10];
> +	char name[20];
>  	int err;

FYI, from internal Sashiko review while backporting this:

[Severity: High]
This is a pre-existing issue, but looking at the error paths around port
parsing, is there a missing IRQ teardown that could lead to a use-after-free
or resource leak?

When mtk_pcie_parse_port() successfully parses a port, mtk_pcie_setup_irq()
allocates an IRQ domain and registers a chained IRQ handler that keeps a
pointer to the port struct.

If a subsequent initialization step fails, such as parsing another port in
mtk_pcie_setup() or pci_host_probe() failing in mtk_pcie_probe(), the probe
function aborts and returns an error:

mtk_pcie_setup() {
    ...
    err = mtk_pcie_parse_port(pcie, child, slot);
    if (err)
        return err;
    ...
}

The error paths, such as the put_resources label in mtk_pcie_probe() which
calls mtk_pcie_put_resources(), will free the memory of the port struct but
fail to unregister the chained IRQ handler or remove the IRQ domain.

Because of this, a freed port pointer remains registered as the handler data.
If the shared interrupt fires, could mtk_pcie_intr_handler() dereference this
freed pointer and crash?

Additionally, if platform_get_irq() fails in mtk_pcie_setup_irq(), it looks
like the function returns an error without destroying the just-created
port->irq_domain:

mtk_pcie_setup_irq() {
    ...
    if (port->irq < 0)
        return port->irq;
    ...
}

Would it be appropriate to add proper teardown functions to these error paths
to unregister the handlers and clean up the domains?

      parent reply	other threads:[~2026-10-02 23:39 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-03  1:46 [PATCH] PCI: mediatek: fix W=1 snpringf warnings Ryder Lee
2026-03-21 12:24 ` Manivannan Sadhasivam
2026-10-02 23:39 ` Bjorn Helgaas [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=20261002233945.GA406594@bhelgaas \
    --to=helgaas@kernel.org \
    --cc=bhelgaas@google.com \
    --cc=jianjun.wang@mediatek.com \
    --cc=kwilczynski@kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lpieralisi@kernel.org \
    --cc=mani@kernel.org \
    --cc=robh@kernel.org \
    --cc=ryder.lee@mediatek.com \
    /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