KY Srinivasan <kys@xxxxxxxxxxxxx> writes: >> -----Original Message----- >> From: Vitaly Kuznetsov [mailto:vkuznets@xxxxxxxxxx] >> Sent: Thursday, February 18, 2016 5:35 AM >> To: devel@xxxxxxxxxxxxxxxxxxxxxx >> Cc: KY Srinivasan <kys@xxxxxxxxxxxxx>; Haiyang Zhang >> <haiyangz@xxxxxxxxxxxxx>; Olaf Hering <olaf@xxxxxxxxx>; Cathy Avery >> <cavery@xxxxxxxxxx> >> Subject: [PATCH] Drivers: hv: util: fix a race with daemons startup >> >> Commit 3cace4a61610 ("Drivers: hv: utils: run polling callback always in >> interrupt context") removed direct *_transaction.state = HVUTIL_READY >> assignments from *_handle_handshake() functions introducing the following >> race: if a userspace daemon connects before we get first non-negotiation >> request from the server hv_poll_channel() won't set transaction state to >> HVUTIL_READY as (!channel) condition will fail, we set it to non-NULL on >> the first real request from the server. Solve the issue by transferring >> the transaction state to HVUTIL_READY directly from all handshake >> functions. >> >> Fixes: 3cace4a61610 ("Drivers: hv: utils: run polling callback always in >> interrupt context") >> Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> >> --- >> drivers/hv/hv_fcopy.c | 1 + >> drivers/hv/hv_kvp.c | 1 + >> drivers/hv/hv_snapshot.c | 1 + >> 3 files changed, 3 insertions(+) >> >> diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c >> index c37a71e..e18b85b 100644 >> --- a/drivers/hv/hv_fcopy.c >> +++ b/drivers/hv/hv_fcopy.c >> @@ -108,6 +108,7 @@ static int fcopy_handle_handshake(u32 version) >> return -EINVAL; >> } >> pr_debug("FCP: userspace daemon ver. %d registered\n", version); >> + fcopy_transaction.state = HVUTIL_READY; >> hv_poll_channel(fcopy_transaction.recv_channel, >> fcopy_poll_wrapper); >> return 0; >> } >> diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c >> index d4ab81b..1162afb 100644 >> --- a/drivers/hv/hv_kvp.c >> +++ b/drivers/hv/hv_kvp.c >> @@ -154,6 +154,7 @@ static int kvp_handle_handshake(struct hv_kvp_msg >> *msg) >> pr_debug("KVP: userspace daemon ver. %d registered\n", >> KVP_OP_REGISTER); >> kvp_register(dm_reg_value); >> + kvp_transaction.state = HVUTIL_READY; >> hv_poll_channel(kvp_transaction.recv_channel, kvp_poll_wrapper); >> >> return 0; >> diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c >> index 67def4a..489203b 100644 >> --- a/drivers/hv/hv_snapshot.c >> +++ b/drivers/hv/hv_snapshot.c >> @@ -113,6 +113,7 @@ static int vss_handle_handshake(struct hv_vss_msg >> *vss_msg) >> default: >> return -EINVAL; >> } >> + vss_transaction.state = HVUTIL_READY; >> hv_poll_channel(vss_transaction.recv_channel, vss_poll_wrapper); >> pr_debug("VSS: userspace daemon ver. %d registered\n", >> dm_reg_value); >> return 0; > > Vitaly, there is a simpler and a safer fix to this issue - stash away the channel pointer early on and so we don't > need to set it on each transaction. This would address the issue on > hand here. Yes, thanks, this is also an option. > This would also eliminate > setting the state only in the interrupt handler. I am putting together a batch of patches to send and if it is > ok with you, I will drop this patch and include a patch on the lines described above. > Thanks! -- Vitaly _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
