[PATCH] Revert "ipmi_si: fix memory leak on new_smi"

[PATCH] Revert "ipmi_si: fix memory leak on new_smi"

[minyard]

From: Corey Minyard <cminyard@mvista.com>

This reverts commit c97e41076a298dbc4e910c33048e553658388eed.

A backport was requested of c0a32fe13cd32 "ipmi_si: fix memory leak on
new_smi", but the backport shouldn't have been done.  This change needs
to be reverted, as it can result in an oops and the previous code is
correct.

Reverts: c97e41076a29 ("ipmi_si: fix memory leak on new_smi")
Link: https://bbs.archlinux.org/viewtopic.php?pid=1757130#p1757130
Reported-by: Neil Romig <neil@sixtythree.me.uk>
Cc: Sasha Levin <alexander.levin@verizon.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---

This is for 4.14 stable tree only.  In hindsight, I should scrutinize
stable kernel requests from others in the IPMI tree.  Sorry about that.

 drivers/char/ipmi/ipmi_si_intf.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index e1cbb78..c04aa11 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -3469,7 +3469,6 @@ static int add_smi(struct smi_info *new_smi)
 				 ipmi_addr_src_to_str(new_smi->addr_source),
 				 si_to_str[new_smi->si_type]);
 			rv = -EBUSY;
-			kfree(new_smi);
 			goto out_err;
 		}
 	}
-- 
2.7.4

Re: [PATCH] Revert "ipmi_si: fix memory leak on new_smi"

[Greg KH]

On Wed, Dec 27, 2017 at 09:41:44AM -0600, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> This reverts commit c97e41076a298dbc4e910c33048e553658388eed.
> 
> A backport was requested of c0a32fe13cd32 "ipmi_si: fix memory leak on
> new_smi", but the backport shouldn't have been done.  This change needs
> to be reverted, as it can result in an oops and the previous code is
> correct.
> 
> Reverts: c97e41076a29 ("ipmi_si: fix memory leak on new_smi")
> Link: https://bbs.archlinux.org/viewtopic.php?pid=1757130#p1757130
> Reported-by: Neil Romig <neil@sixtythree.me.uk>
> Cc: Sasha Levin <alexander.levin@verizon.com>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
> 
> This is for 4.14 stable tree only.  In hindsight, I should scrutinize
> stable kernel requests from others in the IPMI tree.  Sorry about that.

I just queued this patch up from someone else already.  Thanks for it,
and sorry for the problems.

greg k-h

> 
>  drivers/char/ipmi/ipmi_si_intf.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
> index e1cbb78..c04aa11 100644
> --- a/drivers/char/ipmi/ipmi_si_intf.c
> +++ b/drivers/char/ipmi/ipmi_si_intf.c
> @@ -3469,7 +3469,6 @@ static int add_smi(struct smi_info *new_smi)
>  				 ipmi_addr_src_to_str(new_smi->addr_source),
>  				 si_to_str[new_smi->si_type]);
>  			rv = -EBUSY;
> -			kfree(new_smi);
>  			goto out_err;
>  		}
>  	}
> -- 
> 2.7.4

[PATCH] Revert "ipmi_si: fix memory leak on new_smi"

[John Einar Reitan]

This reverts commit c97e41076a298dbc4e910c33048e553658388eed,
which incorrectly was taken from upstream  c0a32fe13cd323ca9420500b16fd69589c9ba91e.

The referenced memory leak doesn't exist on the 4.14 stable branch as
the new logic of doing the kzalloc hasn't moved to this function.
By adding this kfree we actually end up doing double kfree as all callers of
smi_add does a kfree on error.

Sample with SLAB_FREELIST_HARDENED=y:

ipmi_si: Adding ACPI-specified kcs state machine
IPMI System Interface driver.
ipmi_si: probing via SPMI
ipmi_si: SPMI: io 0xca2 regsize 1 spacing 1 irq 0
(NULL device *): SPMI-specified kcs state machine: duplicate
------------[ cut here ]------------
kernel BUG at mm/slub.c:295!
invalid opcode: 0000 [#1] SMP
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.14.8-gentoo-r1 #5
Hardware name: Supermicro X9SCL/X9SCM/X9SCL/X9SCM, BIOS 2.2 02/20/2015
task: ffff88080c208000 task.stack: ffffc90000020000
RIP: 0010:kfree+0xf5/0x157
RSP: 0000:ffffc90000023e58 EFLAGS: 00010246
RAX: ffff88080b2e6200 RBX: ffff88080b2e6200 RCX: ffff88080b2e6200
RDX: 000000000000008e RSI: ffff88082fc1cd60 RDI: ffff88080c003080
RBP: ffffc90000002808 R08: 000000000001cd60 R09: ffffffff814da10e
R10: ffffea00202cb980 R11: 000000000000005c R12: ffffffff814da10e
R13: 00000000ffffffed R14: ffffffff82317bd0 R15: 0000000000000003
FS:  0000000000000000(0000) GS:ffff88082fc00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 0000000002e09001 CR4: 00000000001606f0
Call Trace:
 init_ipmi_si+0x493/0x5c7
 ? cleanup_ipmi_si+0x84/0x84
 ? set_debug_rodata+0xc/0xc
 ? kthread+0x4c/0x11c
 do_one_initcall+0x94/0x13d
 ? set_debug_rodata+0xc/0xc
 kernel_init_freeable+0x112/0x18e
 ? rest_init+0xa0/0xa0
 kernel_init+0x5/0xe1
 ret_from_fork+0x22/0x30
Code: 24 18 49 8b 7a 30 48 8b 37 65 48 8b 56 08 65 48 03 35 3a 29 e2 7e 4c 3b 56 10 75 39 48 8b 0e 48 63 47 20 48 01 d8 48 39 cb 75 02 <0f> 0b 49 89 c0 4c 33
 87 40 01 00 00 4c 31 c1 48 89 08 48 8d 4a
---[ end trace 4ac2e2c100842676 ]---

Signed-off-by: John Einar Reitan <john.einar@gmail.com>
---
 drivers/char/ipmi/ipmi_si_intf.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index e1cbb78c6806..c04aa11f0e21 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -3469,7 +3469,6 @@ static int add_smi(struct smi_info *new_smi)
 				 ipmi_addr_src_to_str(new_smi->addr_source),
 				 si_to_str[new_smi->si_type]);
 			rv = -EBUSY;
-			kfree(new_smi);
 			goto out_err;
 		}
 	}
-- 
2.13.6