Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1645990

Re: work queue of scsi fc transports should be serialized

From Bart Van Assche <Bart.VanAssche@sandisk.com>
Newsgroups linux.kernel
Subject Re: work queue of scsi fc transports should be serialized
Date 2017-05-20 00:40 +0200
Message-ID <tIYIV-nK-5@gated-at.bofh.it> (permalink)
References <tIMy6-6E-3@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Fri, 2017-05-19 at 09:36 +0000, Dashi DS1 Cao wrote:
> It seems there is a race of multiple "fc_starget_delete" of the same rport,
> thus of the same SCSI host. The race leads to the race of scsi_remove_target
> and it cannot be prevented by the code snippet alone, even of the most recent
> version:
>         spin_lock_irqsave(shost->host_lock, flags);
>         list_for_each_entry(starget, &shost->__targets, siblings) {
>                 if (starget->state == STARGET_DEL ||
>                     starget->state == STARGET_REMOVE)
>                         continue;
> If there is a possibility that the starget is under deletion(state ==
> STARGET_DEL), it should be possible that list_next_entry(starget, siblings)
> could cause a read access violation.

Hello Dashi,

Something else must be going on. From scsi_remove_target():

restart:
	spin_lock_irqsave(shost->host_lock, flags);
	list_for_each_entry(starget, &shost->__targets, siblings) {
		if (starget->state == STARGET_DEL ||
		    starget->state == STARGET_REMOVE)
			continue;
		if (starget->dev.parent == dev || &starget->dev == dev) {
			kref_get(&starget->reap_ref);
			starget->state = STARGET_REMOVE;
			spin_unlock_irqrestore(shost->host_lock, flags);
			__scsi_remove_target(starget);
			scsi_target_reap(starget);
			goto restart;
		}
	}
	spin_unlock_irqrestore(shost->host_lock, flags);

In other words, before scsi_remove_target() decides to call
__scsi_remove_target(), it changes the target state into STARGET_REMOVE
while holding the host lock. This means that scsi_remove_target() won't
call __scsi_remove_target() twice and also that it won't invoke
list_next_entry(starget, siblings) after starget has been freed.

Bart.

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

work queue of scsi fc transports should be serialized  Dashi DS1 Cao <caods1@lenovo.com> - 2017-05-19 11:40 +0200
  Re: work queue of scsi fc transports should be serialized Bart Van Assche <Bart.VanAssche@sandisk.com> - 2017-05-20 00:40 +0200
    RE: work queue of scsi fc transports should be serialized Dashi DS1 Cao <caods1@lenovo.com> - 2017-05-20 10:30 +0200
      Re: work queue of scsi fc transports should be serialized Martin Wilck <mwilck@suse.com> - 2017-05-22 22:10 +0200

csiph-web