[dm-devel] dm-mpath: Clear map_context pointer when requeuing

[Hannes Reinecke]

Hi Jun'ichi,

On 12/01/2011 01:12 AM, Jun'ichi Nomura wrote:
> Hi Hannes,
> 
> On 11/30/11 23:49, Mike Snitzer wrote:
>> On Wed, Nov 30 2011 at  9:25am -0500,
>> Hannes Reinecke <hare at suse.de> wrote:
>>
>>> When requeing a request we should be clearing the map_context
>>> pointer, otherwise we might access an invalid memory location.
> 
> Could you elaborate on the mechanism how the map_context->ptr
> (= mpio) is accessed after freeing it?
> 
In short: No. Pure guesswork :-)

The longer answer here is that 'map_context' is managed by the
caller for multipath_map().
So in theory the caller is free to re-use the map_context whenever
'clone' is in use.
So if 'clone' is terminated when it's still requeued the caller
might be calling multipath_end_io(), at which point map_context->ptr
will be pointing to an invalid memory location.

But as I said, this is not a detailed analysis. It's good enough
for me that it solves the problem :-)

> mpio is known to be non-NULL where it is used. So clearing the pointer
> should not make any difference in logic.
> 
It does, see above.

> If this is a preventive change so that we can see NULL dereference
> instead of random invalid access if anything happens, it should be
> noted in the patch description and in the code.
> Otherwise, somebody looking at the code/change in future might be
> confused: "why we have to clear this pointer?"
> 
> And there are other places where mpio is freed.
> (E.g. in dispatch_queued_ios() in dm-mpath.c)
> Don't we need the same change there?
> 
I don't think so. It's just from multipath_map() where we need to
ensure map_context->ptr is correct. All the other places will not
touch the map_context->ptr again.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare at suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)