[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <874nisbvsh.fsf@free.fr>
Date: Mon, 07 Jan 2013 21:08:46 +0100
From: Robert Jarzmik <robert.jarzmik@...e.fr>
To: Guennadi Liakhovetski <g.liakhovetski@....de>
Cc: Julia Lawall <Julia.Lawall@...6.fr>,
kernel-janitors@...r.kernel.org,
Mauro Carvalho Chehab <mchehab@...hat.com>,
Linux Media Mailing List <linux-media@...r.kernel.org>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] drivers/media/platform/soc_camera/pxa_camera.c: reposition free_irq to avoid access to invalid data
Guennadi Liakhovetski <g.liakhovetski@....de> 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
generated.
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:
soc_camera_close()
pxa_camera_remove_device()
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
switch_to(rmmod)
=> yes I know, impossible, the interrupt handler must be run before, but
let's continue for love of discussion ...
rmmod pxa-camera
pxa_camera_remove()
pxa_free_dma() * 3
----> here the IRQ handler kicks in !!!
=> pxa_camera_irq()
pxa_dma_start_channels()
----> 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 :)
Cheers.
--
Robert
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists