* Re: [PATCH v3] net: mana: Fix possible double free in error handling path
From: patchwork-bot+netdevbpf @ 2024-06-27 10:40 UTC (permalink / raw)
To: Ma Ke
Cc: kys, haiyangz, wei.liu, decui, davem, edumazet, kuba, pabeni,
shradhagupta, horms, kotaranov, longli, schakrabarti,
erick.archer, leon, linux-hyperv, netdev, linux-kernel
In-Reply-To: <20240625130314.2661257-1-make24@iscas.ac.cn>
Hello:
This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Tue, 25 Jun 2024 21:03:14 +0800 you wrote:
> When auxiliary_device_add() returns error and then calls
> auxiliary_device_uninit(), callback function adev_release
> calls kfree(madev). We shouldn't call kfree(madev) again
> in the error handling path. Set 'madev' to NULL.
>
> Fixes: a69839d4327d ("net: mana: Add support for auxiliary device")
> Signed-off-by: Ma Ke <make24@iscas.ac.cn>
>
> [...]
Here is the summary with links:
- [v3] net: mana: Fix possible double free in error handling path
https://git.kernel.org/netdev/net/c/1864b8224195
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH] printk: Add a short description string to kmsg_dump()
From: Jocelyn Falempe @ 2024-06-27 7:29 UTC (permalink / raw)
To: Petr Mladek
Cc: Michael Ellerman, Nicholas Piggin, Christophe Leroy,
Naveen N. Rao, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Kees Cook, Tony Luck,
Guilherme G. Piccoli, Steven Rostedt, John Ogness,
Sergey Senozhatsky, Andrew Morton, Jani Nikula,
Greg Kroah-Hartman, Kefeng Wang, Thomas Gleixner, Uros Bizjak,
linuxppc-dev, linux-kernel, dri-devel, linux-hyperv, linux-mtd,
linux-hardening
In-Reply-To: <ZnvKcnC9ruaIHYij@pathway.suse.cz>
On 26/06/2024 10:00, Petr Mladek wrote:
> On Tue 2024-06-25 14:39:29, Jocelyn Falempe wrote:
>> kmsg_dump doesn't forward the panic reason string to the kmsg_dumper
>> callback.
>> This patch adds a new parameter "const char *desc" to the kmsg_dumper
>> dump() callback, and update all drivers that are using it.
>>
>> To avoid updating all kmsg_dump() call, it adds a kmsg_dump_desc()
>> function and a macro for backward compatibility.
>>
>> I've written this for drm_panic, but it can be useful for other
>> kmsg_dumper.
>> It allows to see the panic reason, like "sysrq triggered crash"
>> or "VFS: Unable to mount root fs on xxxx" on the drm panic screen.
>>
>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
>> ---
>> arch/powerpc/kernel/nvram_64.c | 3 ++-
>> arch/powerpc/platforms/powernv/opal-kmsg.c | 3 ++-
>> drivers/gpu/drm/drm_panic.c | 3 ++-
>> drivers/hv/hv_common.c | 3 ++-
>> drivers/mtd/mtdoops.c | 3 ++-
>> fs/pstore/platform.c | 3 ++-
>> include/linux/kmsg_dump.h | 13 ++++++++++---
>> kernel/panic.c | 2 +-
>> kernel/printk/printk.c | 8 +++++---
>> 9 files changed, 28 insertions(+), 13 deletions(-)
>
> The parameter is added into all dumpers. I guess that it would be
> used only drm_panic() because it is graphics and might be "fancy".
> The others simply dump the log buffer and the reason is in
> the dumped log as well.
Ok, I also tried to retrieve the reason from the dumped log, but that's
really fragile.
>
> Anyway, the passed buffer is static. Alternative solution would
> be to make it global and export it like, for example, panic_cpu.
It's not a static buffer, because the string is generated at runtime.
eg: https://elixir.bootlin.com/linux/latest/source/arch/arm/mm/init.c#L158
So it will be hard to avoid race conditions.
>
> Best Regards,
> Petr
>
^ permalink raw reply
* Re: [PATCH] printk: Add a short description string to kmsg_dump()
From: Kees Cook @ 2024-06-26 16:26 UTC (permalink / raw)
To: Jocelyn Falempe
Cc: Michael Ellerman, Nicholas Piggin, Christophe Leroy,
Naveen N. Rao, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Tony Luck,
Guilherme G. Piccoli, Petr Mladek, Steven Rostedt, John Ogness,
Sergey Senozhatsky, Andrew Morton, Jani Nikula,
Greg Kroah-Hartman, Kefeng Wang, Thomas Gleixner, Uros Bizjak,
linuxppc-dev, linux-kernel, dri-devel, linux-hyperv, linux-mtd,
linux-hardening
In-Reply-To: <20240625123954.211184-1-jfalempe@redhat.com>
On Tue, Jun 25, 2024 at 02:39:29PM +0200, Jocelyn Falempe wrote:
> kmsg_dump doesn't forward the panic reason string to the kmsg_dumper
> callback.
> This patch adds a new parameter "const char *desc" to the kmsg_dumper
> dump() callback, and update all drivers that are using it.
>
> To avoid updating all kmsg_dump() call, it adds a kmsg_dump_desc()
> function and a macro for backward compatibility.
>
> I've written this for drm_panic, but it can be useful for other
> kmsg_dumper.
> It allows to see the panic reason, like "sysrq triggered crash"
> or "VFS: Unable to mount root fs on xxxx" on the drm panic screen.
Seems reasonable. Given the prototype before/after:
dump(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason)
dump(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason,
const char *desc)
Perhaps this should instead be a struct that the panic fills in? Then
it'll be easy to adjust the struct in the future:
struct kmsg_dump_detail {
enum kmsg_dump_reason reason;
const char *description;
};
dump(struct kmsg_dumper *dumper, struct kmsg_dump *detail)
This .cocci could do the conversion:
@ dump_func @
identifier DUMPER, CALLBACK;
@@
struct kmsg_dumper DUMPER = {
.dump = CALLBACK,
};
@ detail @
identifier dump_func.CALLBACK;
identifier DUMPER, REASON;
@@
CALLBACK(struct kmsg_dumper *DUMPER,
- enum kmsg_dump_reason REASON
+ struct kmsg_dump_detail *detail
)
{
<...
- REASON
+ detail->reason
...>
}
Also, just to double-check, doesn't the panic reason show up in the
kmsg_dump log itself (at the end?) I ask since for pstore, "desc" is
likely redundant since it's capturing the entire console log.
-Kees
Here's the patch from the above cocci:
diff -u -p a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -207,13 +207,13 @@ static int hv_die_panic_notify_crash(str
* buffer and call into Hyper-V to transfer the data.
*/
static void hv_kmsg_dump(struct kmsg_dumper *dumper,
- enum kmsg_dump_reason reason)
+ struct kmsg_dump_detail *detail)
{
struct kmsg_dump_iter iter;
size_t bytes_written;
/* We are only interested in panics. */
- if (reason != KMSG_DUMP_PANIC || !sysctl_record_panic_msg)
+ if (detail->reason != KMSG_DUMP_PANIC || !sysctl_record_panic_msg)
return;
/*
diff -u -p a/arch/powerpc/platforms/powernv/opal-kmsg.c b/arch/powerpc/platforms/powernv/opal-kmsg.c
--- a/arch/powerpc/platforms/powernv/opal-kmsg.c
+++ b/arch/powerpc/platforms/powernv/opal-kmsg.c
@@ -20,13 +20,13 @@
* message, it just ensures that OPAL completely flushes the console buffer.
*/
static void kmsg_dump_opal_console_flush(struct kmsg_dumper *dumper,
- enum kmsg_dump_reason reason)
+ struct kmsg_dump_detail *detail)
{
/*
* Outside of a panic context the pollers will continue to run,
* so we don't need to do any special flushing.
*/
- if (reason != KMSG_DUMP_PANIC)
+ if (detail->reason != KMSG_DUMP_PANIC)
return;
opal_flush_console(0);
diff -u -p a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
--- a/arch/powerpc/kernel/nvram_64.c
+++ b/arch/powerpc/kernel/nvram_64.c
@@ -73,7 +73,7 @@ static const char *nvram_os_partitions[]
};
static void oops_to_nvram(struct kmsg_dumper *dumper,
- enum kmsg_dump_reason reason);
+ struct kmsg_dump_detail *detail);
static struct kmsg_dumper nvram_kmsg_dumper = {
.dump = oops_to_nvram
@@ -643,7 +643,7 @@ void __init nvram_init_oops_partition(in
* partition. If that's too much, go back and capture uncompressed text.
*/
static void oops_to_nvram(struct kmsg_dumper *dumper,
- enum kmsg_dump_reason reason)
+ struct kmsg_dump_detail *detail)
{
struct oops_log_info *oops_hdr = (struct oops_log_info *)oops_buf;
static unsigned int oops_count = 0;
@@ -655,7 +655,7 @@ static void oops_to_nvram(struct kmsg_du
unsigned int err_type = ERR_TYPE_KERNEL_PANIC_GZ;
int rc = -1;
- switch (reason) {
+ switch (detail->reason) {
case KMSG_DUMP_SHUTDOWN:
/* These are almost always orderly shutdowns. */
return;
@@ -671,7 +671,7 @@ static void oops_to_nvram(struct kmsg_du
break;
default:
pr_err("%s: ignoring unrecognized KMSG_DUMP_* reason %d\n",
- __func__, (int) reason);
+ __func__, (int) detail->reason);
return;
}
warning: detail, node 59: record.reason = ... ;[1,2,21,22,32] in pstore_dump may be inconsistently modified
warning: detail, node 105: if[1,2,21,22,54] in pstore_dump may be inconsistently modified
diff -u -p a/fs/pstore/platform.c b/fs/pstore/platform.c
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -275,7 +275,7 @@ void pstore_record_init(struct pstore_re
* end of the buffer.
*/
static void pstore_dump(struct kmsg_dumper *dumper,
- enum kmsg_dump_reason reason)
+ struct kmsg_dump_detail *detail)
{
struct kmsg_dump_iter iter;
unsigned long total = 0;
@@ -285,9 +285,9 @@ static void pstore_dump(struct kmsg_dump
int saved_ret = 0;
int ret;
- why = kmsg_dump_reason_str(reason);
+ why = kmsg_dump_reason_str(detail->reason);
- if (pstore_cannot_block_path(reason)) {
+ if (pstore_cannot_block_path(detail->reason)) {
if (!spin_trylock_irqsave(&psinfo->buf_lock, flags)) {
pr_err("dump skipped in %s path because of concurrent dump\n",
in_nmi() ? "NMI" : why);
@@ -311,7 +311,7 @@ static void pstore_dump(struct kmsg_dump
pstore_record_init(&record, psinfo);
record.type = PSTORE_TYPE_DMESG;
record.count = oopscount;
- record.reason = reason;
+ record.reason = detail->reason;
record.part = part;
record.buf = psinfo->buf;
@@ -352,7 +352,7 @@ static void pstore_dump(struct kmsg_dump
}
ret = psinfo->write(&record);
- if (ret == 0 && reason == KMSG_DUMP_OOPS) {
+ if (ret == 0 && detail->reason == KMSG_DUMP_OOPS) {
pstore_new_entry = 1;
pstore_timer_kick();
} else {
diff -u -p a/arch/um/kernel/kmsg_dump.c b/arch/um/kernel/kmsg_dump.c
--- a/arch/um/kernel/kmsg_dump.c
+++ b/arch/um/kernel/kmsg_dump.c
@@ -8,7 +8,7 @@
#include <os.h>
static void kmsg_dumper_stdout(struct kmsg_dumper *dumper,
- enum kmsg_dump_reason reason)
+ struct kmsg_dump_detail *detail)
{
static struct kmsg_dump_iter iter;
static DEFINE_SPINLOCK(lock);
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v2] PCI: hv: fix reading of PCI_INTERRUPT_PIN
From: Bjorn Helgaas @ 2024-06-26 15:10 UTC (permalink / raw)
To: Wei Liu
Cc: Linux on Hyper-V List, stable, K. Y. Srinivasan, Haiyang Zhang,
Dexuan Cui, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Bjorn Helgaas, Jake Oshins,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list
In-Reply-To: <20240621210018.350429-1-wei.liu@kernel.org>
1) Capitalize subject to match history
2) Say something more specific than "fix reading ..."
Apparently this returns garbage in some case where you want to return
zero?
On Fri, Jun 21, 2024 at 09:00:18PM +0000, Wei Liu wrote:
> The intent of the code snippet is to always return 0 for both
> PCI_INTERRUPT_LINE and PCI_INTERRUPT_PIN.
>
> The check misses PCI_INTERRUPT_PIN. This patch fixes that.
>
> This is discovered by this call in VFIO:
>
> pci_read_config_byte(vdev->pdev, PCI_INTERRUPT_PIN, &pin);
>
> The old code does not set *val to 0 because it misses the check for
> PCI_INTERRUPT_PIN.
>
> Fixes: 4daace0d8ce8 ("PCI: hv: Add paravirtual PCI front-end for Microsoft Hyper-V VMs")
> Cc: stable@kernel.org
> Signed-off-by: Wei Liu <wei.liu@kernel.org>
> ---
> v2:
> * Change the commit subject line and message
> * Change the code according to feedback
> ---
> drivers/pci/controller/pci-hyperv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 5992280e8110..cdd5be16021d 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -1130,8 +1130,8 @@ static void _hv_pcifront_read_config(struct hv_pci_dev *hpdev, int where,
> PCI_CAPABILITY_LIST) {
> /* ROM BARs are unimplemented */
> *val = 0;
> - } else if (where >= PCI_INTERRUPT_LINE && where + size <=
> - PCI_INTERRUPT_PIN) {
> + } else if ((where >= PCI_INTERRUPT_LINE && where + size <= PCI_INTERRUPT_PIN) ||
> + (where >= PCI_INTERRUPT_PIN && where + size <= PCI_MIN_GNT)) {
> /*
> * Interrupt Line and Interrupt PIN are hard-wired to zero
> * because this front-end only supports message-signaled
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH] printk: Add a short description string to kmsg_dump()
From: Petr Mladek @ 2024-06-26 8:00 UTC (permalink / raw)
To: Jocelyn Falempe
Cc: Michael Ellerman, Nicholas Piggin, Christophe Leroy,
Naveen N. Rao, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Kees Cook, Tony Luck,
Guilherme G. Piccoli, Steven Rostedt, John Ogness,
Sergey Senozhatsky, Andrew Morton, Jani Nikula,
Greg Kroah-Hartman, Kefeng Wang, Thomas Gleixner, Uros Bizjak,
linuxppc-dev, linux-kernel, dri-devel, linux-hyperv, linux-mtd,
linux-hardening
In-Reply-To: <20240625123954.211184-1-jfalempe@redhat.com>
On Tue 2024-06-25 14:39:29, Jocelyn Falempe wrote:
> kmsg_dump doesn't forward the panic reason string to the kmsg_dumper
> callback.
> This patch adds a new parameter "const char *desc" to the kmsg_dumper
> dump() callback, and update all drivers that are using it.
>
> To avoid updating all kmsg_dump() call, it adds a kmsg_dump_desc()
> function and a macro for backward compatibility.
>
> I've written this for drm_panic, but it can be useful for other
> kmsg_dumper.
> It allows to see the panic reason, like "sysrq triggered crash"
> or "VFS: Unable to mount root fs on xxxx" on the drm panic screen.
>
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> ---
> arch/powerpc/kernel/nvram_64.c | 3 ++-
> arch/powerpc/platforms/powernv/opal-kmsg.c | 3 ++-
> drivers/gpu/drm/drm_panic.c | 3 ++-
> drivers/hv/hv_common.c | 3 ++-
> drivers/mtd/mtdoops.c | 3 ++-
> fs/pstore/platform.c | 3 ++-
> include/linux/kmsg_dump.h | 13 ++++++++++---
> kernel/panic.c | 2 +-
> kernel/printk/printk.c | 8 +++++---
> 9 files changed, 28 insertions(+), 13 deletions(-)
The parameter is added into all dumpers. I guess that it would be
used only drm_panic() because it is graphics and might be "fancy".
The others simply dump the log buffer and the reason is in
the dumped log as well.
Anyway, the passed buffer is static. Alternative solution would
be to make it global and export it like, for example, panic_cpu.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH v1 0/3] mm/memory_hotplug: use PageOffline() instead of PageReserved() for !ZONE_DEVICE
From: David Hildenbrand @ 2024-06-26 5:01 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, linux-mm, linux-hyperv, virtualization, xen-devel,
kasan-dev, Mike Rapoport, Oscar Salvador, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Michael S. Tsirkin,
Jason Wang, Xuan Zhuo, Eugenio Pérez, Juergen Gross,
Stefano Stabellini, Oleksandr Tyshchenko, Alexander Potapenko,
Marco Elver, Dmitry Vyukov
In-Reply-To: <20240625154344.9f3db1ddfe2cb9cdd5583783@linux-foundation.org>
On 26.06.24 00:43, Andrew Morton wrote:
> afaict we're in decent state to move this series into mm-stable. I've
> tagged the following issues:
>
> https://lkml.kernel.org/r/80532f73e52e2c21fdc9aac7bce24aefb76d11b0.camel@linux.intel.com
> https://lkml.kernel.org/r/30b5d493-b7c2-4e63-86c1-dcc73d21dc15@redhat.com
>
> Have these been addressed and are we ready to send this series into the world?
Yes, should all be addressed and this should be good to go.
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v1 0/3] mm/memory_hotplug: use PageOffline() instead of PageReserved() for !ZONE_DEVICE
From: Andrew Morton @ 2024-06-25 22:43 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, linux-mm, linux-hyperv, virtualization, xen-devel,
kasan-dev, Mike Rapoport, Oscar Salvador, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Michael S. Tsirkin,
Jason Wang, Xuan Zhuo, Eugenio Pérez, Juergen Gross,
Stefano Stabellini, Oleksandr Tyshchenko, Alexander Potapenko,
Marco Elver, Dmitry Vyukov
In-Reply-To: <20240607090939.89524-1-david@redhat.com>
afaict we're in decent state to move this series into mm-stable. I've
tagged the following issues:
https://lkml.kernel.org/r/80532f73e52e2c21fdc9aac7bce24aefb76d11b0.camel@linux.intel.com
https://lkml.kernel.org/r/30b5d493-b7c2-4e63-86c1-dcc73d21dc15@redhat.com
Have these been addressed and are we ready to send this series into the world?
Thanks.
^ permalink raw reply
* [PATCH v3] net: mana: Fix possible double free in error handling path
From: Ma Ke @ 2024-06-25 13:03 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, davem, edumazet, kuba, pabeni,
shradhagupta, horms, kotaranov, longli, schakrabarti,
erick.archer, leon
Cc: linux-hyperv, netdev, linux-kernel, Ma Ke
When auxiliary_device_add() returns error and then calls
auxiliary_device_uninit(), callback function adev_release
calls kfree(madev). We shouldn't call kfree(madev) again
in the error handling path. Set 'madev' to NULL.
Fixes: a69839d4327d ("net: mana: Add support for auxiliary device")
Signed-off-by: Ma Ke <make24@iscas.ac.cn>
---
Changes in v3:
- added a "Fixes" line as suggested.
Changes in v2:
- streamlined the patch according suggestions;
- revised the description.
---
drivers/net/ethernet/microsoft/mana/mana_en.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index d087cf954f75..608ad31a9702 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -2798,6 +2798,8 @@ static int add_adev(struct gdma_dev *gd)
if (ret)
goto init_fail;
+ /* madev is owned by the auxiliary device */
+ madev = NULL;
ret = auxiliary_device_add(adev);
if (ret)
goto add_fail;
--
2.25.1
^ permalink raw reply related
* [PATCH] printk: Add a short description string to kmsg_dump()
From: Jocelyn Falempe @ 2024-06-25 12:39 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Christophe Leroy,
Naveen N. Rao, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Kees Cook, Tony Luck,
Guilherme G. Piccoli, Petr Mladek, Steven Rostedt, John Ogness,
Sergey Senozhatsky, Jocelyn Falempe, Andrew Morton, Jani Nikula,
Greg Kroah-Hartman, Kefeng Wang, Thomas Gleixner, Uros Bizjak,
linuxppc-dev, linux-kernel, dri-devel, linux-hyperv, linux-mtd,
linux-hardening
kmsg_dump doesn't forward the panic reason string to the kmsg_dumper
callback.
This patch adds a new parameter "const char *desc" to the kmsg_dumper
dump() callback, and update all drivers that are using it.
To avoid updating all kmsg_dump() call, it adds a kmsg_dump_desc()
function and a macro for backward compatibility.
I've written this for drm_panic, but it can be useful for other
kmsg_dumper.
It allows to see the panic reason, like "sysrq triggered crash"
or "VFS: Unable to mount root fs on xxxx" on the drm panic screen.
Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---
arch/powerpc/kernel/nvram_64.c | 3 ++-
arch/powerpc/platforms/powernv/opal-kmsg.c | 3 ++-
drivers/gpu/drm/drm_panic.c | 3 ++-
drivers/hv/hv_common.c | 3 ++-
drivers/mtd/mtdoops.c | 3 ++-
fs/pstore/platform.c | 3 ++-
include/linux/kmsg_dump.h | 13 ++++++++++---
kernel/panic.c | 2 +-
kernel/printk/printk.c | 8 +++++---
9 files changed, 28 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
index e385d3164648c..6b3a80d8cfa64 100644
--- a/arch/powerpc/kernel/nvram_64.c
+++ b/arch/powerpc/kernel/nvram_64.c
@@ -643,7 +643,8 @@ void __init nvram_init_oops_partition(int rtas_partition_exists)
* partition. If that's too much, go back and capture uncompressed text.
*/
static void oops_to_nvram(struct kmsg_dumper *dumper,
- enum kmsg_dump_reason reason)
+ enum kmsg_dump_reason reason,
+ const char *desc)
{
struct oops_log_info *oops_hdr = (struct oops_log_info *)oops_buf;
static unsigned int oops_count = 0;
diff --git a/arch/powerpc/platforms/powernv/opal-kmsg.c b/arch/powerpc/platforms/powernv/opal-kmsg.c
index 6c3bc4b4da983..49b60de6feb04 100644
--- a/arch/powerpc/platforms/powernv/opal-kmsg.c
+++ b/arch/powerpc/platforms/powernv/opal-kmsg.c
@@ -20,7 +20,8 @@
* message, it just ensures that OPAL completely flushes the console buffer.
*/
static void kmsg_dump_opal_console_flush(struct kmsg_dumper *dumper,
- enum kmsg_dump_reason reason)
+ enum kmsg_dump_reason reason,
+ const char *desc)
{
/*
* Outside of a panic context the pollers will continue to run,
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index 293d4dcbc80da..88e9359fe6d78 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -604,7 +604,8 @@ static struct drm_plane *to_drm_plane(struct kmsg_dumper *kd)
return container_of(kd, struct drm_plane, kmsg_panic);
}
-static void drm_panic(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason)
+static void drm_panic(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason,
+ const char *desc)
{
struct drm_plane *plane = to_drm_plane(dumper);
diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index 9c452bfbd5719..b0786ee9c94e3 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -207,7 +207,8 @@ static int hv_die_panic_notify_crash(struct notifier_block *self,
* buffer and call into Hyper-V to transfer the data.
*/
static void hv_kmsg_dump(struct kmsg_dumper *dumper,
- enum kmsg_dump_reason reason)
+ enum kmsg_dump_reason reason,
+ const char *desc)
{
struct kmsg_dump_iter iter;
size_t bytes_written;
diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c
index 2f11585b5613e..c618999a96832 100644
--- a/drivers/mtd/mtdoops.c
+++ b/drivers/mtd/mtdoops.c
@@ -298,7 +298,8 @@ static void find_next_position(struct mtdoops_context *cxt)
}
static void mtdoops_do_dump(struct kmsg_dumper *dumper,
- enum kmsg_dump_reason reason)
+ enum kmsg_dump_reason reason,
+ const char *desc)
{
struct mtdoops_context *cxt = container_of(dumper,
struct mtdoops_context, dump);
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 3497ede88aa01..a6ed5d56021ef 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -275,7 +275,8 @@ void pstore_record_init(struct pstore_record *record,
* end of the buffer.
*/
static void pstore_dump(struct kmsg_dumper *dumper,
- enum kmsg_dump_reason reason)
+ enum kmsg_dump_reason reason,
+ const char *desc)
{
struct kmsg_dump_iter iter;
unsigned long total = 0;
diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
index 906521c2329ca..a8f8a6204542d 100644
--- a/include/linux/kmsg_dump.h
+++ b/include/linux/kmsg_dump.h
@@ -49,13 +49,15 @@ struct kmsg_dump_iter {
*/
struct kmsg_dumper {
struct list_head list;
- void (*dump)(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason);
+ void (*dump)(struct kmsg_dumper *dumper,
+ enum kmsg_dump_reason reason,
+ const char *desc);
enum kmsg_dump_reason max_reason;
bool registered;
};
#ifdef CONFIG_PRINTK
-void kmsg_dump(enum kmsg_dump_reason reason);
+void kmsg_dump_desc(enum kmsg_dump_reason reason, const char *desc);
bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog,
char *line, size_t size, size_t *len);
@@ -71,7 +73,7 @@ int kmsg_dump_unregister(struct kmsg_dumper *dumper);
const char *kmsg_dump_reason_str(enum kmsg_dump_reason reason);
#else
-static inline void kmsg_dump(enum kmsg_dump_reason reason)
+static inline void kmsg_dump_desc(enum kmsg_dump_reason reason, const char *desc)
{
}
@@ -107,4 +109,9 @@ static inline const char *kmsg_dump_reason_str(enum kmsg_dump_reason reason)
}
#endif
+static inline void kmsg_dump(enum kmsg_dump_reason reason)
+{
+ kmsg_dump_desc(reason, NULL);
+}
+
#endif /* _LINUX_KMSG_DUMP_H */
diff --git a/kernel/panic.c b/kernel/panic.c
index 0843a275531a7..0a8b29c44f3c1 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -378,7 +378,7 @@ void panic(const char *fmt, ...)
panic_print_sys_info(false);
- kmsg_dump(KMSG_DUMP_PANIC);
+ kmsg_dump_desc(KMSG_DUMP_PANIC, buf);
/*
* If you doubt kdump always works fine in any situation,
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index d0bff0b0abfdb..2e7195894e41a 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -4287,14 +4287,16 @@ const char *kmsg_dump_reason_str(enum kmsg_dump_reason reason)
EXPORT_SYMBOL_GPL(kmsg_dump_reason_str);
/**
- * kmsg_dump - dump kernel log to kernel message dumpers.
+ * kmsg_dump_desc - dump kernel log to kernel message dumpers.
* @reason: the reason (oops, panic etc) for dumping
+ * @desc: a short string to describe what caused the panic or oops. Can be NULL
+ * if no additional description is available.
*
* Call each of the registered dumper's dump() callback, which can
* retrieve the kmsg records with kmsg_dump_get_line() or
* kmsg_dump_get_buffer().
*/
-void kmsg_dump(enum kmsg_dump_reason reason)
+void kmsg_dump_desc(enum kmsg_dump_reason reason, const char *desc)
{
struct kmsg_dumper *dumper;
@@ -4314,7 +4316,7 @@ void kmsg_dump(enum kmsg_dump_reason reason)
continue;
/* invoke dumper which will iterate over records */
- dumper->dump(dumper, reason);
+ dumper->dump(dumper, reason, desc);
}
rcu_read_unlock();
}
base-commit: f76698bd9a8ca01d3581236082d786e9a6b72bb7
--
2.45.1
^ permalink raw reply related
* RE: [PATCH v2] net: mana: Fix possible double free in error handling path
From: Konstantin Taranov @ 2024-06-25 9:41 UTC (permalink / raw)
To: Ma Ke, KY Srinivasan, Haiyang Zhang, wei.liu@kernel.org,
Dexuan Cui, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com,
shradhagupta@linux.microsoft.com, horms@kernel.org,
schakrabarti@linux.microsoft.com, erick.archer@outlook.com
Cc: linux-hyperv@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20240625083816.2623936-1-make24@iscas.ac.cn>
> When auxiliary_device_add() returns error and then calls
> auxiliary_device_uninit(), callback function adev_release calls kfree(madev).
> We shouldn't call kfree(madev) again in the error handling path. Set 'madev'
> to NULL.
>
> Signed-off-by: Ma Ke <make24@iscas.ac.cn>
> ---
> Changes in v2:
> - streamlined the patch according suggestions;
> - revised the description.
The change is ok, but the commit message is missing a "Fixes" tag/line.
- Konstantin
^ permalink raw reply
* RE: [PATCH v2] net: mana: Fix possible double free in error handling path
From: Konstantin Taranov @ 2024-06-25 9:38 UTC (permalink / raw)
To: Ma Ke, KY Srinivasan, Haiyang Zhang, wei.liu@kernel.org,
Dexuan Cui, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com,
shradhagupta@linux.microsoft.com, horms@kernel.org,
schakrabarti@linux.microsoft.com, erick.archer@outlook.com
Cc: linux-hyperv@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20240625083816.2623936-1-make24@iscas.ac.cn>
> When auxiliary_device_add() returns error and then calls
> auxiliary_device_uninit(), callback function adev_release calls kfree(madev).
> We shouldn't call kfree(madev) again in the error handling path. Set 'madev'
> to NULL.
>
> Signed-off-by: Ma Ke <make24@iscas.ac.cn>
> ---
> Changes in v2:
> - streamlined the patch according suggestions;
> - revised the description.
> ---
> drivers/net/ethernet/microsoft/mana/mana_en.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c
> b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index d087cf954f75..608ad31a9702 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -2798,6 +2798,8 @@ static int add_adev(struct gdma_dev *gd)
> if (ret)
> goto init_fail;
>
> + /* madev is owned by the auxiliary device */
> + madev = NULL;
> ret = auxiliary_device_add(adev);
> if (ret)
> goto add_fail;
> --
> 2.25.1
Reviewed-by: Konstantin Taranov <kotaranov@microsoft.com>
^ permalink raw reply
* Re: [PATCH v2] net: mana: Fix possible double free in error handling path
From: Przemek Kitszel @ 2024-06-25 9:13 UTC (permalink / raw)
To: Ma Ke
Cc: linux-hyperv, netdev, linux-kernel, kys, haiyangz, wei.liu, decui,
davem, edumazet, kuba, pabeni, shradhagupta, horms, kotaranov,
schakrabarti, erick.archer
In-Reply-To: <20240625083816.2623936-1-make24@iscas.ac.cn>
On 6/25/24 10:38, Ma Ke wrote:
> When auxiliary_device_add() returns error and then calls
> auxiliary_device_uninit(), callback function adev_release
> calls kfree(madev). We shouldn't call kfree(madev) again
> in the error handling path. Set 'madev' to NULL.
>
> Signed-off-by: Ma Ke <make24@iscas.ac.cn>
> ---
> Changes in v2:
> - streamlined the patch according suggestions;
> - revised the description.
> ---
> drivers/net/ethernet/microsoft/mana/mana_en.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index d087cf954f75..608ad31a9702 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -2798,6 +2798,8 @@ static int add_adev(struct gdma_dev *gd)
> if (ret)
> goto init_fail;
>
> + /* madev is owned by the auxiliary device */
> + madev = NULL;
> ret = auxiliary_device_add(adev);
> if (ret)
> goto add_fail;
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
^ permalink raw reply
* [PATCH v2] net: mana: Fix possible double free in error handling path
From: Ma Ke @ 2024-06-25 8:38 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, davem, edumazet, kuba, pabeni,
shradhagupta, horms, kotaranov, schakrabarti, erick.archer
Cc: linux-hyperv, netdev, linux-kernel, Ma Ke
When auxiliary_device_add() returns error and then calls
auxiliary_device_uninit(), callback function adev_release
calls kfree(madev). We shouldn't call kfree(madev) again
in the error handling path. Set 'madev' to NULL.
Signed-off-by: Ma Ke <make24@iscas.ac.cn>
---
Changes in v2:
- streamlined the patch according suggestions;
- revised the description.
---
drivers/net/ethernet/microsoft/mana/mana_en.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index d087cf954f75..608ad31a9702 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -2798,6 +2798,8 @@ static int add_adev(struct gdma_dev *gd)
if (ret)
goto init_fail;
+ /* madev is owned by the auxiliary device */
+ madev = NULL;
ret = auxiliary_device_add(adev);
if (ret)
goto add_fail;
--
2.25.1
^ permalink raw reply related
* RE: [RFC 11/12] Drivers: hv: vmbus: Wait for MODIFYCHANNEL to finish when offlining CPUs
From: Michael Kelley @ 2024-06-24 19:32 UTC (permalink / raw)
To: Boqun Feng
Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, tglx@linutronix.de, mingo@redhat.com,
bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org,
hpa@zytor.com, lpieralisi@kernel.org, kw@linux.com,
robh@kernel.org, bhelgaas@google.com,
James.Bottomley@hansenpartnership.com, martin.petersen@oracle.com,
arnd@arndb.de, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-arch@vger.kernel.org,
maz@kernel.org, den@valinux.co.jp, jgowans@amazon.com,
dawei.li@shingroup.cn
In-Reply-To: <ZnmzBi2y1eq269QA@boqun-archlinux>
From: Boqun Feng <boqun.feng@gmail.com> Sent: Monday, June 24, 2024 10:55 AM
>
> Hi Michael,
>
> On Mon, Jun 03, 2024 at 10:09:39PM -0700, mhkelley58@gmail.com wrote:
> [...]
> > diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> > index bf35bb40c55e..571b2955b38e 100644
> > --- a/drivers/hv/hyperv_vmbus.h
> > +++ b/drivers/hv/hyperv_vmbus.h
> > @@ -264,6 +264,14 @@ struct vmbus_connection {
> > struct irq_domain *vmbus_irq_domain;
> > struct irq_chip vmbus_irq_chip;
> >
> > + /*
> > + * VM-wide counts of MODIFYCHANNEL messages sent and completed.
> > + * Used when taking a CPU offline to make sure the relevant
> > + * MODIFYCHANNEL messages have been completed.
> > + */
> > + u64 modchan_sent;
> > + u64 modchan_completed;
> > +
>
> Looks to me, we can just use atomic64_t here: modifying channels is far
> from hotpath, so the cost of atomic increment is not a big issue, and we
> avoid possible data races now and in the future.
>
> Thoughts?
At one point in the development, I did have these as atomic64_t.
And I agree that the usage is pretty infrequent, so the atomic costs
aren't an issue. But both fields are already safe. modchan_sent
is covered by vmbus_connection.set_affinity_lock as held in
vmbus_irq_set_affinity(). And modchan_completed is OK because
it is only ever incremented by code running on VMBUS_CONNECT_CPU,
per the comment in vmbus_onmodifychannel_response().
Converting to atomic64_t wouldn't hurt anything, and arguably
would provide additional robustness, but it just didn't seem
necessary.
Michael
^ permalink raw reply
* Re: [RFC 11/12] Drivers: hv: vmbus: Wait for MODIFYCHANNEL to finish when offlining CPUs
From: Boqun Feng @ 2024-06-24 17:55 UTC (permalink / raw)
To: mhklinux
Cc: kys, haiyangz, wei.liu, decui, tglx, mingo, bp, dave.hansen, x86,
hpa, lpieralisi, kw, robh, bhelgaas, James.Bottomley,
martin.petersen, arnd, linux-hyperv, linux-kernel, linux-pci,
linux-scsi, linux-arch, maz, den, jgowans, dawei.li
In-Reply-To: <20240604050940.859909-12-mhklinux@outlook.com>
Hi Michael,
On Mon, Jun 03, 2024 at 10:09:39PM -0700, mhkelley58@gmail.com wrote:
[...]
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index bf35bb40c55e..571b2955b38e 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -264,6 +264,14 @@ struct vmbus_connection {
> struct irq_domain *vmbus_irq_domain;
> struct irq_chip vmbus_irq_chip;
>
> + /*
> + * VM-wide counts of MODIFYCHANNEL messages sent and completed.
> + * Used when taking a CPU offline to make sure the relevant
> + * MODIFYCHANNEL messages have been completed.
> + */
> + u64 modchan_sent;
> + u64 modchan_completed;
> +
Looks to me, we can just use atomic64_t here: modifying channels is far
from hotpath, so the cost of atomic increment is not a big issue, and we
avoid possible data races now and in the future.
Thoughts?
Regards,
Boqun
> /*
> * An offer message is handled first on the work_queue, and then
> * is further handled on handle_primary_chan_wq or
> --
> 2.25.1
>
^ permalink raw reply
* RE: [PATCH] net: mana: Fix possible double free in error handling path
From: Konstantin Taranov @ 2024-06-24 10:18 UTC (permalink / raw)
To: Sai Krishna Gajula, Ma Ke, KY Srinivasan, Haiyang Zhang,
wei.liu@kernel.org, Dexuan Cui, davem@davemloft.net,
edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
shradhagupta@linux.microsoft.com, horms@kernel.org,
linyunsheng@huawei.com, schakrabarti@linux.microsoft.com,
erick.archer@outlook.com
Cc: linux-hyperv@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <BY3PR18MB47074E845C5C5CD540C8552DA0D42@BY3PR18MB4707.namprd18.prod.outlook.com>
> > - kfree(madev);
> I think you can just avoid using add_fail and keep/retain rest of init_fail, idx_fail
> conditions in old way right?
I do agree with Sai. I think the patch can be just:
@@ -2797,7 +2797,8 @@ static int add_adev(struct gdma_dev *gd)
ret = auxiliary_device_init(adev);
if (ret)
goto init_fail;
-
+ /* madev is owned by the auxiliary device */
+ madev = NULL;
ret = auxiliary_device_add(adev);
if (ret)
goto add_fail;
- Konstantin
^ permalink raw reply
* Re: [PATCH] net: mana: Fix possible double free in error handling path
From: Przemek Kitszel @ 2024-06-24 9:16 UTC (permalink / raw)
To: Ma Ke
Cc: linux-hyperv, netdev, linux-kernel, kys, haiyangz, wei.liu, decui,
davem, edumazet, kuba, pabeni, shradhagupta, horms, kotaranov,
linyunsheng, schakrabarti, erick.archer
In-Reply-To: <20240624032112.2286526-1-make24@iscas.ac.cn>
On 6/24/24 05:21, Ma Ke wrote:
> When auxiliary_device_add() returns error and then calls
> auxiliary_device_uninit(), callback function adev_release
I would add () after function name
> calls kfree(madev) to free memory. We shouldn't call kfree(padev)
there is no `padev` in the patch :)
"to free memory" part sounds redundant.
> again in the error handling path.
>
> Signed-off-by: Ma Ke <make24@iscas.ac.cn>
you need a Fixes: tag
> ---
> drivers/net/ethernet/microsoft/mana/mana_en.c | 31 +++++++++----------
> 1 file changed, 14 insertions(+), 17 deletions(-)
>
^ permalink raw reply
* RE: [PATCH] net: mana: Fix possible double free in error handling path
From: Sai Krishna Gajula @ 2024-06-24 8:38 UTC (permalink / raw)
To: Ma Ke, kys@microsoft.com, haiyangz@microsoft.com,
wei.liu@kernel.org, decui@microsoft.com, davem@davemloft.net,
edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
shradhagupta@linux.microsoft.com, horms@kernel.org,
kotaranov@microsoft.com, linyunsheng@huawei.com,
schakrabarti@linux.microsoft.com, erick.archer@outlook.com
Cc: linux-hyperv@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20240624032112.2286526-1-make24@iscas.ac.cn>
> -----Original Message-----
> From: Ma Ke <make24@iscas.ac.cn>
> Sent: Monday, June 24, 2024 8:51 AM
> To: kys@microsoft.com; haiyangz@microsoft.com; wei.liu@kernel.org;
> decui@microsoft.com; davem@davemloft.net; edumazet@google.com;
> kuba@kernel.org; pabeni@redhat.com; shradhagupta@linux.microsoft.com;
> horms@kernel.org; kotaranov@microsoft.com; linyunsheng@huawei.com;
> schakrabarti@linux.microsoft.com; make24@iscas.ac.cn;
> erick.archer@outlook.com
> Cc: linux-hyperv@vger.kernel.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: [PATCH] net: mana: Fix possible double free in error
> handling path
>
> When auxiliary_device_add() returns error and then calls
> auxiliary_device_uninit(), callback function adev_release calls kfree(madev)
> to free memory. We shouldn't call kfree(padev) again in the error handling
> path. Signed-off-by: Ma Ke <make24@ iscas. ac. cn>
> When auxiliary_device_add() returns error and then calls
> auxiliary_device_uninit(), callback function adev_release calls kfree(madev)
> to free memory. We shouldn't call kfree(padev) again in the error handling
> path.
>
> Signed-off-by: Ma Ke <make24@iscas.ac.cn>
> ---
> drivers/net/ethernet/microsoft/mana/mana_en.c | 31 +++++++++----------
> 1 file changed, 14 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c
> b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index d087cf954f75..1754c92a6c15 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -2785,8 +2785,10 @@ static int add_adev(struct gdma_dev *gd)
>
> adev = &madev->adev;
> ret = mana_adev_idx_alloc();
> - if (ret < 0)
> - goto idx_fail;
> + if (ret < 0) {
> + kfree(madev);
> + return ret;
> + }
> adev->id = ret;
>
> adev->name = "rdma";
> @@ -2795,26 +2797,21 @@ static int add_adev(struct gdma_dev *gd)
> madev->mdev = gd;
>
> ret = auxiliary_device_init(adev);
> - if (ret)
> - goto init_fail;
> + if (ret) {
> + mana_adev_idx_free(adev->id);
> + kfree(madev);
> + return ret;
> + }
>
> ret = auxiliary_device_add(adev);
> - if (ret)
> - goto add_fail;
> + if (ret) {
> + auxiliary_device_uninit(adev);
> + mana_adev_idx_free(adev->id);
> + return ret;
> + }
>
> gd->adev = adev;
> return 0;
> -
> -add_fail:
> - auxiliary_device_uninit(adev);
> -
> -init_fail:
> - mana_adev_idx_free(adev->id);
> -
> -idx_fail:
> - kfree(madev);
I think you can just avoid using add_fail and keep/retain rest of init_fail, idx_fail conditions in old way right?
> -
> - return ret;
> }
>
> int mana_probe(struct gdma_dev *gd, bool resuming)
> --
> 2.25.1
>
^ permalink raw reply
* Re: [RFC 01/12] Drivers: hv: vmbus: Drop unsupported VMBus devices earlier
From: Wei Liu @ 2024-06-24 7:11 UTC (permalink / raw)
To: mhklinux
Cc: kys, haiyangz, wei.liu, decui, tglx, mingo, bp, dave.hansen, x86,
hpa, lpieralisi, kw, robh, bhelgaas, James.Bottomley,
martin.petersen, arnd, linux-hyperv, linux-kernel, linux-pci,
linux-scsi, linux-arch, maz, den, jgowans, dawei.li
In-Reply-To: <20240604050940.859909-2-mhklinux@outlook.com>
On Mon, Jun 03, 2024 at 10:09:29PM -0700, mhkelley58@gmail.com wrote:
> From: Michael Kelley <mhklinux@outlook.com>
>
> Because Hyper-V doesn't know ahead-of-time what operating system will be
> running in a guest VM, it may offer to Linux guests VMBus devices that are
> intended for use only in Windows guests. Currently in Linux, VMBus
> channels are created for these devices, and they are processed by
> vmbus_device_register(). While nothing further happens because no matching
> driver is found, the channel continues to exist.
>
> To avoid having the spurious channel, drop such devices immediately in
> vmbus_onoffer(), based on identifying the device in the
> vmbus_unsupported_devs table. If Hyper-V should issue a rescind request
> for the device, no matching channel will be found and the rescind will
> also be ignored.
>
> Since unsupported devices are dropped early, the check for unsupported
> devices in hv_get_dev_type() is redundant. Remove it.
>
> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
Acked-by: Wei Liu <wei.liu@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 1/1] Documentation: hyperv: Add overview of Confidential Computing VM support
From: Wei Liu @ 2024-06-24 7:07 UTC (permalink / raw)
To: mhklinux
Cc: kys, haiyangz, wei.liu, decui, corbet, linux-kernel, linux-hyperv,
linux-doc, linux-coco
In-Reply-To: <20240618165059.10174-1-mhklinux@outlook.com>
On Tue, Jun 18, 2024 at 09:50:59AM -0700, mhkelley58@gmail.com wrote:
> From: Michael Kelley <mhklinux@outlook.com>
>
> Add documentation topic for Confidential Computing (CoCo) VM support
> in Linux guests on Hyper-V.
>
> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
Applied to hyperv-fixes, thanks.
^ permalink raw reply
* Re: [PATCH v2] clocksource: hyper-v: Use lapic timer in a TDX VM without paravisor
From: Wei Liu @ 2024-06-24 7:06 UTC (permalink / raw)
To: Dexuan Cui
Cc: mhklinux, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
Daniel Lezcano, open list:Hyper-V/Azure CORE AND DRIVERS,
open list:X86 ARCHITECTURE (32-BIT AND 64-BIT), stable,
Roman Kisel
In-Reply-To: <20240621061614.8339-1-decui@microsoft.com>
On Thu, Jun 20, 2024 at 11:16:14PM -0700, Dexuan Cui wrote:
> In a TDX VM without paravisor, currently the default timer is the Hyper-V
> timer, which depends on the slow VM Reference Counter MSR: the Hyper-V TSC
> page is not enabled in such a VM because the VM uses Invariant TSC as a
> better clocksource and it's challenging to mark the Hyper-V TSC page shared
> in very early boot.
>
> Lower the rating of the Hyper-V timer so the local APIC timer becomes the
> the default timer in such a VM, and print a warning in case Invariant TSC
> is unavailable in such a VM. This change should cause no perceivable
> performance difference.
>
> Cc: stable@vger.kernel.org # 6.6+
> Reviewed-by: Roman Kisel <romank@linux.microsoft.com>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
Applied to hyperv-fixes. Thanks.
^ permalink raw reply
* Re: [PATCH] Drivers: hv: Remove deprecated hv_fcopy declarations
From: Wei Liu @ 2024-06-24 7:06 UTC (permalink / raw)
To: Rachel Menge
Cc: linux-hyperv, kys, haiyangz, wei.liu, decui, longli, chrco,
linux-kernel
In-Reply-To: <20240620225040.700563-1-rachelmenge@linux.microsoft.com>
On Thu, Jun 20, 2024 at 06:50:40PM -0400, Rachel Menge wrote:
> There are lingering hv_fcopy declarations which do not have definitions.
> The fcopy driver was removed in commit ec314f61e4fc ("Drivers: hv: Remove
> fcopy driver").
>
> Therefore, remove the hv_fcopy declarations which are no longer needed
> or defined.
>
> Fixes: ec314f61e4fc ("Drivers: hv: Remove fcopy driver")
> Signed-off-by: Rachel Menge <rachelmenge@linux.microsoft.com>
Applied to hyperv-fixes. Thanks.
^ permalink raw reply
* [PATCH] net: mana: Fix possible double free in error handling path
From: Ma Ke @ 2024-06-24 3:21 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, davem, edumazet, kuba, pabeni,
shradhagupta, horms, kotaranov, linyunsheng, schakrabarti, make24,
erick.archer
Cc: linux-hyperv, netdev, linux-kernel
When auxiliary_device_add() returns error and then calls
auxiliary_device_uninit(), callback function adev_release
calls kfree(madev) to free memory. We shouldn't call kfree(padev)
again in the error handling path.
Signed-off-by: Ma Ke <make24@iscas.ac.cn>
---
drivers/net/ethernet/microsoft/mana/mana_en.c | 31 +++++++++----------
1 file changed, 14 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index d087cf954f75..1754c92a6c15 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -2785,8 +2785,10 @@ static int add_adev(struct gdma_dev *gd)
adev = &madev->adev;
ret = mana_adev_idx_alloc();
- if (ret < 0)
- goto idx_fail;
+ if (ret < 0) {
+ kfree(madev);
+ return ret;
+ }
adev->id = ret;
adev->name = "rdma";
@@ -2795,26 +2797,21 @@ static int add_adev(struct gdma_dev *gd)
madev->mdev = gd;
ret = auxiliary_device_init(adev);
- if (ret)
- goto init_fail;
+ if (ret) {
+ mana_adev_idx_free(adev->id);
+ kfree(madev);
+ return ret;
+ }
ret = auxiliary_device_add(adev);
- if (ret)
- goto add_fail;
+ if (ret) {
+ auxiliary_device_uninit(adev);
+ mana_adev_idx_free(adev->id);
+ return ret;
+ }
gd->adev = adev;
return 0;
-
-add_fail:
- auxiliary_device_uninit(adev);
-
-init_fail:
- mana_adev_idx_free(adev->id);
-
-idx_fail:
- kfree(madev);
-
- return ret;
}
int mana_probe(struct gdma_dev *gd, bool resuming)
--
2.25.1
^ permalink raw reply related
* RE: [PATCH v2] PCI: hv: fix reading of PCI_INTERRUPT_PIN
From: Michael Kelley @ 2024-06-23 22:05 UTC (permalink / raw)
To: Wei Liu, Linux on Hyper-V List
Cc: stable@kernel.org, K. Y. Srinivasan, Haiyang Zhang, Dexuan Cui,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, Jake Oshins,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list
In-Reply-To: <20240621210018.350429-1-wei.liu@kernel.org>
From: Wei Liu <wei.liu@kernel.org> Sent: Friday, June 21, 2024 2:00 PM
>
> The intent of the code snippet is to always return 0 for both
> PCI_INTERRUPT_LINE and PCI_INTERRUPT_PIN.
>
> The check misses PCI_INTERRUPT_PIN. This patch fixes that.
>
> This is discovered by this call in VFIO:
>
> pci_read_config_byte(vdev->pdev, PCI_INTERRUPT_PIN, &pin);
>
> The old code does not set *val to 0 because it misses the check for
> PCI_INTERRUPT_PIN.
>
> Fixes: 4daace0d8ce8 ("PCI: hv: Add paravirtual PCI front-end for Microsoft Hyper-V
> VMs")
> Cc: stable@kernel.org
> Signed-off-by: Wei Liu <wei.liu@kernel.org>
> ---
> v2:
> * Change the commit subject line and message
> * Change the code according to feedback
> ---
> drivers/pci/controller/pci-hyperv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 5992280e8110..cdd5be16021d 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -1130,8 +1130,8 @@ static void _hv_pcifront_read_config(struct hv_pci_dev
> *hpdev, int where,
> PCI_CAPABILITY_LIST) {
> /* ROM BARs are unimplemented */
> *val = 0;
> - } else if (where >= PCI_INTERRUPT_LINE && where + size <=
> - PCI_INTERRUPT_PIN) {
> + } else if ((where >= PCI_INTERRUPT_LINE && where + size <=
> PCI_INTERRUPT_PIN) ||
> + (where >= PCI_INTERRUPT_PIN && where + size <= PCI_MIN_GNT)) {
> /*
> * Interrupt Line and Interrupt PIN are hard-wired to zero
> * because this front-end only supports message-signaled
> --
> 2.43.0
>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
^ permalink raw reply
* [PATCH v2] PCI: hv: fix reading of PCI_INTERRUPT_PIN
From: Wei Liu @ 2024-06-21 21:00 UTC (permalink / raw)
To: Linux on Hyper-V List
Cc: Wei Liu, stable, K. Y. Srinivasan, Haiyang Zhang, Dexuan Cui,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, Jake Oshins,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list
The intent of the code snippet is to always return 0 for both
PCI_INTERRUPT_LINE and PCI_INTERRUPT_PIN.
The check misses PCI_INTERRUPT_PIN. This patch fixes that.
This is discovered by this call in VFIO:
pci_read_config_byte(vdev->pdev, PCI_INTERRUPT_PIN, &pin);
The old code does not set *val to 0 because it misses the check for
PCI_INTERRUPT_PIN.
Fixes: 4daace0d8ce8 ("PCI: hv: Add paravirtual PCI front-end for Microsoft Hyper-V VMs")
Cc: stable@kernel.org
Signed-off-by: Wei Liu <wei.liu@kernel.org>
---
v2:
* Change the commit subject line and message
* Change the code according to feedback
---
drivers/pci/controller/pci-hyperv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 5992280e8110..cdd5be16021d 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -1130,8 +1130,8 @@ static void _hv_pcifront_read_config(struct hv_pci_dev *hpdev, int where,
PCI_CAPABILITY_LIST) {
/* ROM BARs are unimplemented */
*val = 0;
- } else if (where >= PCI_INTERRUPT_LINE && where + size <=
- PCI_INTERRUPT_PIN) {
+ } else if ((where >= PCI_INTERRUPT_LINE && where + size <= PCI_INTERRUPT_PIN) ||
+ (where >= PCI_INTERRUPT_PIN && where + size <= PCI_MIN_GNT)) {
/*
* Interrupt Line and Interrupt PIN are hard-wired to zero
* because this front-end only supports message-signaled
--
2.43.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox