From a70d298c44592b95ae962e698c434f2b04e78805 Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Tue, 25 Jan 2022 12:01:31 -0600 Subject: Drivers: hv: vmbus: Use struct_size() helper in kmalloc() Make use of the struct_size() helper instead of an open-coded version, in order to avoid any potential type mistakes or integer overflows that, in the worst scenario, could lead to heap overflows. Also, address the following sparse warnings: drivers/hv/vmbus_drv.c:1132:31: warning: using sizeof on a flexible structure Link: https://github.com/KSPP/linux/issues/174 Signed-off-by: Gustavo A. R. Silva Reviewed-by: Michael Kelley Link: https://lore.kernel.org/r/20220125180131.GA67746@embeddedor Signed-off-by: Wei Liu --- drivers/hv/vmbus_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 17bf55fe3169..cd193456cd84 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -1129,7 +1129,7 @@ void vmbus_on_msg_dpc(unsigned long data) } if (entry->handler_type == VMHT_BLOCKING) { - ctx = kmalloc(sizeof(*ctx) + payload_size, GFP_ATOMIC); + ctx = kmalloc(struct_size(ctx, msg.payload, payload_size), GFP_ATOMIC); if (ctx == NULL) return; -- cgit v1.2.3 From de96e8a09889b35dd8d1cb6d19ef2bb123b05be1 Mon Sep 17 00:00:00 2001 From: Vitaly Kuznetsov Date: Fri, 28 Jan 2022 11:34:11 +0100 Subject: Drivers: hv: Rename 'alloced' to 'allocated' 'Alloced' is not a real word and only saves us two letters, let's use 'allocated' instead. No functional change intended. Signed-off-by: Vitaly Kuznetsov Reviewed-by: Michael Kelley Link: https://lore.kernel.org/r/20220128103412.3033736-2-vkuznets@redhat.com Signed-off-by: Wei Liu --- drivers/hv/channel_mgmt.c | 18 +++++++++--------- drivers/hv/hyperv_vmbus.h | 14 +++++++------- drivers/hv/vmbus_drv.c | 2 +- 3 files changed, 17 insertions(+), 17 deletions(-) (limited to 'drivers') diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c index 60375879612f..52cf6ae525e9 100644 --- a/drivers/hv/channel_mgmt.c +++ b/drivers/hv/channel_mgmt.c @@ -459,7 +459,7 @@ void hv_process_channel_removal(struct vmbus_channel *channel) * init_vp_index() can (re-)use the CPU. */ if (hv_is_perf_channel(channel)) - hv_clear_alloced_cpu(channel->target_cpu); + hv_clear_allocated_cpu(channel->target_cpu); /* * Upon suspend, an in-use hv_sock channel is marked as "rescinded" and @@ -728,7 +728,7 @@ static void init_vp_index(struct vmbus_channel *channel) bool perf_chn = hv_is_perf_channel(channel); u32 i, ncpu = num_online_cpus(); cpumask_var_t available_mask; - struct cpumask *alloced_mask; + struct cpumask *allocated_mask; u32 target_cpu; int numa_node; @@ -745,7 +745,7 @@ static void init_vp_index(struct vmbus_channel *channel) */ channel->target_cpu = VMBUS_CONNECT_CPU; if (perf_chn) - hv_set_alloced_cpu(VMBUS_CONNECT_CPU); + hv_set_allocated_cpu(VMBUS_CONNECT_CPU); return; } @@ -760,22 +760,22 @@ static void init_vp_index(struct vmbus_channel *channel) continue; break; } - alloced_mask = &hv_context.hv_numa_map[numa_node]; + allocated_mask = &hv_context.hv_numa_map[numa_node]; - if (cpumask_weight(alloced_mask) == + if (cpumask_weight(allocated_mask) == cpumask_weight(cpumask_of_node(numa_node))) { /* * We have cycled through all the CPUs in the node; - * reset the alloced map. + * reset the allocated map. */ - cpumask_clear(alloced_mask); + cpumask_clear(allocated_mask); } - cpumask_xor(available_mask, alloced_mask, + cpumask_xor(available_mask, allocated_mask, cpumask_of_node(numa_node)); target_cpu = cpumask_first(available_mask); - cpumask_set_cpu(target_cpu, alloced_mask); + cpumask_set_cpu(target_cpu, allocated_mask); if (channel->offermsg.offer.sub_channel_index >= ncpu || i > ncpu || !hv_cpuself_used(target_cpu, channel)) diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h index 3a1f007b678a..6b45c22bb717 100644 --- a/drivers/hv/hyperv_vmbus.h +++ b/drivers/hv/hyperv_vmbus.h @@ -405,7 +405,7 @@ static inline bool hv_is_perf_channel(struct vmbus_channel *channel) return vmbus_devs[channel->device_id].perf_device; } -static inline bool hv_is_alloced_cpu(unsigned int cpu) +static inline bool hv_is_allocated_cpu(unsigned int cpu) { struct vmbus_channel *channel, *sc; @@ -427,23 +427,23 @@ static inline bool hv_is_alloced_cpu(unsigned int cpu) return false; } -static inline void hv_set_alloced_cpu(unsigned int cpu) +static inline void hv_set_allocated_cpu(unsigned int cpu) { cpumask_set_cpu(cpu, &hv_context.hv_numa_map[cpu_to_node(cpu)]); } -static inline void hv_clear_alloced_cpu(unsigned int cpu) +static inline void hv_clear_allocated_cpu(unsigned int cpu) { - if (hv_is_alloced_cpu(cpu)) + if (hv_is_allocated_cpu(cpu)) return; cpumask_clear_cpu(cpu, &hv_context.hv_numa_map[cpu_to_node(cpu)]); } -static inline void hv_update_alloced_cpus(unsigned int old_cpu, +static inline void hv_update_allocated_cpus(unsigned int old_cpu, unsigned int new_cpu) { - hv_set_alloced_cpu(new_cpu); - hv_clear_alloced_cpu(old_cpu); + hv_set_allocated_cpu(new_cpu); + hv_clear_allocated_cpu(old_cpu); } #ifdef CONFIG_HYPERV_TESTING diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index cd193456cd84..6f1da72b7516 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -1874,7 +1874,7 @@ static ssize_t target_cpu_store(struct vmbus_channel *channel, /* See init_vp_index(). */ if (hv_is_perf_channel(channel)) - hv_update_alloced_cpus(origin_cpu, target_cpu); + hv_update_allocated_cpus(origin_cpu, target_cpu); /* Currently set only for storvsc channels. */ if (channel->change_target_cpu_callback) { -- cgit v1.2.3 From 4ee524587105011ef43e5bd3ef5ed019715363dd Mon Sep 17 00:00:00 2001 From: Vitaly Kuznetsov Date: Fri, 28 Jan 2022 11:34:12 +0100 Subject: Drivers: hv: Compare cpumasks and not their weights in init_vp_index() The condition is supposed to check whether 'allocated_mask' got fully exhausted, i.e. there's no free CPU on the NUMA node left so we have to use one of the already used CPUs. As only bits which correspond to CPUs from 'cpumask_of_node(numa_node)' get set in 'allocated_mask', checking for the equal weights is technically correct but not obvious. Let's compare cpumasks directly. No functional change intended. Suggested-by: Michael Kelley Signed-off-by: Vitaly Kuznetsov Reviewed-by: Michael Kelley Link: https://lore.kernel.org/r/20220128103412.3033736-3-vkuznets@redhat.com Signed-off-by: Wei Liu --- drivers/hv/channel_mgmt.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c index 52cf6ae525e9..26d269ba947c 100644 --- a/drivers/hv/channel_mgmt.c +++ b/drivers/hv/channel_mgmt.c @@ -762,8 +762,7 @@ static void init_vp_index(struct vmbus_channel *channel) } allocated_mask = &hv_context.hv_numa_map[numa_node]; - if (cpumask_weight(allocated_mask) == - cpumask_weight(cpumask_of_node(numa_node))) { + if (cpumask_equal(allocated_mask, cpumask_of_node(numa_node))) { /* * We have cycled through all the CPUs in the node; * reset the allocated map. -- cgit v1.2.3 From 6de74d1069b821e96460d0fc2edfc35785db04fb Mon Sep 17 00:00:00 2001 From: Michael Kelley Date: Wed, 9 Feb 2022 08:11:10 -0800 Subject: hv_utils: Add comment about max VMbus packet size in VSS driver The VSS driver allocates a VMbus receive buffer significantly larger than sizeof(hv_vss_msg), with no explanation. To help prevent future mistakes, add a #define and comment about why this is done. No functional change. Signed-off-by: Michael Kelley Link: https://lore.kernel.org/r/1644423070-75125-1-git-send-email-mikelley@microsoft.com Signed-off-by: Wei Liu --- drivers/hv/hv_snapshot.c | 7 +++++-- include/uapi/linux/hyperv.h | 11 +++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c index 6018b9d1b1fb..0d2184be1691 100644 --- a/drivers/hv/hv_snapshot.c +++ b/drivers/hv/hv_snapshot.c @@ -31,6 +31,9 @@ static const int fw_versions[] = { UTIL_FW_VERSION }; +/* See comment with struct hv_vss_msg regarding the max VMbus packet size */ +#define VSS_MAX_PKT_SIZE (HV_HYP_PAGE_SIZE * 2) + /* * Timeout values are based on expecations from host */ @@ -298,7 +301,7 @@ void hv_vss_onchannelcallback(void *context) if (vss_transaction.state > HVUTIL_READY) return; - if (vmbus_recvpacket(channel, recv_buffer, HV_HYP_PAGE_SIZE * 2, &recvlen, &requestid)) { + if (vmbus_recvpacket(channel, recv_buffer, VSS_MAX_PKT_SIZE, &recvlen, &requestid)) { pr_err_ratelimited("VSS request received. Could not read into recv buf\n"); return; } @@ -375,7 +378,7 @@ hv_vss_init(struct hv_util_service *srv) } recv_buffer = srv->recv_buffer; vss_transaction.recv_channel = srv->channel; - vss_transaction.recv_channel->max_pkt_size = HV_HYP_PAGE_SIZE * 2; + vss_transaction.recv_channel->max_pkt_size = VSS_MAX_PKT_SIZE; /* * When this driver loads, the user level daemon that diff --git a/include/uapi/linux/hyperv.h b/include/uapi/linux/hyperv.h index daf82a230c0e..aaa502a7bff4 100644 --- a/include/uapi/linux/hyperv.h +++ b/include/uapi/linux/hyperv.h @@ -90,6 +90,17 @@ struct hv_vss_check_dm_info { __u32 flags; } __attribute__((packed)); +/* + * struct hv_vss_msg encodes the fields that the Linux VSS + * driver accesses. However, FREEZE messages from Hyper-V contain + * additional LUN information that Linux doesn't use and are not + * represented in struct hv_vss_msg. A received FREEZE message may + * be as large as 6,260 bytes, so the driver must allocate at least + * that much space, not sizeof(struct hv_vss_msg). Other messages + * such as AUTO_RECOVER may be as large as 12,500 bytes. However, + * because the Linux VSS driver responds that it doesn't support + * auto-recovery, it should not receive such messages. + */ struct hv_vss_msg { union { struct hv_vss_hdr vss_hdr; -- cgit v1.2.3 From d57d6fe5bf3463aad1332ecfc7555a6888b04fd9 Mon Sep 17 00:00:00 2001 From: Stephen Brennan Date: Mon, 14 Feb 2022 17:37:35 -0800 Subject: drivers: hv: log when enabling crash_kexec_post_notifiers Recently I went down a rabbit hole looking at a race condition in panic() on a Hyper-V guest. I assumed, since it was missing from the command line, that crash_kexec_post_notifiers was disabled. Only after a rather long reproduction and analysis process did I learn that Hyper-V actually enables this setting unconditionally. Users and debuggers alike would like to know when these things happen. I think it would be good to print a message to the kernel log when this happens, so that a grep for "crash_kexec_post_notifiers" shows relevant results. Signed-off-by: Stephen Brennan Reviewed-by: Michael Kelley Link: https://lore.kernel.org/r/20220215013735.358327-1-stephen.s.brennan@oracle.com Signed-off-by: Wei Liu --- drivers/hv/hv_common.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c index 181d16bbf49d..c1dd21d0d7ef 100644 --- a/drivers/hv/hv_common.c +++ b/drivers/hv/hv_common.c @@ -79,8 +79,10 @@ int __init hv_common_init(void) * calling crash enlightment interface before running kdump * kernel. */ - if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) + if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) { crash_kexec_post_notifiers = true; + pr_info("Hyper-V: enabling crash_kexec_post_notifiers\n"); + } /* * Allocate the per-CPU state for the hypercall input arg. -- cgit v1.2.3 From 1d7286729aa616772be334eb908e11f527e1e291 Mon Sep 17 00:00:00 2001 From: Anssi Hannula Date: Tue, 22 Feb 2022 16:14:00 +0200 Subject: hv_balloon: rate-limit "Unhandled message" warning For a couple of times I have encountered a situation where hv_balloon: Unhandled message: type: 12447 is being flooded over 1 million times per second with various values, filling the log and consuming cycles, making debugging difficult. Add rate limiting to the message. Most other Hyper-V drivers already have similar rate limiting in their message callbacks. The cause of the floods in my case was probably fixed by 96d9d1fa5cd5 ("Drivers: hv: balloon: account for vmbus packet header in max_pkt_size"). Fixes: 9aa8b50b2b3d ("Drivers: hv: Add Hyper-V balloon driver") Signed-off-by: Anssi Hannula Reviewed-by: Michael Kelley Link: https://lore.kernel.org/r/20220222141400.98160-1-anssi.hannula@bitwise.fi Signed-off-by: Wei Liu --- drivers/hv/hv_balloon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c index f2d05bff4245..439f99b8b5de 100644 --- a/drivers/hv/hv_balloon.c +++ b/drivers/hv/hv_balloon.c @@ -1563,7 +1563,7 @@ static void balloon_onchannelcallback(void *context) break; default: - pr_warn("Unhandled message: type: %d\n", dm_hdr->type); + pr_warn_ratelimited("Unhandled message: type: %d\n", dm_hdr->type); } } -- cgit v1.2.3