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=-0.9 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no 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 C1000C433DF for ; Tue, 2 Jun 2020 19:46:37 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 7E301206A2 for ; Tue, 2 Jun 2020 19:46:37 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=ziepe.ca header.i=@ziepe.ca header.b="MejNCIwg" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7E301206A2 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ziepe.ca Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id F350180007; Tue, 2 Jun 2020 15:46:36 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id EBE1C8E0006; Tue, 2 Jun 2020 15:46:36 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DACEF80007; Tue, 2 Jun 2020 15:46:36 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0075.hostedemail.com [216.40.44.75]) by kanga.kvack.org (Postfix) with ESMTP id C1B8D8E0006 for ; Tue, 2 Jun 2020 15:46:36 -0400 (EDT) Received: from smtpin26.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 802BF18ECE for ; Tue, 2 Jun 2020 19:46:36 +0000 (UTC) X-FDA: 76885303992.26.fight58_1c878028c465a Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin26.hostedemail.com (Postfix) with ESMTP id 5D35118025C5A for ; Tue, 2 Jun 2020 19:46:36 +0000 (UTC) X-HE-Tag: fight58_1c878028c465a X-Filterd-Recvd-Size: 5105 Received: from mail-qt1-f194.google.com (mail-qt1-f194.google.com [209.85.160.194]) by imf43.hostedemail.com (Postfix) with ESMTP for ; Tue, 2 Jun 2020 19:46:35 +0000 (UTC) Received: by mail-qt1-f194.google.com with SMTP id y1so11655346qtv.12 for ; Tue, 02 Jun 2020 12:46:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ziepe.ca; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=cDCbUswdOzWPBxtVOkNTOR8EXwqtruy6d+xYE1G/nrs=; b=MejNCIwgWGcd8L2XowGwm3NoBogEtmKYfDuWZoqncV+MARHDUvpzuuHI9oIfOFnoN9 dxhDF+ZfE6GSaT/0KMRiWYW0kI45FW6Df355Rqa+m5TWa7hDoJm8M7K7wgQBtutzHXiS rBy1T40rd4pXrsT7bnff1tVrYHCu3R9x3xXXeIfJJP/cNsh5yyZdOneP1oF26Xw0qjk3 JSkwyNEpo8k5KkuphfemPhDhMRpY4qdF+UyTE4AIh8UMf4pZ3NJ4lX4p9hrqCAgmILmH PTBvt1K9SUIjU9gMAZvkys0PA/VDHMA6wdk/3vtCpQqs14Hs6GbRytOxuG7VtJLq/4CD hMow== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=cDCbUswdOzWPBxtVOkNTOR8EXwqtruy6d+xYE1G/nrs=; b=d61/eTpqkD59H5NCbs2o94SvlJsYX6izBrc1BXgY3VeKXz0GORX7FPkiqXwJOcxAIm yaTLJDDUfSbLRubJxFfi+T/tiZHm8ZJ7tojEdUMI+/BtYDD+ywmxYYa+iqdaWAD5kTrb z18kLLdfuiYDyG9IFV8AQqFF1rux3Uq7/4O7mvtmwZwARiEGzvlqLiLEFeVf4B1mEb/i pYTNM6xcMJcXBOY82Ujn1HV24/Gb8mpT5PQsDflvDqW7wvfVpxaSUCx3nHsxO1dUj+kh 0H61LNKqryga2aZ+6mxjOMD9CcHOnWf3WOIAWOzbpW+l8SIN4/VLJOcvy2de//OnW1IH wjzg== X-Gm-Message-State: AOAM533f8L7/q6rk6khZof0ENSdBVCx42AgVmXtnT1Oh0QdXmOJ8ilB5 QdOHJiR8Hq8rT10+gjfmDKZ5Hw== X-Google-Smtp-Source: ABdhPJw92gFC8+2YBYL/OOKMyCjSItRlsz1/b9Z/IdkcVVcXUTXpgngzFeDgO6+U+XhQjZjf1zq67Q== X-Received: by 2002:ac8:6d1b:: with SMTP id o27mr29064704qtt.102.1591127195246; Tue, 02 Jun 2020 12:46:35 -0700 (PDT) Received: from ziepe.ca (hlfxns017vw-156-34-48-30.dhcp-dynamic.fibreop.ns.bellaliant.net. [156.34.48.30]) by smtp.gmail.com with ESMTPSA id a1sm2418465qkn.87.2020.06.02.12.46.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 02 Jun 2020 12:46:34 -0700 (PDT) Received: from jgg by mlx with local (Exim 4.93) (envelope-from ) id 1jgCrp-000JR5-Qn; Tue, 02 Jun 2020 16:46:33 -0300 Date: Tue, 2 Jun 2020 16:46:33 -0300 From: Jason Gunthorpe To: Matthew Wilcox Cc: Wang Hai , cl@linux.com, penberg@kernel.org, rientjes@google.com, iamjoonsoo.kim@lge.com, akpm@linux-foundation.org, khlebnikov@yandex-team.ru, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Greg Kroah-Hartman Subject: Re: kobject_init_and_add is easy to misuse Message-ID: <20200602194633.GC6578@ziepe.ca> References: <20200602115033.1054-1-wanghai38@huawei.com> <20200602121035.GL19604@bombadil.infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200602121035.GL19604@bombadil.infradead.org> X-Rspamd-Queue-Id: 5D35118025C5A X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam01 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Jun 02, 2020 at 05:10:35AM -0700, Matthew Wilcox wrote: > On Tue, Jun 02, 2020 at 07:50:33PM +0800, Wang Hai wrote: > > syzkaller reports for memory leak when kobject_init_and_add() > > returns an error in the function sysfs_slab_add() [1] > > > > When this happened, the function kobject_put() is not called for the > > corresponding kobject, which potentially leads to memory leak. > > > > This patch fixes the issue by calling kobject_put() even if > > kobject_init_and_add() fails. > > I think this speaks to a deeper problem with kobject_init_and_add() > to most users. This same bug appears in the first three users of > kobject_init_and_add() that I checked -- > arch/ia64/kernel/topology.c > drivers/firmware/dmi-sysfs.c > drivers/firmware/efi/esrt.c > drivers/scsi/iscsi_boot_sysfs.c > > Some do get it right -- > arch/powerpc/kernel/cacheinfo.c > drivers/gpu/drm/ttm/ttm_bo.c > drivers/gpu/drm/ttm/ttm_memory.c > drivers/infiniband/hw/mlx4/sysfs.c > > I'd argue that the current behaviour is wrong, that kobject_init_and_add() > should call kobject_put() if the add fails. There are APIs that auto-free their argument on failure and last times I checked one, about half the tree had a tricky use-after free bug on the error path. > This would need a tree-wide audit. But somebody needs to do that > anyway because based on my random sampling, half of the users > currently get it wrong. IMHO these functions that hide an 'init' inside (eg the switch from kfree to refcount in the error unwind) are tricky. The caller must switch from some kfree goto error unwind to a put goto error unwind, which is very unnatural and strange. I think it is better if the init is near the kalloc and the entire error unwind always uses put. No switching. Jason