Re: [PATCH RESEND] scsi: Add STARGET_CREATED_REMOVE state to scsi_target_state

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jun 27, 2017 at 02:55:58PM -0400, Ewan Milne wrote:
> From: "Ewan D. Milne" <emilne@xxxxxxxxxx>
> 
> The addition of the STARGET_REMOVE state had the side effect of
> introducing a race condition that can cause a crash.
> 
> scsi_target_reap_ref_release() checks the starget->state to
> see if it still in STARGET_CREATED, and if so, skips calling
> transport_remove_device() and device_del(), because the starget->state
> is only set to STARGET_RUNNING after scsi_target_add() has called
> device_add() and transport_add_device().
> 
> However, if an rport loss occurs while a target is being scanned,
> it can happen that scsi_remove_target() will be called while the
> starget is still in the STARGET_CREATED state.  In this case, the
> starget->state will be set to STARGET_REMOVE, and as a result,
> scsi_target_reap_ref_release() will take the wrong path.  The end
> result is a panic:

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@xxxxxxx>

Although we've been tampering with the target removal code for quite some
time now, so I really have the gut feeling we haven't really fixed the
root cause yet.

I once tried building a regression test for this (with qemu hot plugging UAS
devices) but that didn't really go far. Maybe we should add a scsi_target
to scsi_debug and add some methods to toggle remove it again. Just to have
a sensible unit test for that code path.

Byte,
	Johannes

-- 
Johannes Thumshirn                                          Storage
jthumshirn@xxxxxxx                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]