From: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx> Subject: staging: hv: Fix race condition on IC channel initialization There is a possible race condition when hv_utils starts to load immediately after hv_vmbus is loading - null pointer error could happen. This patch added an atomic counter to ensure all channels are ready before vmbus_init() returns. So another module won't have any uninitialized channel. Signed-off-by: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx> Signed-off-by: Hank Janssen <hjanssen@xxxxxxxxxxxxx> --- drivers/staging/hv/channel_mgmt.c | 25 ++++++++++++++----------- drivers/staging/hv/hv_utils.c | 7 ++++--- drivers/staging/hv/utils.h | 5 +++++ drivers/staging/hv/vmbus_drv.c | 5 +++++ 4 files changed, 28 insertions(+), 14 deletions(-) diff --git a/drivers/staging/hv/channel_mgmt.c b/drivers/staging/hv/channel_mgmt.c index 3f53b4d..68dfa0f 100644 --- a/drivers/staging/hv/channel_mgmt.c +++ b/drivers/staging/hv/channel_mgmt.c @@ -33,7 +33,6 @@ struct vmbus_channel_message_table_entry { void (*messageHandler)(struct vmbus_channel_message_header *msg); }; -#define MAX_MSG_TYPES 3 #define MAX_NUM_DEVICE_CLASSES_SUPPORTED 7 static const struct hv_guid @@ -233,6 +232,10 @@ struct hyperv_service_callback hv_cb_utils[MAX_MSG_TYPES] = { }; EXPORT_SYMBOL(hv_cb_utils); +/* Counter of IC channels initialized */ +atomic_t hv_utils_initcnt = ATOMIC_INIT(0); + + /* * AllocVmbusChannel - Allocate and initialize a vmbus channel object */ @@ -373,22 +376,22 @@ static void VmbusChannelProcessOffer(void *context) * can cleanup properly */ newChannel->State = CHANNEL_OPEN_STATE; - cnt = 0; - while (cnt != MAX_MSG_TYPES) { + /* Open IC channels */ + for (cnt = 0; cnt < MAX_MSG_TYPES; cnt++) { if (memcmp(&newChannel->OfferMsg.Offer.InterfaceType, &hv_cb_utils[cnt].data, - sizeof(struct hv_guid)) == 0) { + sizeof(struct hv_guid)) == 0 && + VmbusChannelOpen(newChannel, 2 * PAGE_SIZE, + 2 * PAGE_SIZE, NULL, 0, + hv_cb_utils[cnt].callback, + newChannel) == 0) { + hv_cb_utils[cnt].channel = newChannel; + mb(); DPRINT_INFO(VMBUS, "%s", hv_cb_utils[cnt].log_msg); - - if (VmbusChannelOpen(newChannel, 2 * PAGE_SIZE, - 2 * PAGE_SIZE, NULL, 0, - hv_cb_utils[cnt].callback, - newChannel) == 0) - hv_cb_utils[cnt].channel = newChannel; + atomic_inc(&hv_utils_initcnt); } - cnt++; } } DPRINT_EXIT(VMBUS); diff --git a/drivers/staging/hv/hv_utils.c b/drivers/staging/hv/hv_utils.c index 8a49aaf..22f6f4a 100644 --- a/drivers/staging/hv/hv_utils.c +++ b/drivers/staging/hv/hv_utils.c @@ -253,7 +253,7 @@ static void heartbeat_onchannelcallback(void *context) static int __init init_hyperv_utils(void) { - printk(KERN_INFO "Registering HyperV Utility Driver\n"); + printk(KERN_INFO "Registering HyperV Utility Driver...\n"); hv_cb_utils[HV_SHUTDOWN_MSG].channel->OnChannelCallback = &shutdown_onchannelcallback; @@ -267,13 +267,12 @@ static int __init init_hyperv_utils(void) &heartbeat_onchannelcallback; hv_cb_utils[HV_HEARTBEAT_MSG].callback = &heartbeat_onchannelcallback; + printk(KERN_INFO "Registered HyperV Utility Driver.\n"); return 0; } static void exit_hyperv_utils(void) { - printk(KERN_INFO "De-Registered HyperV Utility Driver\n"); - hv_cb_utils[HV_SHUTDOWN_MSG].channel->OnChannelCallback = &chn_cb_negotiate; hv_cb_utils[HV_SHUTDOWN_MSG].callback = &chn_cb_negotiate; @@ -285,6 +284,8 @@ static void exit_hyperv_utils(void) hv_cb_utils[HV_HEARTBEAT_MSG].channel->OnChannelCallback = &chn_cb_negotiate; hv_cb_utils[HV_HEARTBEAT_MSG].callback = &chn_cb_negotiate; + + printk(KERN_INFO "De-Registered HyperV Utility Driver.\n"); } module_init(init_hyperv_utils); diff --git a/drivers/staging/hv/utils.h b/drivers/staging/hv/utils.h index 7c07499..3291ab4 100644 --- a/drivers/staging/hv/utils.h +++ b/drivers/staging/hv/utils.h @@ -98,6 +98,10 @@ struct ictimesync_data{ u8 flags; } __attribute__((packed)); + +/* Number of IC types supported */ +#define MAX_MSG_TYPES 3 + /* Index for each IC struct in array hv_cb_utils[] */ #define HV_SHUTDOWN_MSG 0 #define HV_TIMESYNC_MSG 1 @@ -115,5 +119,6 @@ extern void prep_negotiate_resp(struct icmsg_hdr *, struct icmsg_negotiate *, u8 *); extern void chn_cb_negotiate(void *); extern struct hyperv_service_callback hv_cb_utils[]; +extern atomic_t hv_utils_initcnt; #endif /* __HV_UTILS_H_ */ diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c index c21731a..160ee92 100644 --- a/drivers/staging/hv/vmbus_drv.c +++ b/drivers/staging/hv/vmbus_drv.c @@ -31,6 +31,7 @@ #include "osd.h" #include "logging.h" #include "vmbus.h" +#include "utils.h" /* FIXME! We need to do this dynamically for PIC and APIC system */ @@ -1005,6 +1006,10 @@ static int __init vmbus_init(void) ret = vmbus_bus_init(VmbusInitialize); + /* Wait until all IC channels are initialized */ + while (atomic_read(&hv_utils_initcnt) < MAX_MSG_TYPES) + msleep(100); + DPRINT_EXIT(VMBUS_DRV); return ret; } -- 1.6.3.2
Attachment:
0521-Fix-race-condition-on-IC-channel-initialization.patch
Description: 0521-Fix-race-condition-on-IC-channel-initialization.patch
_______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel