From mboxrd@z Thu Jan 1 00:00:00 1970 From: Oliver Neukum Subject: Re: [usb-storage] [PATCH] usb: storage: Fix a possible data race in uas_queuecommand_lck Date: Tue, 08 May 2018 10:27:50 +0200 Message-ID: <1525768070.24345.8.camel@suse.com> References: <20180508074743.13622-1-baijiaju1990@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20180508074743.13622-1-baijiaju1990@gmail.com> Sender: linux-kernel-owner@vger.kernel.org To: Jia-Ju Bai , stern@rowland.harvard.edu, gregkh@linuxfoundation.org Cc: linux-usb@vger.kernel.org, linux-scsi@vger.kernel.org, usb-storage@lists.one-eyed-alien.net, linux-kernel@vger.kernel.org List-Id: linux-scsi@vger.kernel.org Am Dienstag, den 08.05.2018, 15:47 +0800 schrieb Jia-Ju Bai: > The write operations to "cmnd->result" and "cmnd->scsi_done" > are protected by the lock on line 642-643, but the write operations > to these data on line 634-635 are not protected by the lock. > Thus, there may exist a data race for "cmnd->result" > and "cmnd->scsi_done". No, the write operations need no lock. The low level driver at this point owns the command. We cannot race with abort() for a command within queuecommand(). We take the lock where we take it to protect dev->resetting. I don't see why the scope of the lock would need to be enlarged. Regards Oliver > To fix this data race, the write operations on line 634-635 > should be also protected by the lock. > > Signed-off-by: Jia-Ju Bai Nacked-by: Oliver Neukum From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: usb: storage: Fix a possible data race in uas_queuecommand_lck From: Oliver Neukum Message-Id: <1525768070.24345.8.camel@suse.com> Date: Tue, 08 May 2018 10:27:50 +0200 To: Jia-Ju Bai , stern@rowland.harvard.edu, gregkh@linuxfoundation.org Cc: linux-usb@vger.kernel.org, linux-scsi@vger.kernel.org, usb-storage@lists.one-eyed-alien.net, linux-kernel@vger.kernel.org List-ID: QW0gRGllbnN0YWcsIGRlbiAwOC4wNS4yMDE4LCAxNTo0NyArMDgwMCBzY2hyaWViIEppYS1KdSBC YWk6Cj4gVGhlIHdyaXRlIG9wZXJhdGlvbnMgdG8gImNtbmQtPnJlc3VsdCIgYW5kICJjbW5kLT5z Y3NpX2RvbmUiIAo+IGFyZSBwcm90ZWN0ZWQgYnkgdGhlIGxvY2sgb24gbGluZSA2NDItNjQzLCBi dXQgdGhlIHdyaXRlIG9wZXJhdGlvbnMgCj4gdG8gdGhlc2UgZGF0YSBvbiBsaW5lIDYzNC02MzUg YXJlIG5vdCBwcm90ZWN0ZWQgYnkgdGhlIGxvY2suCj4gVGh1cywgdGhlcmUgbWF5IGV4aXN0IGEg ZGF0YSByYWNlIGZvciAiY21uZC0+cmVzdWx0IiAKPiBhbmQgImNtbmQtPnNjc2lfZG9uZSIuCgpO bywKCnRoZSB3cml0ZSBvcGVyYXRpb25zIG5lZWQgbm8gbG9jay4gVGhlIGxvdyBsZXZlbCBkcml2 ZXIgYXQgdGhpcyBwb2ludApvd25zIHRoZSBjb21tYW5kLiBXZSBjYW5ub3QgcmFjZSB3aXRoIGFi b3J0KCkgZm9yIGEgY29tbWFuZCB3aXRoaW4KcXVldWVjb21tYW5kKCkuIFdlIHRha2UgdGhlIGxv Y2sgd2hlcmUgd2UgdGFrZSBpdCB0byBwcm90ZWN0CmRldi0+cmVzZXR0aW5nLgoKSSBkb24ndCBz ZWUgd2h5IHRoZSBzY29wZSBvZiB0aGUgbG9jayB3b3VsZCBuZWVkIHRvIGJlIGVubGFyZ2VkLgoK CVJlZ2FyZHMKCQlPbGl2ZXIKCj4gVG8gZml4IHRoaXMgZGF0YSByYWNlLCB0aGUgd3JpdGUgb3Bl cmF0aW9ucyBvbiBsaW5lIDYzNC02MzUgCj4gc2hvdWxkIGJlIGFsc28gcHJvdGVjdGVkIGJ5IHRo ZSBsb2NrLgo+IAo+IFNpZ25lZC1vZmYtYnk6IEppYS1KdSBCYWkgPGJhaWppYWp1MTk5MEBnbWFp bC5jb20+Ck5hY2tlZC1ieTogT2xpdmVyIE5ldWt1bSA8b25ldWt1bUBzdXNlLmNvbT4KLS0tClRv IHVuc3Vic2NyaWJlIGZyb20gdGhpcyBsaXN0OiBzZW5kIHRoZSBsaW5lICJ1bnN1YnNjcmliZSBs aW51eC11c2IiIGluCnRoZSBib2R5IG9mIGEgbWVzc2FnZSB0byBtYWpvcmRvbW9Admdlci5rZXJu ZWwub3JnCk1vcmUgbWFqb3Jkb21vIGluZm8gYXQgIGh0dHA6Ly92Z2VyLmtlcm5lbC5vcmcvbWFq b3Jkb21vLWluZm8uaHRtbAo=