* [PATCH 0/3] Drivers: hv: vmbus: Some miscellaneous fixes
@ 2016-06-04 0:09 K. Y. Srinivasan
2016-06-04 0:09 ` [PATCH 1/3] Drivers: hv: avoid vfree() on crash K. Y. Srinivasan
2016-07-29 14:25 ` [PATCH 0/3] Drivers: hv: vmbus: Some miscellaneous fixes Vitaly Kuznetsov
0 siblings, 2 replies; 6+ messages in thread
From: K. Y. Srinivasan @ 2016-06-04 0:09 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
Cc: K. Y. Srinivasan
Some miscellaneous fixes.
Vitaly Kuznetsov (3):
Drivers: hv: avoid vfree() on crash
Drivers: hv: get rid of redundant messagecount in
create_gpadl_header()
Drivers: hv: don't leak memory in vmbus_establish_gpadl()
drivers/hv/channel.c | 44 +++++++++++++++++++++-----------------------
drivers/hv/hv.c | 8 +++++---
drivers/hv/hyperv_vmbus.h | 2 +-
drivers/hv/vmbus_drv.c | 8 ++++----
4 files changed, 31 insertions(+), 31 deletions(-)
--
1.7.4.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] Drivers: hv: avoid vfree() on crash
2016-06-04 0:09 [PATCH 0/3] Drivers: hv: vmbus: Some miscellaneous fixes K. Y. Srinivasan
@ 2016-06-04 0:09 ` K. Y. Srinivasan
2016-06-04 0:09 ` [PATCH 2/3] Drivers: hv: get rid of redundant messagecount in create_gpadl_header() K. Y. Srinivasan
2016-06-04 0:09 ` [PATCH 3/3] Drivers: hv: don't leak memory in vmbus_establish_gpadl() K. Y. Srinivasan
2016-07-29 14:25 ` [PATCH 0/3] Drivers: hv: vmbus: Some miscellaneous fixes Vitaly Kuznetsov
1 sibling, 2 replies; 6+ messages in thread
From: K. Y. Srinivasan @ 2016-06-04 0:09 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
Cc: K. Y. Srinivasan
From: Vitaly Kuznetsov <vkuznets@redhat.com>
When we crash from NMI context (e.g. after NMI injection from host when
'sysctl -w kernel.unknown_nmi_panic=1' is set) we hit
kernel BUG at mm/vmalloc.c:1530!
as vfree() is denied. While the issue could be solved with in_nmi() check
instead I opted for skipping vfree on all sorts of crashes to reduce the
amount of work which can cause consequent crashes. We don't really need to
free anything on crash.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/hv.c | 8 +++++---
drivers/hv/hyperv_vmbus.h | 2 +-
drivers/hv/vmbus_drv.c | 8 ++++----
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index a1c086b..60dbd6c 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -278,7 +278,7 @@ cleanup:
*
* This routine is called normally during driver unloading or exiting.
*/
-void hv_cleanup(void)
+void hv_cleanup(bool crash)
{
union hv_x64_msr_hypercall_contents hypercall_msr;
@@ -288,7 +288,8 @@ void hv_cleanup(void)
if (hv_context.hypercall_page) {
hypercall_msr.as_uint64 = 0;
wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
- vfree(hv_context.hypercall_page);
+ if (!crash)
+ vfree(hv_context.hypercall_page);
hv_context.hypercall_page = NULL;
}
@@ -308,7 +309,8 @@ void hv_cleanup(void)
hypercall_msr.as_uint64 = 0;
wrmsrl(HV_X64_MSR_REFERENCE_TSC, hypercall_msr.as_uint64);
- vfree(hv_context.tsc_page);
+ if (!crash)
+ vfree(hv_context.tsc_page);
hv_context.tsc_page = NULL;
}
#endif
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 718b5c7..dfa9fac 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -495,7 +495,7 @@ struct hv_ring_buffer_debug_info {
extern int hv_init(void);
-extern void hv_cleanup(void);
+extern void hv_cleanup(bool crash);
extern int hv_post_message(union hv_connection_id connection_id,
enum hv_message_type message_type,
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 952f20f..d11690e 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -871,7 +871,7 @@ err_alloc:
bus_unregister(&hv_bus);
err_cleanup:
- hv_cleanup();
+ hv_cleanup(false);
return ret;
}
@@ -1323,7 +1323,7 @@ static void hv_kexec_handler(void)
vmbus_initiate_unload(false);
for_each_online_cpu(cpu)
smp_call_function_single(cpu, hv_synic_cleanup, NULL, 1);
- hv_cleanup();
+ hv_cleanup(false);
};
static void hv_crash_handler(struct pt_regs *regs)
@@ -1335,7 +1335,7 @@ static void hv_crash_handler(struct pt_regs *regs)
* for kdump.
*/
hv_synic_cleanup(NULL);
- hv_cleanup();
+ hv_cleanup(true);
};
static int __init hv_acpi_init(void)
@@ -1395,7 +1395,7 @@ static void __exit vmbus_exit(void)
&hyperv_panic_block);
}
bus_unregister(&hv_bus);
- hv_cleanup();
+ hv_cleanup(false);
for_each_online_cpu(cpu) {
tasklet_kill(hv_context.event_dpc[cpu]);
smp_call_function_single(cpu, hv_synic_cleanup, NULL, 1);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] Drivers: hv: get rid of redundant messagecount in create_gpadl_header()
2016-06-04 0:09 ` [PATCH 1/3] Drivers: hv: avoid vfree() on crash K. Y. Srinivasan
@ 2016-06-04 0:09 ` K. Y. Srinivasan
2016-06-04 0:09 ` [PATCH 3/3] Drivers: hv: don't leak memory in vmbus_establish_gpadl() K. Y. Srinivasan
1 sibling, 0 replies; 6+ messages in thread
From: K. Y. Srinivasan @ 2016-06-04 0:09 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
Cc: K. Y. Srinivasan
From: Vitaly Kuznetsov <vkuznets@redhat.com>
We use messagecount only once in vmbus_establish_gpadl() to check if
it is safe to iterate through the submsglist. We can just initialize
the list header in all cases in create_gpadl_header() instead.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/channel.c | 38 ++++++++++++++++----------------------
1 files changed, 16 insertions(+), 22 deletions(-)
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 56dd261..2b109e8 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -238,8 +238,7 @@ EXPORT_SYMBOL_GPL(vmbus_send_tl_connect_request);
* create_gpadl_header - Creates a gpadl for the specified buffer
*/
static int create_gpadl_header(void *kbuffer, u32 size,
- struct vmbus_channel_msginfo **msginfo,
- u32 *messagecount)
+ struct vmbus_channel_msginfo **msginfo)
{
int i;
int pagecount;
@@ -283,7 +282,6 @@ static int create_gpadl_header(void *kbuffer, u32 size,
gpadl_header->range[0].pfn_array[i] = slow_virt_to_phys(
kbuffer + PAGE_SIZE * i) >> PAGE_SHIFT;
*msginfo = msgheader;
- *messagecount = 1;
pfnsum = pfncount;
pfnleft = pagecount - pfncount;
@@ -323,7 +321,6 @@ static int create_gpadl_header(void *kbuffer, u32 size,
}
msgbody->msgsize = msgsize;
- (*messagecount)++;
gpadl_body =
(struct vmbus_channel_gpadl_body *)msgbody->msg;
@@ -352,6 +349,8 @@ static int create_gpadl_header(void *kbuffer, u32 size,
msgheader = kzalloc(msgsize, GFP_KERNEL);
if (msgheader == NULL)
goto nomem;
+
+ INIT_LIST_HEAD(&msgheader->submsglist);
msgheader->msgsize = msgsize;
gpadl_header = (struct vmbus_channel_gpadl_header *)
@@ -366,7 +365,6 @@ static int create_gpadl_header(void *kbuffer, u32 size,
kbuffer + PAGE_SIZE * i) >> PAGE_SHIFT;
*msginfo = msgheader;
- *messagecount = 1;
}
return 0;
@@ -391,7 +389,6 @@ int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
struct vmbus_channel_gpadl_body *gpadl_body;
struct vmbus_channel_msginfo *msginfo = NULL;
struct vmbus_channel_msginfo *submsginfo;
- u32 msgcount;
struct list_head *curr;
u32 next_gpadl_handle;
unsigned long flags;
@@ -400,7 +397,7 @@ int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
next_gpadl_handle =
(atomic_inc_return(&vmbus_connection.next_gpadl_handle) - 1);
- ret = create_gpadl_header(kbuffer, size, &msginfo, &msgcount);
+ ret = create_gpadl_header(kbuffer, size, &msginfo);
if (ret)
return ret;
@@ -423,24 +420,21 @@ int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
if (ret != 0)
goto cleanup;
- if (msgcount > 1) {
- list_for_each(curr, &msginfo->submsglist) {
+ list_for_each(curr, &msginfo->submsglist) {
+ submsginfo = (struct vmbus_channel_msginfo *)curr;
+ gpadl_body =
+ (struct vmbus_channel_gpadl_body *)submsginfo->msg;
- submsginfo = (struct vmbus_channel_msginfo *)curr;
- gpadl_body =
- (struct vmbus_channel_gpadl_body *)submsginfo->msg;
+ gpadl_body->header.msgtype =
+ CHANNELMSG_GPADL_BODY;
+ gpadl_body->gpadl = next_gpadl_handle;
- gpadl_body->header.msgtype =
- CHANNELMSG_GPADL_BODY;
- gpadl_body->gpadl = next_gpadl_handle;
+ ret = vmbus_post_msg(gpadl_body,
+ submsginfo->msgsize -
+ sizeof(*submsginfo));
+ if (ret != 0)
+ goto cleanup;
- ret = vmbus_post_msg(gpadl_body,
- submsginfo->msgsize -
- sizeof(*submsginfo));
- if (ret != 0)
- goto cleanup;
-
- }
}
wait_for_completion(&msginfo->waitevent);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] Drivers: hv: don't leak memory in vmbus_establish_gpadl()
2016-06-04 0:09 ` [PATCH 1/3] Drivers: hv: avoid vfree() on crash K. Y. Srinivasan
2016-06-04 0:09 ` [PATCH 2/3] Drivers: hv: get rid of redundant messagecount in create_gpadl_header() K. Y. Srinivasan
@ 2016-06-04 0:09 ` K. Y. Srinivasan
1 sibling, 0 replies; 6+ messages in thread
From: K. Y. Srinivasan @ 2016-06-04 0:09 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
Cc: K. Y. Srinivasan
From: Vitaly Kuznetsov <vkuznets@redhat.com>
In some cases create_gpadl_header() allocates submessages but we never
free them.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/channel.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 2b109e8..a68830c 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -388,7 +388,7 @@ int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
struct vmbus_channel_gpadl_header *gpadlmsg;
struct vmbus_channel_gpadl_body *gpadl_body;
struct vmbus_channel_msginfo *msginfo = NULL;
- struct vmbus_channel_msginfo *submsginfo;
+ struct vmbus_channel_msginfo *submsginfo, *tmp;
struct list_head *curr;
u32 next_gpadl_handle;
unsigned long flags;
@@ -445,6 +445,10 @@ cleanup:
spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags);
list_del(&msginfo->msglistentry);
spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
+ list_for_each_entry_safe(submsginfo, tmp, &msginfo->submsglist,
+ msglistentry) {
+ kfree(submsginfo);
+ }
kfree(msginfo);
return ret;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] Drivers: hv: vmbus: Some miscellaneous fixes
2016-06-04 0:09 [PATCH 0/3] Drivers: hv: vmbus: Some miscellaneous fixes K. Y. Srinivasan
2016-06-04 0:09 ` [PATCH 1/3] Drivers: hv: avoid vfree() on crash K. Y. Srinivasan
@ 2016-07-29 14:25 ` Vitaly Kuznetsov
2016-07-29 14:45 ` Greg KH
1 sibling, 1 reply; 6+ messages in thread
From: Vitaly Kuznetsov @ 2016-07-29 14:25 UTC (permalink / raw)
To: gregkh, K. Y. Srinivasan
Cc: linux-kernel, devel, olaf, apw, jasowang, leann.ogasawara
"K. Y. Srinivasan" <kys@microsoft.com> writes:
> Some miscellaneous fixes.
>
> Vitaly Kuznetsov (3):
> Drivers: hv: avoid vfree() on crash
> Drivers: hv: get rid of redundant messagecount in
> create_gpadl_header()
> Drivers: hv: don't leak memory in vmbus_establish_gpadl()
Greg, K. Y.,
I'm sorry for the ping bug this series was sent a while ago and I'm
starting to get worried it'll miss 4.8 as I don't see it in
char-misc-next.
There is a bunch of other series waiting to get merged, I'm aware of:
"[PATCH 0/3] Drivers: hv: vmbus: Miscellaneous adjustments"
"[PATCH 0/4] Drivers: hv: vmbus: Make in-place consumption always
possible"
"[PATCH 1/1] Drivers: hv: Introduce a policy for controlling channel
affinity"
"[PATCH 1/1] Tools: hv: kvp: ensure kvp device fd is closed on exec"
I'm sure there are more. Please take a look.
Thanks,
--
Vitaly
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] Drivers: hv: vmbus: Some miscellaneous fixes
2016-07-29 14:25 ` [PATCH 0/3] Drivers: hv: vmbus: Some miscellaneous fixes Vitaly Kuznetsov
@ 2016-07-29 14:45 ` Greg KH
0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2016-07-29 14:45 UTC (permalink / raw)
To: Vitaly Kuznetsov
Cc: K. Y. Srinivasan, olaf, jasowang, linux-kernel, apw, devel,
leann.ogasawara
On Fri, Jul 29, 2016 at 04:25:09PM +0200, Vitaly Kuznetsov wrote:
> "K. Y. Srinivasan" <kys@microsoft.com> writes:
>
> > Some miscellaneous fixes.
> >
> > Vitaly Kuznetsov (3):
> > Drivers: hv: avoid vfree() on crash
> > Drivers: hv: get rid of redundant messagecount in
> > create_gpadl_header()
> > Drivers: hv: don't leak memory in vmbus_establish_gpadl()
>
> Greg, K. Y.,
>
> I'm sorry for the ping bug this series was sent a while ago and I'm
> starting to get worried it'll miss 4.8 as I don't see it in
> char-misc-next.
>
> There is a bunch of other series waiting to get merged, I'm aware of:
>
> "[PATCH 0/3] Drivers: hv: vmbus: Miscellaneous adjustments"
> "[PATCH 0/4] Drivers: hv: vmbus: Make in-place consumption always
> possible"
> "[PATCH 1/1] Drivers: hv: Introduce a policy for controlling channel
> affinity"
> "[PATCH 1/1] Tools: hv: kvp: ensure kvp device fd is closed on exec"
>
> I'm sure there are more. Please take a look.
I see lots of hv patches in my to-apply queue, that will be processed
after 4.8-rc1 is out. They came in too late to make 4.8, sorry, due to
vacation and travel on my part.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-07-29 14:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-04 0:09 [PATCH 0/3] Drivers: hv: vmbus: Some miscellaneous fixes K. Y. Srinivasan
2016-06-04 0:09 ` [PATCH 1/3] Drivers: hv: avoid vfree() on crash K. Y. Srinivasan
2016-06-04 0:09 ` [PATCH 2/3] Drivers: hv: get rid of redundant messagecount in create_gpadl_header() K. Y. Srinivasan
2016-06-04 0:09 ` [PATCH 3/3] Drivers: hv: don't leak memory in vmbus_establish_gpadl() K. Y. Srinivasan
2016-07-29 14:25 ` [PATCH 0/3] Drivers: hv: vmbus: Some miscellaneous fixes Vitaly Kuznetsov
2016-07-29 14:45 ` Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).