* [PATCH] PCI: endpoint: pci-epf-test: NULL check dma channels before release
@ 2025-09-12 7:11 Shin'ichiro Kawasaki
2025-09-12 7:31 ` Damien Le Moal
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Shin'ichiro Kawasaki @ 2025-09-12 7:11 UTC (permalink / raw)
To: linux-pci, Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I
Cc: Bjorn Helgaas, Frank Li, Niklas Cassel, Damien Le Moal,
Shin'ichiro Kawasaki
When endpoint controller driver is immature, the fields dma_chan_tx and
dma_chan_rx of the struct pci_epf_test could be NULL even after epf
initialization. However, pci_epf_test_clean_dma_chan() assumes that they
are always non-NULL valid values, and causes kernel panic when the
fields are NULL. To avoid the kernel panic, NULL check the fields before
release.
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
drivers/pci/endpoint/functions/pci-epf-test.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index e091193bd8a8..1c29d5dd4382 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -301,15 +301,20 @@ static void pci_epf_test_clean_dma_chan(struct pci_epf_test *epf_test)
if (!epf_test->dma_supported)
return;
- dma_release_channel(epf_test->dma_chan_tx);
- if (epf_test->dma_chan_tx == epf_test->dma_chan_rx) {
+ if (epf_test->dma_chan_tx) {
+ dma_release_channel(epf_test->dma_chan_tx);
+ if (epf_test->dma_chan_tx == epf_test->dma_chan_rx) {
+ epf_test->dma_chan_tx = NULL;
+ epf_test->dma_chan_rx = NULL;
+ return;
+ }
epf_test->dma_chan_tx = NULL;
- epf_test->dma_chan_rx = NULL;
- return;
}
- dma_release_channel(epf_test->dma_chan_rx);
- epf_test->dma_chan_rx = NULL;
+ if (epf_test->dma_chan_rx) {
+ dma_release_channel(epf_test->dma_chan_rx);
+ epf_test->dma_chan_rx = NULL;
+ }
}
static void pci_epf_test_print_rate(struct pci_epf_test *epf_test,
--
2.51.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] PCI: endpoint: pci-epf-test: NULL check dma channels before release
2025-09-12 7:11 [PATCH] PCI: endpoint: pci-epf-test: NULL check dma channels before release Shin'ichiro Kawasaki
@ 2025-09-12 7:31 ` Damien Le Moal
2025-09-12 9:59 ` Shinichiro Kawasaki
2025-09-12 10:38 ` Damien Le Moal
2025-09-12 16:58 ` Manivannan Sadhasivam
2 siblings, 1 reply; 7+ messages in thread
From: Damien Le Moal @ 2025-09-12 7:31 UTC (permalink / raw)
To: Shin'ichiro Kawasaki, linux-pci, Manivannan Sadhasivam,
Krzysztof Wilczyński, Kishon Vijay Abraham I
Cc: Bjorn Helgaas, Frank Li, Niklas Cassel
On 9/12/25 16:11, Shin'ichiro Kawasaki wrote:
> When endpoint controller driver is immature, the fields dma_chan_tx and
> dma_chan_rx of the struct pci_epf_test could be NULL even after epf
> initialization. However, pci_epf_test_clean_dma_chan() assumes that they
> are always non-NULL valid values, and causes kernel panic when the
> fields are NULL. To avoid the kernel panic, NULL check the fields before
> release.
>
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---
> drivers/pci/endpoint/functions/pci-epf-test.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index e091193bd8a8..1c29d5dd4382 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -301,15 +301,20 @@ static void pci_epf_test_clean_dma_chan(struct pci_epf_test *epf_test)
> if (!epf_test->dma_supported)
> return;
>
> - dma_release_channel(epf_test->dma_chan_tx);
> - if (epf_test->dma_chan_tx == epf_test->dma_chan_rx) {
> + if (epf_test->dma_chan_tx) {
> + dma_release_channel(epf_test->dma_chan_tx);
> + if (epf_test->dma_chan_tx == epf_test->dma_chan_rx) {
> + epf_test->dma_chan_tx = NULL;
> + epf_test->dma_chan_rx = NULL;
> + return;
> + }
> epf_test->dma_chan_tx = NULL;
> - epf_test->dma_chan_rx = NULL;
> - return;
> }
Can we simplify here ?
if (epf_test->dma_chan_tx) {
dma_release_channel(epf_test->dma_chan_tx);
epf_test->dma_chan_tx = NULL;
if (epf_test->dma_chan_tx == epf_test->dma_chan_rx) {
epf_test->dma_chan_rx = NULL;
return;
}
}
>
> - dma_release_channel(epf_test->dma_chan_rx);
> - epf_test->dma_chan_rx = NULL;
> + if (epf_test->dma_chan_rx) {
> + dma_release_channel(epf_test->dma_chan_rx);
> + epf_test->dma_chan_rx = NULL;
> + }
> }
>
> static void pci_epf_test_print_rate(struct pci_epf_test *epf_test,
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] PCI: endpoint: pci-epf-test: NULL check dma channels before release
2025-09-12 7:31 ` Damien Le Moal
@ 2025-09-12 9:59 ` Shinichiro Kawasaki
2025-09-12 10:38 ` Damien Le Moal
0 siblings, 1 reply; 7+ messages in thread
From: Shinichiro Kawasaki @ 2025-09-12 9:59 UTC (permalink / raw)
To: Damien Le Moal
Cc: linux-pci@vger.kernel.org, Manivannan Sadhasivam,
Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Frank Li, Niklas Cassel
On Sep 12, 2025 / 16:31, Damien Le Moal wrote:
> On 9/12/25 16:11, Shin'ichiro Kawasaki wrote:
> > When endpoint controller driver is immature, the fields dma_chan_tx and
> > dma_chan_rx of the struct pci_epf_test could be NULL even after epf
> > initialization. However, pci_epf_test_clean_dma_chan() assumes that they
> > are always non-NULL valid values, and causes kernel panic when the
> > fields are NULL. To avoid the kernel panic, NULL check the fields before
> > release.
> >
> > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> > ---
> > drivers/pci/endpoint/functions/pci-epf-test.c | 17 +++++++++++------
> > 1 file changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> > index e091193bd8a8..1c29d5dd4382 100644
> > --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> > @@ -301,15 +301,20 @@ static void pci_epf_test_clean_dma_chan(struct pci_epf_test *epf_test)
> > if (!epf_test->dma_supported)
> > return;
> >
> > - dma_release_channel(epf_test->dma_chan_tx);
> > - if (epf_test->dma_chan_tx == epf_test->dma_chan_rx) {
> > + if (epf_test->dma_chan_tx) {
> > + dma_release_channel(epf_test->dma_chan_tx);
> > + if (epf_test->dma_chan_tx == epf_test->dma_chan_rx) {
> > + epf_test->dma_chan_tx = NULL;
> > + epf_test->dma_chan_rx = NULL;
> > + return;
> > + }
> > epf_test->dma_chan_tx = NULL;
> > - epf_test->dma_chan_rx = NULL;
> > - return;
> > }
>
> Can we simplify here ?
I'm afraid, no,
>
> if (epf_test->dma_chan_tx) {
> dma_release_channel(epf_test->dma_chan_tx);
> epf_test->dma_chan_tx = NULL;
because the line above affects the comparison below.
> if (epf_test->dma_chan_tx == epf_test->dma_chan_rx) {
> epf_test->dma_chan_rx = NULL;
> return;
> }
> }
>
> >
> > - dma_release_channel(epf_test->dma_chan_rx);
> > - epf_test->dma_chan_rx = NULL;
> > + if (epf_test->dma_chan_rx) {
> > + dma_release_channel(epf_test->dma_chan_rx);
> > + epf_test->dma_chan_rx = NULL;
> > + }
> > }
> >
> > static void pci_epf_test_print_rate(struct pci_epf_test *epf_test,
>
>
> --
> Damien Le Moal
> Western Digital Research
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] PCI: endpoint: pci-epf-test: NULL check dma channels before release
2025-09-12 9:59 ` Shinichiro Kawasaki
@ 2025-09-12 10:38 ` Damien Le Moal
0 siblings, 0 replies; 7+ messages in thread
From: Damien Le Moal @ 2025-09-12 10:38 UTC (permalink / raw)
To: Shinichiro Kawasaki
Cc: linux-pci@vger.kernel.org, Manivannan Sadhasivam,
Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Frank Li, Niklas Cassel
On 9/12/25 18:59, Shinichiro Kawasaki wrote:
> On Sep 12, 2025 / 16:31, Damien Le Moal wrote:
>> On 9/12/25 16:11, Shin'ichiro Kawasaki wrote:
>>> When endpoint controller driver is immature, the fields dma_chan_tx and
>>> dma_chan_rx of the struct pci_epf_test could be NULL even after epf
>>> initialization. However, pci_epf_test_clean_dma_chan() assumes that they
>>> are always non-NULL valid values, and causes kernel panic when the
>>> fields are NULL. To avoid the kernel panic, NULL check the fields before
>>> release.
>>>
>>> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
>>> ---
>>> drivers/pci/endpoint/functions/pci-epf-test.c | 17 +++++++++++------
>>> 1 file changed, 11 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
>>> index e091193bd8a8..1c29d5dd4382 100644
>>> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
>>> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
>>> @@ -301,15 +301,20 @@ static void pci_epf_test_clean_dma_chan(struct pci_epf_test *epf_test)
>>> if (!epf_test->dma_supported)
>>> return;
>>>
>>> - dma_release_channel(epf_test->dma_chan_tx);
>>> - if (epf_test->dma_chan_tx == epf_test->dma_chan_rx) {
>>> + if (epf_test->dma_chan_tx) {
>>> + dma_release_channel(epf_test->dma_chan_tx);
>>> + if (epf_test->dma_chan_tx == epf_test->dma_chan_rx) {
>>> + epf_test->dma_chan_tx = NULL;
>>> + epf_test->dma_chan_rx = NULL;
>>> + return;
>>> + }
>>> epf_test->dma_chan_tx = NULL;
>>> - epf_test->dma_chan_rx = NULL;
>>> - return;
>>> }
>>
>> Can we simplify here ?
>
> I'm afraid, no,
>
>>
>> if (epf_test->dma_chan_tx) {
>> dma_release_channel(epf_test->dma_chan_tx);
>> epf_test->dma_chan_tx = NULL;
>
> because the line above affects the comparison below.
Arg... Of course ! Sorry about the noise.
>
>> if (epf_test->dma_chan_tx == epf_test->dma_chan_rx) {
>> epf_test->dma_chan_rx = NULL;
>> return;
>> }
>> }
>>
>>>
>>> - dma_release_channel(epf_test->dma_chan_rx);
>>> - epf_test->dma_chan_rx = NULL;
>>> + if (epf_test->dma_chan_rx) {
>>> + dma_release_channel(epf_test->dma_chan_rx);
>>> + epf_test->dma_chan_rx = NULL;
>>> + }
>>> }
>>>
>>> static void pci_epf_test_print_rate(struct pci_epf_test *epf_test,
>>
>>
>> --
>> Damien Le Moal
>> Western Digital Research
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: endpoint: pci-epf-test: NULL check dma channels before release
2025-09-12 7:11 [PATCH] PCI: endpoint: pci-epf-test: NULL check dma channels before release Shin'ichiro Kawasaki
2025-09-12 7:31 ` Damien Le Moal
@ 2025-09-12 10:38 ` Damien Le Moal
2025-09-12 16:58 ` Manivannan Sadhasivam
2 siblings, 0 replies; 7+ messages in thread
From: Damien Le Moal @ 2025-09-12 10:38 UTC (permalink / raw)
To: Shin'ichiro Kawasaki, linux-pci, Manivannan Sadhasivam,
Krzysztof Wilczyński, Kishon Vijay Abraham I
Cc: Bjorn Helgaas, Frank Li, Niklas Cassel
On 9/12/25 16:11, Shin'ichiro Kawasaki wrote:
> When endpoint controller driver is immature, the fields dma_chan_tx and
> dma_chan_rx of the struct pci_epf_test could be NULL even after epf
> initialization. However, pci_epf_test_clean_dma_chan() assumes that they
> are always non-NULL valid values, and causes kernel panic when the
> fields are NULL. To avoid the kernel panic, NULL check the fields before
> release.
>
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: endpoint: pci-epf-test: NULL check dma channels before release
2025-09-12 7:11 [PATCH] PCI: endpoint: pci-epf-test: NULL check dma channels before release Shin'ichiro Kawasaki
2025-09-12 7:31 ` Damien Le Moal
2025-09-12 10:38 ` Damien Le Moal
@ 2025-09-12 16:58 ` Manivannan Sadhasivam
2025-09-13 5:34 ` Shinichiro Kawasaki
2 siblings, 1 reply; 7+ messages in thread
From: Manivannan Sadhasivam @ 2025-09-12 16:58 UTC (permalink / raw)
To: Shin'ichiro Kawasaki
Cc: linux-pci, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Frank Li, Niklas Cassel, Damien Le Moal
On Fri, Sep 12, 2025 at 04:11:40PM GMT, Shin'ichiro Kawasaki wrote:
> When endpoint controller driver is immature, the fields dma_chan_tx and
> dma_chan_rx of the struct pci_epf_test could be NULL even after epf
> initialization. However, pci_epf_test_clean_dma_chan() assumes that they
> are always non-NULL valid values, and causes kernel panic when the
> fields are NULL. To avoid the kernel panic, NULL check the fields before
> release.
>
Have you seen the kernel panic or just predicting it by the code? If you have
seen it, it is better to add the logs, Fixes tag and CC stable list. Even if
not, Fixes tag would be needed since it is a bug fix.
- Mani
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---
> drivers/pci/endpoint/functions/pci-epf-test.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index e091193bd8a8..1c29d5dd4382 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -301,15 +301,20 @@ static void pci_epf_test_clean_dma_chan(struct pci_epf_test *epf_test)
> if (!epf_test->dma_supported)
> return;
>
> - dma_release_channel(epf_test->dma_chan_tx);
> - if (epf_test->dma_chan_tx == epf_test->dma_chan_rx) {
> + if (epf_test->dma_chan_tx) {
> + dma_release_channel(epf_test->dma_chan_tx);
> + if (epf_test->dma_chan_tx == epf_test->dma_chan_rx) {
> + epf_test->dma_chan_tx = NULL;
> + epf_test->dma_chan_rx = NULL;
> + return;
> + }
> epf_test->dma_chan_tx = NULL;
> - epf_test->dma_chan_rx = NULL;
> - return;
> }
>
> - dma_release_channel(epf_test->dma_chan_rx);
> - epf_test->dma_chan_rx = NULL;
> + if (epf_test->dma_chan_rx) {
> + dma_release_channel(epf_test->dma_chan_rx);
> + epf_test->dma_chan_rx = NULL;
> + }
> }
>
> static void pci_epf_test_print_rate(struct pci_epf_test *epf_test,
> --
> 2.51.0
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] PCI: endpoint: pci-epf-test: NULL check dma channels before release
2025-09-12 16:58 ` Manivannan Sadhasivam
@ 2025-09-13 5:34 ` Shinichiro Kawasaki
0 siblings, 0 replies; 7+ messages in thread
From: Shinichiro Kawasaki @ 2025-09-13 5:34 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: linux-pci@vger.kernel.org, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Frank Li, Niklas Cassel,
Damien Le Moal
On Sep 12, 2025 / 22:28, Manivannan Sadhasivam wrote:
> On Fri, Sep 12, 2025 at 04:11:40PM GMT, Shin'ichiro Kawasaki wrote:
> > When endpoint controller driver is immature, the fields dma_chan_tx and
> > dma_chan_rx of the struct pci_epf_test could be NULL even after epf
> > initialization. However, pci_epf_test_clean_dma_chan() assumes that they
> > are always non-NULL valid values, and causes kernel panic when the
> > fields are NULL. To avoid the kernel panic, NULL check the fields before
> > release.
> >
>
> Have you seen the kernel panic or just predicting it by the code? If you have
> seen it, it is better to add the logs, Fixes tag and CC stable list. Even if
> not, Fixes tag would be needed since it is a bug fix.
I saw the panic. Will add the kernel message I observed to the commit message
as well as Fixes and CC stable tags. Thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-09-13 5:34 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-12 7:11 [PATCH] PCI: endpoint: pci-epf-test: NULL check dma channels before release Shin'ichiro Kawasaki
2025-09-12 7:31 ` Damien Le Moal
2025-09-12 9:59 ` Shinichiro Kawasaki
2025-09-12 10:38 ` Damien Le Moal
2025-09-12 10:38 ` Damien Le Moal
2025-09-12 16:58 ` Manivannan Sadhasivam
2025-09-13 5:34 ` Shinichiro Kawasaki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox