linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hannes Reinecke <hare@suse.de>
To: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
Cc: Mike Snitzer <snitzer@redhat.com>,
	linux-scsi@vger.kernel.org,
	James Bottomley <jbottomley@parallels.com>,
	"Alasdair G. Kergon" <agk@redhat.com>,
	dm-devel@redhat.com
Subject: Re: dm-mpath: Clear map_context pointer when requeuing
Date: Fri, 02 Dec 2011 17:19:27 +0100	[thread overview]
Message-ID: <4ED8FA8F.20109@suse.de> (raw)
In-Reply-To: <4ED6C67A.3060305@ce.jp.nec.com>

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

  reply	other threads:[~2011-12-02 16:19 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-30 14:25 [PATCH] dm-mpath: Clear map_context pointer when requeuing Hannes Reinecke
2011-11-30 14:49 ` Mike Snitzer
2011-12-01  0:12   ` Jun'ichi Nomura
2011-12-02 16:19     ` Hannes Reinecke [this message]
2011-12-05 10:49       ` Jun'ichi Nomura
2011-12-05 16:23         ` Hannes Reinecke
2011-12-06  4:44           ` Jun'ichi Nomura
2012-03-19 15:00             ` [dm-devel] " Alasdair G Kergon
2011-12-05 14:15 ` [dm-devel] [PATCH] " Dave Wysochanski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4ED8FA8F.20109@suse.de \
    --to=hare@suse.de \
    --cc=agk@redhat.com \
    --cc=dm-devel@redhat.com \
    --cc=j-nomura@ce.jp.nec.com \
    --cc=jbottomley@parallels.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=snitzer@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).