From b9d8e35256251e6e6a27f5f190d4ec462be7f620 Mon Sep 17 00:00:00 2001 From: "K. Y. Srinivasan" Date: Mon, 16 May 2011 06:44:36 -0700 Subject: [PATCH] Staging: hv: Re-implement the synchronization for util channels The util module expects that the util channels are fully initialized when the module loads. To deal with the race condition which can result in a NULL pointer dereferencing if the util module were to load before all the util channels are fully initialized, in commit: commit: 8b5d6d3bd3e34e4cc67d875c8c88007c1c9aa96 Author: Haiyang Zhang Date: Fri May 28 23:22:44 2010 +000 code was introduced in the vmbus driver to ensure that all the util channels were fully initialized before returning from the load of the vmbus driver. This solution has several problems: if for whatever reason, any util channel were to fail to initialize, vmbus driver would wait indefinitely. We deal with this synchronization issue very differently in this patch. Signed-off-by: K. Y. Srinivasan Signed-off-by: Haiyang Zhang Signed-off-by: Abhishek Kane Signed-off-by: Hank Janssen Signed-off-by: Greg Kroah-Hartman --- drivers/staging/hv/channel_mgmt.c | 24 +++++++++++++++---- drivers/staging/hv/hv_util.c | 39 ++++++++++++++----------------- 2 files changed, 37 insertions(+), 26 deletions(-) diff --git a/drivers/staging/hv/channel_mgmt.c b/drivers/staging/hv/channel_mgmt.c index 1021713ea4c0..b7343bc72b40 100644 --- a/drivers/staging/hv/channel_mgmt.c +++ b/drivers/staging/hv/channel_mgmt.c @@ -181,6 +181,24 @@ void chn_cb_negotiate(void *context) struct icmsg_hdr *icmsghdrp; struct icmsg_negotiate *negop = NULL; + if (channel->util_index >= 0) { + /* + * This is a properly initialized util channel. + * Route this callback appropriately and setup state + * so that we don't need to reroute again. + */ + if (hv_cb_utils[channel->util_index].callback != NULL) { + /* + * The util driver has established a handler for + * this service; do the magic. + */ + channel->onchannel_callback = + hv_cb_utils[channel->util_index].callback; + (hv_cb_utils[channel->util_index].callback)(channel); + return; + } + } + buflen = PAGE_SIZE; buf = kmalloc(buflen, GFP_ATOMIC); @@ -217,7 +235,6 @@ struct hyperv_service_callback hv_cb_utils[MAX_MSG_TYPES] = { 0x31, 0x60, 0x0B, 0X0E, 0x13, 0x52, 0x34, 0x49, 0x81, 0x8B, 0x38, 0XD9, 0x0C, 0xED, 0x39, 0xDB }, - .callback = chn_cb_negotiate, .log_msg = "Shutdown channel functionality initialized" }, @@ -229,7 +246,6 @@ struct hyperv_service_callback hv_cb_utils[MAX_MSG_TYPES] = { 0x30, 0xe6, 0x27, 0x95, 0xae, 0xd0, 0x7b, 0x49, 0xad, 0xce, 0xe8, 0x0a, 0xb0, 0x17, 0x5c, 0xaf }, - .callback = chn_cb_negotiate, .log_msg = "Timesync channel functionality initialized" }, /* {57164f39-9115-4e78-ab55-382f3bd5422d} */ @@ -240,7 +256,6 @@ struct hyperv_service_callback hv_cb_utils[MAX_MSG_TYPES] = { 0x39, 0x4f, 0x16, 0x57, 0x15, 0x91, 0x78, 0x4e, 0xab, 0x55, 0x38, 0x2f, 0x3b, 0xd5, 0x42, 0x2d }, - .callback = chn_cb_negotiate, .log_msg = "Heartbeat channel functionality initialized" }, /* {A9A0F4E7-5A45-4d96-B827-8A841E8C03E6} */ @@ -250,7 +265,6 @@ struct hyperv_service_callback hv_cb_utils[MAX_MSG_TYPES] = { 0xe7, 0xf4, 0xa0, 0xa9, 0x45, 0x5a, 0x96, 0x4d, 0xb8, 0x27, 0x8a, 0x84, 0x1e, 0x8c, 0x3, 0xe6 }, - .callback = chn_cb_negotiate, .log_msg = "KVP channel functionality initialized" }, }; @@ -428,7 +442,7 @@ static void vmbus_process_offer(struct work_struct *work) sizeof(struct hv_guid)) == 0 && vmbus_open(newchannel, 2 * PAGE_SIZE, 2 * PAGE_SIZE, NULL, 0, - hv_cb_utils[cnt].callback, + chn_cb_negotiate, newchannel) == 0) { hv_cb_utils[cnt].channel = newchannel; newchannel->util_index = cnt; diff --git a/drivers/staging/hv/hv_util.c b/drivers/staging/hv/hv_util.c index 5605374c3464..c164b54b4cd7 100644 --- a/drivers/staging/hv/hv_util.c +++ b/drivers/staging/hv/hv_util.c @@ -256,22 +256,13 @@ static int __init init_hyperv_utils(void) return -ENOMEM; } - hv_cb_utils[HV_SHUTDOWN_MSG].channel->onchannel_callback = - &shutdown_onchannelcallback; hv_cb_utils[HV_SHUTDOWN_MSG].callback = &shutdown_onchannelcallback; - hv_cb_utils[HV_TIMESYNC_MSG].channel->onchannel_callback = - ×ync_onchannelcallback; hv_cb_utils[HV_TIMESYNC_MSG].callback = ×ync_onchannelcallback; - hv_cb_utils[HV_HEARTBEAT_MSG].channel->onchannel_callback = - &heartbeat_onchannelcallback; hv_cb_utils[HV_HEARTBEAT_MSG].callback = &heartbeat_onchannelcallback; - hv_cb_utils[HV_KVP_MSG].channel->onchannel_callback = - &hv_kvp_onchannelcallback; - - + hv_cb_utils[HV_KVP_MSG].callback = &hv_kvp_onchannelcallback; return 0; } @@ -280,20 +271,26 @@ static void exit_hyperv_utils(void) { pr_info("De-Registered HyperV Utility Driver\n"); - hv_cb_utils[HV_SHUTDOWN_MSG].channel->onchannel_callback = - &chn_cb_negotiate; - hv_cb_utils[HV_SHUTDOWN_MSG].callback = &chn_cb_negotiate; + if (hv_cb_utils[HV_SHUTDOWN_MSG].channel != NULL) + hv_cb_utils[HV_SHUTDOWN_MSG].channel->onchannel_callback = + &chn_cb_negotiate; + hv_cb_utils[HV_SHUTDOWN_MSG].callback = NULL; + + if (hv_cb_utils[HV_TIMESYNC_MSG].channel != NULL) + hv_cb_utils[HV_TIMESYNC_MSG].channel->onchannel_callback = + &chn_cb_negotiate; + hv_cb_utils[HV_TIMESYNC_MSG].callback = NULL; - hv_cb_utils[HV_TIMESYNC_MSG].channel->onchannel_callback = - &chn_cb_negotiate; - hv_cb_utils[HV_TIMESYNC_MSG].callback = &chn_cb_negotiate; + if (hv_cb_utils[HV_HEARTBEAT_MSG].channel != NULL) + hv_cb_utils[HV_HEARTBEAT_MSG].channel->onchannel_callback = + &chn_cb_negotiate; + hv_cb_utils[HV_HEARTBEAT_MSG].callback = NULL; - hv_cb_utils[HV_HEARTBEAT_MSG].channel->onchannel_callback = - &chn_cb_negotiate; - hv_cb_utils[HV_HEARTBEAT_MSG].callback = &chn_cb_negotiate; + if (hv_cb_utils[HV_KVP_MSG].channel != NULL) + hv_cb_utils[HV_KVP_MSG].channel->onchannel_callback = + &chn_cb_negotiate; + hv_cb_utils[HV_KVP_MSG].callback = NULL; - hv_cb_utils[HV_KVP_MSG].channel->onchannel_callback = - &chn_cb_negotiate; hv_kvp_deinit(); kfree(shut_txf_buf); -- 2.39.5