From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: EDAC: Fix memory leak in creating CSROW object From: Greg Kroah-Hartman Message-Id: <20190427214925.GE16338@kroah.com> Date: Sat, 27 Apr 2019 23:49:25 +0200 To: Borislav Petkov Cc: PanBian , Mauro Carvalho Chehab , James Morse , linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org List-ID: T24gRnJpLCBBcHIgMTksIDIwMTkgYXQgMDI6NDU6MTZBTSArMDIwMCwgQm9yaXNsYXYgUGV0a292 IHdyb3RlOgo+IE9uIEZyaSwgQXByIDE5LCAyMDE5IGF0IDA4OjM1OjM2QU0gKzA4MDAsIFBhbkJp YW4gd3JvdGU6Cj4gPiBZZXMsIEkgc2VlIHRoYXQuIEJlY2F1c2UgdGhlIGxvb3Agc3RhcnQgd2l0 aCAoLS1pKSwgdGhlcmUgaXMgbm8gcHV0Cj4gPiBvcGVyYXRpb24gZm9yIHRoZSBkZXZpY2UgdGhh dCBmYWlscyB0byBjcmVhdGUuIFNvLCBJIHRoaW5rIHdlIGNhbm5vdAo+ID4gcnVsZSBvdXQgdGhl IHBvc3NpYmlsaXR5IG9mIG1lbW9yeSBsZWFrLgo+IAo+IE9rLCBzbyB0aGlzIGlzIG5vdCBzb21l dGhpbmcgeW91IHRyaWdnZXIgLSB5b3UncmUgYmFzaWNhbGx5IHN0YXJpbmcgYXQKPiB0aGUgY29k ZT8KPiAKPiBXZWxsLCB0aGVyZSdzIHNvbWV0aGluZyBlbHNlIHF1ZXN0aW9uYWJsZSBpbiB0aGF0 IGNvZGUgd2hpY2ggSSBhc2tlZAo+IEdyZWcgYWJvdXQgdG9kYXkgYnV0IHdlIGRpZG4ndCBmaW5p c2ggdGhhdCBjb252ZXJzYXRpb24sIGxldCBtZSBDQyBoaW0uCj4gCj4gU28gQUZBSVUsIGRldmlj ZXMgZm9yIHdoaWNoIGRldmljZV9hZGQoKSBoYXMgcmV0dXJuZWQgc3VjY2VzcywKPiBzaG91bGQg YmUgcmVtb3ZlZCB3aXRoIHRoZWlyIGNvdW50ZXJwYXJ0IGRldmljZV9kZWwoKS4KPiBlZGFjX2Ny ZWF0ZV9jc3Jvd19vYmplY3RzKCksIGhvd2V2ZXIsIGRvZXMgcHV0X2RldmljZSgpIG9uIHRob3Nl IGluIHRoZQo+ICJ1bndpbmRpbmciIGxvb3AuCj4gCj4gQW5kIGZvciB0aGUgY2FzZSB3aGVyZSBk ZXZpY2VfYWRkKCkgZmFpbHMsIHlvdSBzaG91bGQgZG8gcHV0X2RldmljZSgpIHRvCj4gaXQuIEku ZS4sIHdoYXQgeW91J3JlIHNheWluZy4KPiAKPiBTbyBJIHRoaW5rIHdlIG5lZWQgdG8gZmlndXJl IHdoYXQgbmVlZHMgdG8gYmUgZG9uZSB3aGVuIGJlZm9yZSBmaXhpbmcKPiB0aGlzIHByb3Blcmx5 Lgo+IAo+IEdyZWc/CgpIb3cgYWJvdXQgdGhpcyBwYXRjaCwgSSB0aGluayBpdCBmaXhlcyB1cCBl dmVyeXRoaW5nIHlvdSBuZWVkIHRvIGRvCmhlcmUsIHJpZ2h0PwoKZGlmZiAtLWdpdCBhL2RyaXZl cnMvZWRhYy9lZGFjX21jX3N5c2ZzLmMgYi9kcml2ZXJzL2VkYWMvZWRhY19tY19zeXNmcy5jCmlu ZGV4IDQ2NDE3NDY4NTU4OS4uMGZiMmQxZGU2ZDBlIDEwMDY0NAotLS0gYS9kcml2ZXJzL2VkYWMv ZWRhY19tY19zeXNmcy5jCisrKyBiL2RyaXZlcnMvZWRhYy9lZGFjX21jX3N5c2ZzLmMKQEAgLTQw NCw2ICs0MDQsOCBAQCBzdGF0aWMgaW5saW5lIGludCBucl9wYWdlc19wZXJfY3Nyb3coc3RydWN0 IGNzcm93X2luZm8gKmNzcm93KQogc3RhdGljIGludCBlZGFjX2NyZWF0ZV9jc3Jvd19vYmplY3Qo c3RydWN0IG1lbV9jdGxfaW5mbyAqbWNpLAogCQkJCSAgICBzdHJ1Y3QgY3Nyb3dfaW5mbyAqY3Ny b3csIGludCBpbmRleCkKIHsKKwlpbnQgcmV0dmFsOworCiAJY3Nyb3ctPmRldi50eXBlID0gJmNz cm93X2F0dHJfdHlwZTsKIAljc3Jvdy0+ZGV2Lmdyb3VwcyA9IGNzcm93X2Rldl9ncm91cHM7CiAJ ZGV2aWNlX2luaXRpYWxpemUoJmNzcm93LT5kZXYpOwpAQCAtNDE1LDcgKzQxNywxMCBAQCBzdGF0 aWMgaW50IGVkYWNfY3JlYXRlX2Nzcm93X29iamVjdChzdHJ1Y3QgbWVtX2N0bF9pbmZvICptY2ks CiAJZWRhY19kYmcoMCwgImNyZWF0aW5nICh2aXJ0dWFsKSBjc3JvdyBub2RlICVzXG4iLAogCQkg ZGV2X25hbWUoJmNzcm93LT5kZXYpKTsKIAotCXJldHVybiBkZXZpY2VfYWRkKCZjc3Jvdy0+ZGV2 KTsKKwlyZXR2YWwgPSBkZXZpY2VfYWRkKCZjc3Jvdy0+ZGV2KTsKKwlpZiAocmV0dmFsKQorCQlw dXRfZGV2aWNlKCZjc3Jvdy0+ZGV2KTsKKwlyZXR1cm4gcmV0dmFsOwogfQogCiAvKiBDcmVhdGUg YSBDU1JPVyBvYmplY3QgdW5kZXIgc3BlY2lmZWQgZWRhY19tY19kZXZpY2UgKi8KQEAgLTY0OSw2 ICs2NTQsOCBAQCBzdGF0aWMgaW50IGVkYWNfY3JlYXRlX2RpbW1fb2JqZWN0KHN0cnVjdCBtZW1f Y3RsX2luZm8gKm1jaSwKIAogCWVkYWNfZGJnKDAsICJjcmVhdGluZyByYW5rL2RpbW0gZGV2aWNl ICVzXG4iLCBkZXZfbmFtZSgmZGltbS0+ZGV2KSk7CiAKKwlpZiAoZXJyKQorCQlwdXRfZGV2aWNl KCZkaW1tLT5kZXYpOwogCXJldHVybiBlcnI7CiB9CiAKQEAgLTkyOCw2ICs5MzUsNyBAQCBpbnQg ZWRhY19jcmVhdGVfc3lzZnNfbWNpX2RldmljZShzdHJ1Y3QgbWVtX2N0bF9pbmZvICptY2ksCiAJ ZXJyID0gZGV2aWNlX2FkZCgmbWNpLT5kZXYpOwogCWlmIChlcnIgPCAwKSB7CiAJCWVkYWNfZGJn KDEsICJmYWlsdXJlOiBjcmVhdGUgZGV2aWNlICVzXG4iLCBkZXZfbmFtZSgmbWNpLT5kZXYpKTsK KwkJcHV0X2RldmljZShtY2ktPmRldik7CiAJCWdvdG8gb3V0OwogCX0KIAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.6 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_PASS,USER_AGENT_MUTT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 96102C43218 for ; Sat, 27 Apr 2019 21:49:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 56195208C2 for ; Sat, 27 Apr 2019 21:49:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1556401769; bh=ad4NRf2D1FTn7+5by/ebxnREERSQZJsRoAGBTwdZ7K4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=rJcIEq5eIJIJd/VKq/+Z25xbToRtnGOLMOQZ01MnpMGNVveo0uzoBC+DTzrmkf0D9 rkSFWYYvXWwDzdEOA8oNai0zATt/bFnWWmk6WaDO/vLEXw+0SZOGisfdChlypIeGrx OlTtY1OqllV20C9+aJLoO8Snp9roUv/HCpb91suk= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726150AbfD0Vt2 (ORCPT ); Sat, 27 Apr 2019 17:49:28 -0400 Received: from mail.kernel.org ([198.145.29.99]:54028 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726088AbfD0Vt2 (ORCPT ); Sat, 27 Apr 2019 17:49:28 -0400 Received: from localhost (83-86-89-107.cable.dynamic.v4.ziggo.nl [83.86.89.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id CB39620645; Sat, 27 Apr 2019 21:49:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1556401767; bh=ad4NRf2D1FTn7+5by/ebxnREERSQZJsRoAGBTwdZ7K4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=dq89bU2UJRp3k78iGEbCoJWQNVPkrxfNzNex5TBzfb5CKbmQ8ebcusXSQS/4FEm4g Nwrm1da3E1DtofGW7FdXzw7ncTI09feAE8QR9ENn7DkNLHThOv+THrOGT27WutXD97 O9u7hApR9qV7Ae3sb24mah5W0Rf/BTT//XfZV520= Date: Sat, 27 Apr 2019 23:49:25 +0200 From: Greg KH To: Borislav Petkov Cc: PanBian , Mauro Carvalho Chehab , James Morse , linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: EDAC: Fix memory leak in creating CSROW object Message-ID: <20190427214925.GE16338@kroah.com> References: <1555554438-103953-1-git-send-email-bianpan2016@163.com> <20190418172548.GL27160@zn.tnic> <20190419003536.GA57795@bianpan2016@163.com> <20190419004516.GC559@zn.tnic> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Disposition: inline In-Reply-To: <20190419004516.GC559@zn.tnic> User-Agent: Mutt/1.11.4 (2019-03-13) Sender: linux-edac-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-edac@vger.kernel.org Message-ID: <20190427214925.t5LN2ZRHFAuckYIz1rlFCtmzcrcTff-UXR05JkPkzRc@z> On Fri, Apr 19, 2019 at 02:45:16AM +0200, Borislav Petkov wrote: > On Fri, Apr 19, 2019 at 08:35:36AM +0800, PanBian wrote: > > Yes, I see that. Because the loop start with (--i), there is no put > > operation for the device that fails to create. So, I think we cannot > > rule out the possibility of memory leak. > > Ok, so this is not something you trigger - you're basically staring at > the code? > > Well, there's something else questionable in that code which I asked > Greg about today but we didn't finish that conversation, let me CC him. > > So AFAIU, devices for which device_add() has returned success, > should be removed with their counterpart device_del(). > edac_create_csrow_objects(), however, does put_device() on those in the > "unwinding" loop. > > And for the case where device_add() fails, you should do put_device() to > it. I.e., what you're saying. > > So I think we need to figure what needs to be done when before fixing > this properly. > > Greg? How about this patch, I think it fixes up everything you need to do here, right? diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c index 464174685589..0fb2d1de6d0e 100644 --- a/drivers/edac/edac_mc_sysfs.c +++ b/drivers/edac/edac_mc_sysfs.c @@ -404,6 +404,8 @@ static inline int nr_pages_per_csrow(struct csrow_info *csrow) static int edac_create_csrow_object(struct mem_ctl_info *mci, struct csrow_info *csrow, int index) { + int retval; + csrow->dev.type = &csrow_attr_type; csrow->dev.groups = csrow_dev_groups; device_initialize(&csrow->dev); @@ -415,7 +417,10 @@ static int edac_create_csrow_object(struct mem_ctl_info *mci, edac_dbg(0, "creating (virtual) csrow node %s\n", dev_name(&csrow->dev)); - return device_add(&csrow->dev); + retval = device_add(&csrow->dev); + if (retval) + put_device(&csrow->dev); + return retval; } /* Create a CSROW object under specifed edac_mc_device */ @@ -649,6 +654,8 @@ static int edac_create_dimm_object(struct mem_ctl_info *mci, edac_dbg(0, "creating rank/dimm device %s\n", dev_name(&dimm->dev)); + if (err) + put_device(&dimm->dev); return err; } @@ -928,6 +935,7 @@ int edac_create_sysfs_mci_device(struct mem_ctl_info *mci, err = device_add(&mci->dev); if (err < 0) { edac_dbg(1, "failure: create device %s\n", dev_name(&mci->dev)); + put_device(mci->dev); goto out; }