From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2992980AbcBSOPI (ORCPT ); Fri, 19 Feb 2016 09:15:08 -0500 Received: from vms173015pub.verizon.net ([206.46.173.15]:60066 "EHLO vms173015pub.verizon.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1947080AbcBSOPF (ORCPT ); Fri, 19 Feb 2016 09:15:05 -0500 X-Greylist: delayed 3600 seconds by postgrey-1.27 at vger.kernel.org; Fri, 19 Feb 2016 09:15:05 EST X-CMAE-Score: 0 X-CMAE-Analysis: v=2.1 cv=btqxfxui c=1 sm=1 tr=0 a=a3t/a0oOYlYu/nGe1mf8ZA==:117 a=IkcTkHD0fZMA:10 a=xqWC_Br6kY4A:10 a=jFJIQSaiL_oA:10 a=FOH2dFAWAAAA:8 a=9mBQ3ABwkZ5mlkxz0lUA:9 a=aYYzxaxB1S-ZOrwH:21 a=Y092SsGPaAIE6U_e:21 a=QEXdDO2ut3YA:10 Reply-to: minyard@acm.org Subject: Re: [PATCH][QUESTION] Intentional memory leak in ipmi_msghandler? References: <20160219064132.GA2859126@devbig337.prn1.facebook.com> To: Calvin Owens , openipmi-developer@lists.sourceforge.net Cc: linux-kernel@vger.kernel.org, kernel-team@fb.com From: Corey Minyard Message-id: <56C7154F.3040604@acm.org> Date: Fri, 19 Feb 2016 07:14:55 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-version: 1.0 In-reply-to: <20160219064132.GA2859126@devbig337.prn1.facebook.com> Content-type: text/plain; charset=utf-8; format=flowed Content-transfer-encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 02/19/2016 12:41 AM, Calvin Owens wrote: > Hello, > > I've got a few boxes that are leaking memory in handle_new_recv_msgs() > in ipmi_msghandler. AFAICS this is intentional, there's even an explicit > counter that tracks the number of times smi_msg is leaked. Are you 100% sure about this? There's no intentional leak, a negative return from this function means the message was used for another purpose and thus shouldn't be freed. There's only one situation where this happens and you should never hit it in normal operation. > I'm guessing there was a reason for doing this, but there wasn't any > discussion about it on LKML when the patch was accepted. Can you clarify > why something like the below patch won't work? I tried it on one of my > leaky boxes and nothing obviously horrible happened. Well, that's because nothing probably happened, and it probably had no effect on the leak. A better comment on this code is probably in order. But that patch is incorrect. I doubt the leak is here. If you are having a leak, is it possible to characterize it better? Are you handling commands from IPMB? Are you handling LAN commands here? -corey > > Thanks, > Calvin > > ----8<---- > From: Calvin Owens > Subject: [PATCH] ipmi_msghandler: Don't leak memory on errors > > Signed-off-by: Calvin Owens > --- > drivers/char/ipmi/ipmi_msghandler.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c > index 94fb407..ed82ffa 100644 > --- a/drivers/char/ipmi/ipmi_msghandler.c > +++ b/drivers/char/ipmi/ipmi_msghandler.c > @@ -3834,10 +3834,7 @@ static void handle_new_recv_msgs(ipmi_smi_t intf) > break; > } else { > list_del(&smi_msg->link); > - if (rv == 0) > - /* Message handled */ > - ipmi_free_smi_msg(smi_msg); > - /* If rv < 0, fatal error, del but don't free. */ > + ipmi_free_smi_msg(smi_msg); > } > } > if (!run_to_completion)