[PATCH 1/1] vmbus: hvsock: add proper sync for vmbus_hvsock_device_unregister()

[PATCH 1/1] vmbus: hvsock: add proper sync for vmbus_hvsock_device_unregister()

[kys]

From: Dexuan Cui <decui@microsoft.com>

Without the patch, vmbus_hvsock_device_unregister() can destroy the device
prematurely when close() is called, and can cause NULl dereferencing or
potential data loss (the last portion of the data stream may be dropped
prematurely).

Please consider this for 4.14.

Signed-off-by: Dexuan Cui <decui@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel_mgmt.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 018d2e0f8ec5..379b0df123be 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -937,7 +937,10 @@ void vmbus_hvsock_device_unregister(struct vmbus_channel *channel)
 {
 	BUG_ON(!is_hvsock_channel(channel));
 
-	channel->rescind = true;
+	/* We always get a rescind msg when a connection is closed. */
+	while (!READ_ONCE(channel->probe_done) || !READ_ONCE(channel->rescind))
+		msleep(1);
+
 	vmbus_device_unregister(channel->device_obj);
 }
 EXPORT_SYMBOL_GPL(vmbus_hvsock_device_unregister);
-- 
2.14.1

Re: [PATCH 1/1] vmbus: hvsock: add proper sync for vmbus_hvsock_device_unregister()

[Dan Carpenter]

On Tue, Oct 10, 2017 at 10:38:51PM -0700, kys@exchange.microsoft.com wrote:
> From: Dexuan Cui <decui@microsoft.com>
> 
> Without the patch, vmbus_hvsock_device_unregister() can destroy the device
> prematurely when close() is called, and can cause NULl dereferencing or
> potential data loss (the last portion of the data stream may be dropped
> prematurely).
> 
> Please consider this for 4.14.

Put these meta comments under the --- cut off line.  They don't add
any value to the final upstream git log.

> 
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> Cc: Haiyang Zhang <haiyangz@microsoft.com>
> Cc: Stephen Hemminger <sthemmin@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  drivers/hv/channel_mgmt.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)


regards,
dan carpenter

RE: [PATCH 1/1] vmbus: hvsock: add proper sync for vmbus_hvsock_device_unregister()

[KY Srinivasan]

Thanks Dan. Will do.

K. Y

> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Wednesday, October 11, 2017 2:42 AM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> vkuznets@redhat.com; jasowang@redhat.com;
> leann.ogasawara@canonical.com; marcelo.cerri@canonical.com; Stephen
> Hemminger <sthemmin@microsoft.com>; Haiyang Zhang
> <haiyangz@microsoft.com>
> Subject: Re: [PATCH 1/1] vmbus: hvsock: add proper sync for
> vmbus_hvsock_device_unregister()
> 
> On Tue, Oct 10, 2017 at 10:38:51PM -0700, kys@exchange.microsoft.com
> wrote:
> > From: Dexuan Cui <decui@microsoft.com>
> >
> > Without the patch, vmbus_hvsock_device_unregister() can destroy the
> device
> > prematurely when close() is called, and can cause NULl dereferencing or
> > potential data loss (the last portion of the data stream may be dropped
> > prematurely).
> >
> > Please consider this for 4.14.
> 
> Put these meta comments under the --- cut off line.  They don't add
> any value to the final upstream git log.
> 
> >
> > Signed-off-by: Dexuan Cui <decui@microsoft.com>
> > Cc: Haiyang Zhang <haiyangz@microsoft.com>
> > Cc: Stephen Hemminger <sthemmin@microsoft.com>
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > ---
> >  drivers/hv/channel_mgmt.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> 
> regards,
> dan carpenter