From: Vitaly Kuznetsov <vkuznets@redhat.com>
To: "K. Y. Srinivasan" <kys@microsoft.com>, devel@linuxdriverproject.org
Cc: "Haiyang Zhang" <haiyangz@microsoft.com>,
linux-kernel@vger.kernel.org, "Dexuan Cui" <decui@microsoft.com>,
"Jason Wang" <jasowang@redhat.com>,
"Radim Krčmář" <rkrcmar@redhat.com>
Subject: [PATCH v2 3/3] Drivers: hv: vmbus: serialize Offer and Rescind offer
Date: Mon, 19 Jan 2015 17:56:13 +0100 [thread overview]
Message-ID: <1421686573-12771-4-git-send-email-vkuznets@redhat.com> (raw)
In-Reply-To: <1421686573-12771-1-git-send-email-vkuznets@redhat.com>
Commit 4b2f9abea52a ("staging: hv: convert channel_mgmt.c to not call
osd_schedule_callback")' was written under an assumption that we never receive
Rescind offer while we're still processing the initial Offer request. However,
the issue we fixed in 04a258c162a8 could be caused by this assumption not
always being true.
In particular, we need to protect against the following:
1) Receiving a Rescind offer after we do queue_work() for processing an Offer
request and before we actually enter vmbus_process_offer(). work.func points
to vmbus_process_offer() at this moment and in vmbus_onoffer_rescind() we do
another queue_work() without a check so we'll enter vmbus_process_offer()
twice.
2) Receiving a Rescind offer after we enter vmbus_process_offer() and
especially after we set >state = CHANNEL_OPEN_STATE. Many things can go
wrong in that case, e.g. we can call free_channel() while we're still using
it.
Implement the required protection by changing work->func at the very end of
vmbus_process_offer() and checking work->func in vmbus_onoffer_rescind(). In
case we receive rescind offer during or before vmbus_process_offer() is done
we set rescind flag to true and we check it at the end of vmbus_process_offer()
so such offer will not get lost.
Suggested-by: Radim Krčmář <rkrcmar@redhat.com>
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
drivers/hv/channel_mgmt.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index c526ed2..512b74a 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -279,9 +279,6 @@ static void vmbus_process_offer(struct work_struct *work)
int ret;
unsigned long flags;
- /* The next possible work is rescind handling */
- INIT_WORK(&newchannel->work, vmbus_process_rescind_offer);
-
/* Make sure this is a new offer */
spin_lock_irqsave(&vmbus_connection.channel_lock, flags);
@@ -383,6 +380,13 @@ static void vmbus_process_offer(struct work_struct *work)
goto error;
}
out:
+ spin_lock_irqsave(&newchannel->lock, flags);
+ /* The next possible work is rescind handling */
+ INIT_WORK(&newchannel->work, vmbus_process_rescind_offer);
+ /* Check if rescind offer was already received */
+ if (newchannel->rescind)
+ queue_work(newchannel->controlwq, &newchannel->work);
+ spin_unlock_irqrestore(&newchannel->lock, flags);
return;
error:
free_channel(newchannel);
@@ -520,6 +524,7 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
{
struct vmbus_channel_rescind_offer *rescind;
struct vmbus_channel *channel;
+ unsigned long flags;
rescind = (struct vmbus_channel_rescind_offer *)hdr;
channel = relid2channel(rescind->child_relid);
@@ -528,11 +533,21 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
/* Just return here, no channel found */
return;
+ spin_lock_irqsave(&channel->lock, flags);
channel->rescind = true;
+ if (channel->work.func != vmbus_process_rescind_offer)
+ /*
+ * We're still processing offer request, postponing rescind
+ * offer processing. It will be done when vmbus_process_offer()
+ * is done as it checks rescind flag.
+ */
+ goto out;
/* work is initialized for vmbus_process_rescind_offer() from
* vmbus_process_offer() where the channel got created */
queue_work(channel->controlwq, &channel->work);
+out:
+ spin_unlock_irqrestore(&channel->lock, flags);
}
/*
--
1.9.3
prev parent reply other threads:[~2015-01-19 16:56 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-19 16:56 [PATCH v2 0/3] Drivers: hv: vmbus: protect Offer/Rescind offer processing Vitaly Kuznetsov
2015-01-19 16:56 ` [PATCH v2 1/3] Drivers: hv: check vmbus_device_create() return value in vmbus_process_offer() Vitaly Kuznetsov
2015-01-19 17:58 ` Dan Carpenter
2015-01-19 18:03 ` Dan Carpenter
2015-01-20 9:43 ` Vitaly Kuznetsov
2015-01-20 10:51 ` Dan Carpenter
2015-01-19 16:56 ` [PATCH v2 2/3] Drivers: hv: rename sc_lock to the more generic lock Vitaly Kuznetsov
2015-01-19 16:56 ` Vitaly Kuznetsov [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1421686573-12771-4-git-send-email-vkuznets@redhat.com \
--to=vkuznets@redhat.com \
--cc=decui@microsoft.com \
--cc=devel@linuxdriverproject.org \
--cc=haiyangz@microsoft.com \
--cc=jasowang@redhat.com \
--cc=kys@microsoft.com \
--cc=linux-kernel@vger.kernel.org \
--cc=rkrcmar@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.