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@xxxxxxx> 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@xxxxxxx +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) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
