| Message ID | 1484266817-6725-1-git-send-email-aospan@netup.ru |
|---|---|
| State | Not Applicable |
| Headers | show |
On 01/12/2017 04:20 PM, Abylay Ospan wrote: > pcie->dev->of_node not always defined (NULL) and can cause crash: > > [ 19.053195] Unable to handle kernel NULL pointer dereference at > virtual address 00000020 > [<c0b0370c>] (of_n_addr_cells) from [<c06599c4>] > (iproc_pcie_setup+0x30c/0xce0) > > this patch adds sanity check to prevent crash. Humm, how can it not be defined based on your earlier comment that you are using this on NSP which is Device Tree exclusively? I would agree if this was seen on e.g: MIPS/BCMA (47xx). > > Signed-off-by: Abylay Ospan <aospan@netup.ru> > --- > drivers/pci/host/pcie-iproc.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c > index 3ebc025..f2836a9 100644 > --- a/drivers/pci/host/pcie-iproc.c > +++ b/drivers/pci/host/pcie-iproc.c > @@ -952,6 +952,9 @@ static int pci_dma_range_parser_init(struct of_pci_range_parser *parser, > const int na = 3, ns = 2; > int rlen; > > + if (!node) > + return -ENOENT; > + > parser->node = node; > parser->pna = of_n_addr_cells(node); > parser->np = parser->pna + na + ns; >
On 1/12/2017 4:20 PM, Abylay Ospan wrote: > pcie->dev->of_node not always defined (NULL) and can cause crash: Ah I guess this can happen with the BCMA based platforms that do not use device tree for PCIe? > > [ 19.053195] Unable to handle kernel NULL pointer dereference at > virtual address 00000020 > [<c0b0370c>] (of_n_addr_cells) from [<c06599c4>] > (iproc_pcie_setup+0x30c/0xce0) > > this patch adds sanity check to prevent crash. > > Signed-off-by: Abylay Ospan <aospan@netup.ru> > --- > drivers/pci/host/pcie-iproc.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c > index 3ebc025..f2836a9 100644 > --- a/drivers/pci/host/pcie-iproc.c > +++ b/drivers/pci/host/pcie-iproc.c > @@ -952,6 +952,9 @@ static int pci_dma_range_parser_init(struct of_pci_range_parser *parser, > const int na = 3, ns = 2; > int rlen; > > + if (!node) > + return -ENOENT; > + Looks like a valid check to me. Acked-by: Ray Jui <ray.jui@broadcom.com> > parser->node = node; > parser->pna = of_n_addr_cells(node); > parser->np = parser->pna + na + ns; > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Florian, On 1/12/2017 4:22 PM, Florian Fainelli wrote: > On 01/12/2017 04:20 PM, Abylay Ospan wrote: >> pcie->dev->of_node not always defined (NULL) and can cause crash: >> >> [ 19.053195] Unable to handle kernel NULL pointer dereference at >> virtual address 00000020 >> [<c0b0370c>] (of_n_addr_cells) from [<c06599c4>] >> (iproc_pcie_setup+0x30c/0xce0) >> >> this patch adds sanity check to prevent crash. > > Humm, how can it not be defined based on your earlier comment that you > are using this on NSP which is Device Tree exclusively? I would agree if > this was seen on e.g: MIPS/BCMA (47xx). I thought Abylay mentioned: "Tested on Broadcom NorthStar machine ('Edgecore ECW7220-L') with two PCIe wifi adapters (b43 BCM4331 and ath10k QCA988X)." That is a NorthStar device which is BCMA based? > >> >> Signed-off-by: Abylay Ospan <aospan@netup.ru> >> --- >> drivers/pci/host/pcie-iproc.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c >> index 3ebc025..f2836a9 100644 >> --- a/drivers/pci/host/pcie-iproc.c >> +++ b/drivers/pci/host/pcie-iproc.c >> @@ -952,6 +952,9 @@ static int pci_dma_range_parser_init(struct of_pci_range_parser *parser, >> const int na = 3, ns = 2; >> int rlen; >> >> + if (!node) >> + return -ENOENT; >> + >> parser->node = node; >> parser->pna = of_n_addr_cells(node); >> parser->np = parser->pna + na + ns; >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/12/2017 04:48 PM, Ray Jui wrote: > Hi Florian, > > On 1/12/2017 4:22 PM, Florian Fainelli wrote: >> On 01/12/2017 04:20 PM, Abylay Ospan wrote: >>> pcie->dev->of_node not always defined (NULL) and can cause crash: >>> >>> [ 19.053195] Unable to handle kernel NULL pointer dereference at >>> virtual address 00000020 >>> [<c0b0370c>] (of_n_addr_cells) from [<c06599c4>] >>> (iproc_pcie_setup+0x30c/0xce0) >>> >>> this patch adds sanity check to prevent crash. >> >> Humm, how can it not be defined based on your earlier comment that you >> are using this on NSP which is Device Tree exclusively? I would agree if >> this was seen on e.g: MIPS/BCMA (47xx). > > I thought Abylay mentioned: > > "Tested on Broadcom NorthStar machine ('Edgecore ECW7220-L') with two > PCIe wifi > adapters (b43 BCM4331 and ath10k QCA988X)." > > That is a NorthStar device which is BCMA based? Still, upstream Linux support for Northstar is Device Tree, and BCMA bus should fill in of_nodes accordingly, if not, that's a bug that must be fixed at the BCMA layer. > >> >>> >>> Signed-off-by: Abylay Ospan <aospan@netup.ru> >>> --- >>> drivers/pci/host/pcie-iproc.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c >>> index 3ebc025..f2836a9 100644 >>> --- a/drivers/pci/host/pcie-iproc.c >>> +++ b/drivers/pci/host/pcie-iproc.c >>> @@ -952,6 +952,9 @@ static int pci_dma_range_parser_init(struct of_pci_range_parser *parser, >>> const int na = 3, ns = 2; >>> int rlen; >>> >>> + if (!node) >>> + return -ENOENT; >>> + >>> parser->node = node; >>> parser->pna = of_n_addr_cells(node); >>> parser->np = parser->pna + na + ns; >>> >> >>
FYI, here is my tree (based on linux-next): https://github.com/aospan/linux-next-bcm4708-edgecore-ecw7220-l/commits/master last patches adding defconfig and dts I'm using for this device. This files are draft yet. 2017-01-12 19:22 GMT-05:00 Florian Fainelli <f.fainelli@gmail.com>: > On 01/12/2017 04:20 PM, Abylay Ospan wrote: >> pcie->dev->of_node not always defined (NULL) and can cause crash: >> >> [ 19.053195] Unable to handle kernel NULL pointer dereference at >> virtual address 00000020 >> [<c0b0370c>] (of_n_addr_cells) from [<c06599c4>] >> (iproc_pcie_setup+0x30c/0xce0) >> >> this patch adds sanity check to prevent crash. > > Humm, how can it not be defined based on your earlier comment that you > are using this on NSP which is Device Tree exclusively? I would agree if > this was seen on e.g: MIPS/BCMA (47xx). > >> >> Signed-off-by: Abylay Ospan <aospan@netup.ru> >> --- >> drivers/pci/host/pcie-iproc.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c >> index 3ebc025..f2836a9 100644 >> --- a/drivers/pci/host/pcie-iproc.c >> +++ b/drivers/pci/host/pcie-iproc.c >> @@ -952,6 +952,9 @@ static int pci_dma_range_parser_init(struct of_pci_range_parser *parser, >> const int na = 3, ns = 2; >> int rlen; >> >> + if (!node) >> + return -ENOENT; >> + >> parser->node = node; >> parser->pna = of_n_addr_cells(node); >> parser->np = parser->pna + na + ns; >> > > > -- > Florian
On Fri, Jan 13, 2017 at 03:20:17AM +0300, Abylay Ospan wrote: > pcie->dev->of_node not always defined (NULL) and can cause crash: > > [ 19.053195] Unable to handle kernel NULL pointer dereference at > virtual address 00000020 > [<c0b0370c>] (of_n_addr_cells) from [<c06599c4>] > (iproc_pcie_setup+0x30c/0xce0) > > this patch adds sanity check to prevent crash. > > Signed-off-by: Abylay Ospan <aospan@netup.ru> Hi Abylay, this didn't look quite fully baked yet, so I'm dropping it for now. Please repost if everything's been worked out and the patch is still necessary. > --- > drivers/pci/host/pcie-iproc.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c > index 3ebc025..f2836a9 100644 > --- a/drivers/pci/host/pcie-iproc.c > +++ b/drivers/pci/host/pcie-iproc.c > @@ -952,6 +952,9 @@ static int pci_dma_range_parser_init(struct of_pci_range_parser *parser, > const int na = 3, ns = 2; > int rlen; > > + if (!node) > + return -ENOENT; > + > parser->node = node; > parser->pna = of_n_addr_cells(node); > parser->np = parser->pna + na + ns; > -- > 2.7.4 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c index 3ebc025..f2836a9 100644 --- a/drivers/pci/host/pcie-iproc.c +++ b/drivers/pci/host/pcie-iproc.c @@ -952,6 +952,9 @@ static int pci_dma_range_parser_init(struct of_pci_range_parser *parser, const int na = 3, ns = 2; int rlen; + if (!node) + return -ENOENT; + parser->node = node; parser->pna = of_n_addr_cells(node); parser->np = parser->pna + na + ns;
pcie->dev->of_node not always defined (NULL) and can cause crash: [ 19.053195] Unable to handle kernel NULL pointer dereference at virtual address 00000020 [<c0b0370c>] (of_n_addr_cells) from [<c06599c4>] (iproc_pcie_setup+0x30c/0xce0) this patch adds sanity check to prevent crash. Signed-off-by: Abylay Ospan <aospan@netup.ru> --- drivers/pci/host/pcie-iproc.c | 3 +++ 1 file changed, 3 insertions(+)