* [PATCH] dma: mv_xor: fix kernel crash on probe error @ 2013-12-12 21:11 Aaro Koskinen 2013-12-12 21:15 ` Jason Cooper 2013-12-12 22:42 ` Russell King - ARM Linux 0 siblings, 2 replies; 7+ messages in thread From: Aaro Koskinen @ 2013-12-12 21:11 UTC (permalink / raw) To: linux-arm-kernel If the non-DT channel add path fails, the kernel will crash as the channel is not set to NULL and it will try to release the channel using the error value. Fix that. Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi> --- drivers/dma/mv_xor.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c index 7807f0ef4e20..2cb35a62c7f0 100644 --- a/drivers/dma/mv_xor.c +++ b/drivers/dma/mv_xor.c @@ -1227,6 +1227,7 @@ static int mv_xor_probe(struct platform_device *pdev) cd->cap_mask, irq); if (IS_ERR(xordev->channels[i])) { ret = PTR_ERR(xordev->channels[i]); + xordev->channels[i] = NULL; goto err_channel_add; } } -- 1.8.5.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] dma: mv_xor: fix kernel crash on probe error 2013-12-12 21:11 [PATCH] dma: mv_xor: fix kernel crash on probe error Aaro Koskinen @ 2013-12-12 21:15 ` Jason Cooper 2013-12-12 22:42 ` Russell King - ARM Linux 1 sibling, 0 replies; 7+ messages in thread From: Jason Cooper @ 2013-12-12 21:15 UTC (permalink / raw) To: linux-arm-kernel On Thu, Dec 12, 2013 at 11:11:55PM +0200, Aaro Koskinen wrote: > If the non-DT channel add path fails, the kernel will crash as the > channel is not set to NULL and it will try to release the channel using > the error value. Fix that. > > Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi> > --- > drivers/dma/mv_xor.c | 1 + > 1 file changed, 1 insertion(+) Acked-by: Jason Cooper <jason@lakedaemon.net> thx, Jason. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] dma: mv_xor: fix kernel crash on probe error 2013-12-12 21:11 [PATCH] dma: mv_xor: fix kernel crash on probe error Aaro Koskinen 2013-12-12 21:15 ` Jason Cooper @ 2013-12-12 22:42 ` Russell King - ARM Linux 2013-12-12 23:36 ` Dan Williams 1 sibling, 1 reply; 7+ messages in thread From: Russell King - ARM Linux @ 2013-12-12 22:42 UTC (permalink / raw) To: linux-arm-kernel On Thu, Dec 12, 2013 at 11:11:55PM +0200, Aaro Koskinen wrote: > If the non-DT channel add path fails, the kernel will crash as the > channel is not set to NULL and it will try to release the channel using > the error value. Fix that. > > Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi> > --- > drivers/dma/mv_xor.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c > index 7807f0ef4e20..2cb35a62c7f0 100644 > --- a/drivers/dma/mv_xor.c > +++ b/drivers/dma/mv_xor.c > @@ -1227,6 +1227,7 @@ static int mv_xor_probe(struct platform_device *pdev) > cd->cap_mask, irq); > if (IS_ERR(xordev->channels[i])) { > ret = PTR_ERR(xordev->channels[i]); > + xordev->channels[i] = NULL; > goto err_channel_add; > } > } Yes, I found this too, and although this is _a_ fix, it's not my preferred. I'd much prefer this instead - avoid writing invalid channels to xordev->channels[i] in the first place... Slightly larger patch but IMHO more correct. diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c index 7807f0ef4e20..a7e91090443e 100644 --- a/drivers/dma/mv_xor.c +++ b/drivers/dma/mv_xor.c @@ -1176,6 +1176,7 @@ static int mv_xor_probe(struct platform_device *pdev) int i = 0; for_each_child_of_node(pdev->dev.of_node, np) { + struct mv_xor_chan *chan; dma_cap_mask_t cap_mask; int irq; @@ -1193,21 +1194,21 @@ static int mv_xor_probe(struct platform_device *pdev) goto err_channel_add; } - xordev->channels[i] = - mv_xor_channel_add(xordev, pdev, i, - cap_mask, irq); - if (IS_ERR(xordev->channels[i])) { - ret = PTR_ERR(xordev->channels[i]); - xordev->channels[i] = NULL; + chan = mv_xor_channel_add(xordev, pdev, i, + cap_mask, irq); + if (IS_ERR(chan)) { + ret = PTR_ERR(chan); irq_dispose_mapping(irq); goto err_channel_add; } + xordev->channels[i] = chan; i++; } } else if (pdata && pdata->channels) { for (i = 0; i < MV_XOR_MAX_CHANNELS; i++) { struct mv_xor_channel_data *cd; + struct mv_xor_chan *chan; int irq; cd = &pdata->channels[i]; @@ -1222,13 +1223,14 @@ static int mv_xor_probe(struct platform_device *pdev) goto err_channel_add; } - xordev->channels[i] = - mv_xor_channel_add(xordev, pdev, i, - cd->cap_mask, irq); - if (IS_ERR(xordev->channels[i])) { - ret = PTR_ERR(xordev->channels[i]); + chan = mv_xor_channel_add(xordev, pdev, i, + cd->cap_mask, irq); + if (IS_ERR(chan)) { + ret = PTR_ERR(chan); goto err_channel_add; } + + xordev->channels[i] = chan; } } ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] dma: mv_xor: fix kernel crash on probe error 2013-12-12 22:42 ` Russell King - ARM Linux @ 2013-12-12 23:36 ` Dan Williams 2013-12-12 23:44 ` Aaro Koskinen 0 siblings, 1 reply; 7+ messages in thread From: Dan Williams @ 2013-12-12 23:36 UTC (permalink / raw) To: linux-arm-kernel On Thu, Dec 12, 2013 at 2:42 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Thu, Dec 12, 2013 at 11:11:55PM +0200, Aaro Koskinen wrote: >> If the non-DT channel add path fails, the kernel will crash as the >> channel is not set to NULL and it will try to release the channel using >> the error value. Fix that. >> >> Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi> >> --- >> drivers/dma/mv_xor.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c >> index 7807f0ef4e20..2cb35a62c7f0 100644 >> --- a/drivers/dma/mv_xor.c >> +++ b/drivers/dma/mv_xor.c >> @@ -1227,6 +1227,7 @@ static int mv_xor_probe(struct platform_device *pdev) >> cd->cap_mask, irq); >> if (IS_ERR(xordev->channels[i])) { >> ret = PTR_ERR(xordev->channels[i]); >> + xordev->channels[i] = NULL; >> goto err_channel_add; >> } >> } > > Yes, I found this too, and although this is _a_ fix, it's not my > preferred. I'd much prefer this instead - avoid writing invalid > channels to xordev->channels[i] in the first place... Slightly > larger patch but IMHO more correct. > Looks good to me. Aaro? Can I have one with a sign-off? ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] dma: mv_xor: fix kernel crash on probe error 2013-12-12 23:36 ` Dan Williams @ 2013-12-12 23:44 ` Aaro Koskinen 2013-12-12 23:59 ` Russell King - ARM Linux 0 siblings, 1 reply; 7+ messages in thread From: Aaro Koskinen @ 2013-12-12 23:44 UTC (permalink / raw) To: linux-arm-kernel On Thu, Dec 12, 2013 at 03:36:13PM -0800, Dan Williams wrote: > On Thu, Dec 12, 2013 at 2:42 PM, Russell King - ARM Linux > <linux@arm.linux.org.uk> wrote: > > On Thu, Dec 12, 2013 at 11:11:55PM +0200, Aaro Koskinen wrote: > >> If the non-DT channel add path fails, the kernel will crash as the > >> channel is not set to NULL and it will try to release the channel using > >> the error value. Fix that. > >> > >> Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi> > >> --- > >> drivers/dma/mv_xor.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c > >> index 7807f0ef4e20..2cb35a62c7f0 100644 > >> --- a/drivers/dma/mv_xor.c > >> +++ b/drivers/dma/mv_xor.c > >> @@ -1227,6 +1227,7 @@ static int mv_xor_probe(struct platform_device *pdev) > >> cd->cap_mask, irq); > >> if (IS_ERR(xordev->channels[i])) { > >> ret = PTR_ERR(xordev->channels[i]); > >> + xordev->channels[i] = NULL; > >> goto err_channel_add; > >> } > >> } > > > > Yes, I found this too, and although this is _a_ fix, it's not my > > preferred. I'd much prefer this instead - avoid writing invalid > > channels to xordev->channels[i] in the first place... Slightly > > larger patch but IMHO more correct. > > > > Looks good to me. Aaro? I'm also fine this version. Just to be sure, I tested the error path on my Marvell OpenRD client, so you can put my Tested-by to the final patch. A. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] dma: mv_xor: fix kernel crash on probe error 2013-12-12 23:44 ` Aaro Koskinen @ 2013-12-12 23:59 ` Russell King - ARM Linux 2013-12-18 16:35 ` Vinod Koul 0 siblings, 1 reply; 7+ messages in thread From: Russell King - ARM Linux @ 2013-12-12 23:59 UTC (permalink / raw) To: linux-arm-kernel From: Russell King <rmk+kernel@arm.linux.org.uk> Subject: [PATCH] dmaengine: mv_xor: fix oops when channels fail to initialise When a channel fails to initialise, we error out and clean up any previously unregistered channels by walking the entire xordev->channels array. Unfortunately, there are paths which end up storing an error pointer in this array, which we then try and dereference in the cleanup code, which causes an oops. Fix this by avoiding writing invalid pointers to this array in the first place. Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> --- drivers/dma/mv_xor.c | 24 +++++++++++++----------- 1 files changed, 13 insertions(+), 11 deletions(-) diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c index 7807f0ef4e20..a7e91090443e 100644 --- a/drivers/dma/mv_xor.c +++ b/drivers/dma/mv_xor.c @@ -1176,6 +1176,7 @@ static int mv_xor_probe(struct platform_device *pdev) int i = 0; for_each_child_of_node(pdev->dev.of_node, np) { + struct mv_xor_chan *chan; dma_cap_mask_t cap_mask; int irq; @@ -1193,21 +1194,21 @@ static int mv_xor_probe(struct platform_device *pdev) goto err_channel_add; } - xordev->channels[i] = - mv_xor_channel_add(xordev, pdev, i, - cap_mask, irq); - if (IS_ERR(xordev->channels[i])) { - ret = PTR_ERR(xordev->channels[i]); - xordev->channels[i] = NULL; + chan = mv_xor_channel_add(xordev, pdev, i, + cap_mask, irq); + if (IS_ERR(chan)) { + ret = PTR_ERR(chan); irq_dispose_mapping(irq); goto err_channel_add; } + xordev->channels[i] = chan; i++; } } else if (pdata && pdata->channels) { for (i = 0; i < MV_XOR_MAX_CHANNELS; i++) { struct mv_xor_channel_data *cd; + struct mv_xor_chan *chan; int irq; cd = &pdata->channels[i]; @@ -1222,13 +1223,14 @@ static int mv_xor_probe(struct platform_device *pdev) goto err_channel_add; } - xordev->channels[i] = - mv_xor_channel_add(xordev, pdev, i, - cd->cap_mask, irq); - if (IS_ERR(xordev->channels[i])) { - ret = PTR_ERR(xordev->channels[i]); + chan = mv_xor_channel_add(xordev, pdev, i, + cd->cap_mask, irq); + if (IS_ERR(chan)) { + ret = PTR_ERR(chan); goto err_channel_add; } + + xordev->channels[i] = chan; } } ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] dma: mv_xor: fix kernel crash on probe error 2013-12-12 23:59 ` Russell King - ARM Linux @ 2013-12-18 16:35 ` Vinod Koul 0 siblings, 0 replies; 7+ messages in thread From: Vinod Koul @ 2013-12-18 16:35 UTC (permalink / raw) To: linux-arm-kernel On Thu, Dec 12, 2013 at 11:59:08PM +0000, Russell King - ARM Linux wrote: > From: Russell King <rmk+kernel@arm.linux.org.uk> > Subject: [PATCH] dmaengine: mv_xor: fix oops when channels fail to initialise > > When a channel fails to initialise, we error out and clean up any > previously unregistered channels by walking the entire xordev->channels > array. Unfortunately, there are paths which end up storing an error > pointer in this array, which we then try and dereference in the cleanup > code, which causes an oops. > > Fix this by avoiding writing invalid pointers to this array in the first > place. > > Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi> > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> Acked-by: Vinod Koul <vinod.koul@intel.com> -- ~Vinod > --- > drivers/dma/mv_xor.c | 24 +++++++++++++----------- > 1 files changed, 13 insertions(+), 11 deletions(-) > > diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c > index 7807f0ef4e20..a7e91090443e 100644 > --- a/drivers/dma/mv_xor.c > +++ b/drivers/dma/mv_xor.c > @@ -1176,6 +1176,7 @@ static int mv_xor_probe(struct platform_device *pdev) > int i = 0; > > for_each_child_of_node(pdev->dev.of_node, np) { > + struct mv_xor_chan *chan; > dma_cap_mask_t cap_mask; > int irq; > > @@ -1193,21 +1194,21 @@ static int mv_xor_probe(struct platform_device *pdev) > goto err_channel_add; > } > > - xordev->channels[i] = > - mv_xor_channel_add(xordev, pdev, i, > - cap_mask, irq); > - if (IS_ERR(xordev->channels[i])) { > - ret = PTR_ERR(xordev->channels[i]); > - xordev->channels[i] = NULL; > + chan = mv_xor_channel_add(xordev, pdev, i, > + cap_mask, irq); > + if (IS_ERR(chan)) { > + ret = PTR_ERR(chan); > irq_dispose_mapping(irq); > goto err_channel_add; > } > > + xordev->channels[i] = chan; > i++; > } > } else if (pdata && pdata->channels) { > for (i = 0; i < MV_XOR_MAX_CHANNELS; i++) { > struct mv_xor_channel_data *cd; > + struct mv_xor_chan *chan; > int irq; > > cd = &pdata->channels[i]; > @@ -1222,13 +1223,14 @@ static int mv_xor_probe(struct platform_device *pdev) > goto err_channel_add; > } > > - xordev->channels[i] = > - mv_xor_channel_add(xordev, pdev, i, > - cd->cap_mask, irq); > - if (IS_ERR(xordev->channels[i])) { > - ret = PTR_ERR(xordev->channels[i]); > + chan = mv_xor_channel_add(xordev, pdev, i, > + cd->cap_mask, irq); > + if (IS_ERR(chan)) { > + ret = PTR_ERR(chan); > goto err_channel_add; > } > + > + xordev->channels[i] = chan; > } > } > -- ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-12-18 16:35 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-12-12 21:11 [PATCH] dma: mv_xor: fix kernel crash on probe error Aaro Koskinen 2013-12-12 21:15 ` Jason Cooper 2013-12-12 22:42 ` Russell King - ARM Linux 2013-12-12 23:36 ` Dan Williams 2013-12-12 23:44 ` Aaro Koskinen 2013-12-12 23:59 ` Russell King - ARM Linux 2013-12-18 16:35 ` Vinod Koul
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).