Re: [PATCH 1/2] drivers/media/platform/soc_camera/pxa_camera.c:reposition free_irq to avoid access to invalid data
From: Julia Lawall
Date: Mon Jan 07 2013 - 16:09:15 EST
On Mon, 7 Jan 2013, Robert Jarzmik wrote:
> Guennadi Liakhovetski <g.liakhovetski@xxxxxx> writes:
> > (adding Robert to CC)
> > I don't think any data is freed by pxa_free_dma(), it only disables DMA on
> > a certain channel. Theoretically there could be a different problem:
> > pxa_free_dma() deactivates DMA, whereas pxa_dma_start_channels() activates
> > it. But I think we're also protected against that: by the time
> > pxa_camera_remove() is called, and operation on the interface has been
> > stopped, client devices have been detached, pxa_camera_remove_device() has
> > been called, which has also stopped the interface clock. And with clock
> > stopped no interrupts can be generated. And the case of interrupt having
> > been generated before clk_disabled() and only delivered to the driver so
> > much later, that we're already unloading the module, seems really
> > impossible to me. Robert, you agree?
> Agreed that pxa_free_dma() doesn't free anything, that one is easy :)
> And agreed too for the second part, with a slighly different explanation :
> - pxa_camera_remove_device() has been called as you said
> - inside this function, check comment
> "/* disable capture, disable interrupts */"
> => this ensures no interrupt can be generated anymore
> So after pxa_camera_remove_device() has been called, no interrupts can be
> Yet as you said, it leaves the "almost impossible" scenario :
> - a user begins a capture
> - the user closes the capture device and unloads pxa-camera.ko:
> the IRQ line is asserted but doesn't trigger yet the interrupt handler
> (yes I know, improbable)
> meanwhile, IRQs are disabled, DMA channels are stopped
> => yes I know, impossible, the interrupt handler must be run before, but
> let's continue for love of discussion ...
> rmmod pxa-camera
> pxa_free_dma() * 3
> ----> here the IRQ handler kicks in !!!
> => pxa_camera_irq()
> ----> it hurts !
> My call is that this is impossible because the switch_to() should run the IRQ
> handler before pxa_camera_remove() is called.
> So all this to say that I think we're safe, unless a heavy ion or a cosmic ray
> strikes the PXA :)
Thanks for the explanation.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/