ACK: [c/azure][PATCH] scsi: storvsc: Fix a race in sub-channel creation that can cause panic

Seth Forshee seth.forshee at canonical.com
Tue Apr 9 14:08:43 UTC 2019


On Tue, Apr 09, 2019 at 10:55:27AM -0300, Marcelo Henrique Cerri wrote:
> From: Dexuan Cui <decui at microsoft.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1823805
> 
> We can concurrently try to open the same sub-channel from 2 paths:
> 
> path #1: vmbus_onoffer() -> vmbus_process_offer() -> handle_sc_creation().
> path #2: storvsc_probe() -> storvsc_connect_to_vsp() ->
> 	 -> storvsc_channel_init() -> handle_multichannel_storage() ->
> 	 -> vmbus_are_subchannels_present() -> handle_sc_creation().
> 
> They conflict with each other, but it was not an issue before the recent
> commit ae6935ed7d42 ("vmbus: split ring buffer allocation from open"),
> because at the beginning of vmbus_open() we checked newchannel->state so
> only one path could succeed, and the other would return with -EINVAL.
> 
> After ae6935ed7d42, the failing path frees the channel's ringbuffer by
> vmbus_free_ring(), and this causes a panic later.
> 
> Commit ae6935ed7d42 itself is good, and it just reveals the longstanding
> race. We can resolve the issue by removing path #2, i.e. removing the
> second vmbus_are_subchannels_present() in handle_multichannel_storage().
> 
> BTW, the comment "Check to see if sub-channels have already been created"
> in handle_multichannel_storage() is incorrect: when we unload the driver,
> we first close the sub-channel(s) and then close the primary channel, next
> the host sends rescind-offer message(s) so primary->sc_list will become
> empty. This means the first vmbus_are_subchannels_present() in
> handle_multichannel_storage() is never useful.
> 
> Fixes: ae6935ed7d42 ("vmbus: split ring buffer allocation from open")
> Cc: stable at vger.kernel.org
> Cc: Long Li <longli at microsoft.com>
> Cc: Stephen Hemminger <sthemmin at microsoft.com>
> Cc: K. Y. Srinivasan <kys at microsoft.com>
> Cc: Haiyang Zhang <haiyangz at microsoft.com>
> Signed-off-by: Dexuan Cui <decui at microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys at microsoft.com>
> Signed-off-by: Martin K. Petersen <martin.petersen at oracle.com>
> (cherry picked from commit c967590457cae5ba4f018704c341641bdcecfdcf)
> Signed-off-by: Marcelo Henrique Cerri <marcelo.cerri at canonical.com>

Upstream cherry pick which has been included in stable releases, fixes a
serious issue.

Acked-by: Seth Forshee <seth.forshee at canonical.com>



More information about the kernel-team mailing list