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=-6.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 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 2ACD2C433E0 for ; Mon, 21 Dec 2020 15:27:22 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id DCA7622C97 for ; Mon, 21 Dec 2020 15:27:21 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DCA7622C97 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=lechnology.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=PSk6hZsgRcmzfTypzVCo+lXiDutqyh4cdiiiKHokjsQ=; b=q6LQWuRFZCkArODV/O5T+NKJT fOcokHyhz2kmi8EfXu+4ULugTs6tLqC5JllnRjS1iyEeHQuHI1Vvsby6orUZ+qmxRm/OtQYTxRVD9 cMoG4/OWBoWdFau34ifj7EDPJChKx8mDjoWgy6ddVRm/voLhN7qBxrxvWFgihNWhv+yA6zhfggBya fnKZYLok5ce9MltVjKpQoORYUr4Pntb3OJHpt/rIBwcRAFLKHdil/27x2ZyQwCs+5oOy7sAhKqw68 +KgoIop5Ri42GbzzD1NDbYZI8Jer+tovc/qtn+iNTAsWADGL4qzzZX+WIV6NDDU3hPsxap/xfWGsU qkss8Jmew==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1krN4b-0004qW-7N; Mon, 21 Dec 2020 15:26:09 +0000 Received: from vern.gendns.com ([98.142.107.122]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1krN4Z-0004q9-3o for linux-arm-kernel@lists.infradead.org; Mon, 21 Dec 2020 15:26:07 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lechnology.com; s=default; h=Content-Transfer-Encoding:Content-Type: In-Reply-To:MIME-Version:Date:Message-ID:From:References:Cc:To:Subject:Sender :Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help: List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=Z6HZk7Rcqf/SkkBth/RgoLSQOT9bXljXp/uxtQNS6ns=; b=VS2TDpRcdGQLF321oOjoUZ4XUk kLC+WLwnckx544GfvwqbtUK+XbjuQD2Lqg5ClmYKa8a8abcE1vHT6ubwCMd1D3Vtjsbrz3MCgd9gm OaG3jo514MZvUqVxx8JJ4OVzkptKFnrRlb0dGjgN5Nq1bY8mBvhbatRaCrLgXFwVEwJIy+aeexuD5 22oINfR3rFXfUIWpDKPZQlnvyn4yNHbPiPd10PQhXVaDzQn/1PaXP5ZpWfvciRuPMBbshKcRnWg+F kpRztksPyt0aoDYDRLcTJ3iZMbFpwqDS5a3FQEDGux6D63+1FKjyK8rKuX4P6qxbyHdCAspd254a6 l6DSNWJQ==; Received: from 108-198-5-147.lightspeed.okcbok.sbcglobal.net ([108.198.5.147]:51348 helo=[192.168.0.134]) by vern.gendns.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.93) (envelope-from ) id 1krN4V-0006ij-Sq; Mon, 21 Dec 2020 10:26:03 -0500 Subject: Re: [PATCH v6 1/5] counter: Internalize sysfs interface code To: William Breathitt Gray References: <950660d49af7d12b09bc9d3b1db6f8ff74209c26.1606075915.git.vilhelm.gray@gmail.com> <9fe4090e-2780-31b8-8ffa-2c665c6a2a4e@lechnology.com> From: David Lechner Message-ID: <11343644-dd98-37db-0480-3b8e9c88c631@lechnology.com> Date: Mon, 21 Dec 2020 09:26:02 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - vern.gendns.com X-AntiAbuse: Original Domain - lists.infradead.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - lechnology.com X-Get-Message-Sender-Via: vern.gendns.com: authenticated_id: davidmain+lechnology.com/only user confirmed/virtual account not confirmed X-Authenticated-Sender: vern.gendns.com: davidmain@lechnology.com X-Source: X-Source-Args: X-Source-Dir: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201221_102607_212699_2090E68A X-CRM114-Status: GOOD ( 22.50 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kamel.bouhara@bootlin.com, gwendal@chromium.org, a.fatoum@pengutronix.de, mcoquelin.stm32@gmail.com, linux-iio@vger.kernel.org, patrick.havelange@essensium.com, alexandre.belloni@bootlin.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kernel@pengutronix.de, Dan Carpenter , fabrice.gasnier@st.com, syednwaris@gmail.com, linux-stm32@st-md-mailman.stormreply.com, jic23@kernel.org, alexandre.torgue@st.com Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 12/20/20 4:11 PM, William Breathitt Gray wrote: > On Sun, Dec 13, 2020 at 05:15:00PM -0600, David Lechner wrote: >> On 11/22/20 2:29 PM, William Breathitt Gray wrote: >> >>> 14 files changed, 1806 insertions(+), 2546 deletions(-) >> >> It would be really nice if we could break this down into smaller >> pieces and start getting it merged. It is really tough to keep >> reviewing this much code in one patch over and over again. > > Yes, this is a pretty massive patch. I could break this across the > individual files affected to make it simpler to review, but in the end > all those patches would need to end up squashed together before merge > again (for the sake of git bisect), so the effort feels somewhat moot. > > Luckily, I don't think there will be much change in the next revision > since it's looking like it'll mainly be a few bug fixes; hopefully this > coming version 7 will be the final revision before merge. > >> Here are some initial findings from testing: >> >> >>> +static void counter_device_release(struct device *dev) >>> +{ >>> + struct counter_device *const counter = dev_get_drvdata(dev); >>> + >>> + counter_chrdev_remove(counter); >>> + ida_simple_remove(&counter_ida, counter->id); >>> +} >> >> >> I got the following error after `modprobe -r ti-eqep`: >> >> [ 1186.045766] ------------[ cut here ]------------ >> [ 1186.050647] WARNING: CPU: 0 PID: 2625 at lib/refcount.c:28 counter_device_release+0x10/0x24 [counter] >> [ 1186.059976] refcount_t: underflow; use-after-free. >> [ 1186.064790] Modules linked in: aes_arm_bs(+) crypto_simd cryptd ccm usb_f_mass_storage usb_f_acm u_serial usb_f_ecm rfcomm usb_f_rndis u_ether libcomposite aes_arm aes_generic cmac bnep wl18xx wlcore mac80211 libarc4 sha256_generic libsha256 sha256_arm cfg80211 ti_am335x_adc kfifo_buf omap_aes_driver omap_crypto omap_sham crypto_engine pm33xx ti_emif_sram hci_uart omap_rng btbcm rng_core ti_eqep(-) counter bluetooth c_can_platform c_can ecdh_generic bmp280_spi ecc can_dev libaes bmp280_i2c bmp280 industrialio omap_mailbox musb_dsps wlcore_sdio musb_hdrc udc_core usbcore wkup_m3_ipc at24 omap_wdt phy_am335x watchdog phy_am335x_control ti_am335x_tscadc phy_generic wkup_m3_rproc usb_common cppi41 rtc_omap leds_gpio led_class cpufreq_dt pwm_tiehrpwm autofs4 >> [ 1186.132376] CPU: 0 PID: 2625 Comm: modprobe Not tainted 5.10.0-rc7bone-counter+ #23 >> [ 1186.140070] Hardware name: Generic AM33XX (Flattened Device Tree) >> [ 1186.146225] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) >> [ 1186.154017] [] (show_stack) from [] (dump_stack+0xc4/0xe4) >> [ 1186.161285] [] (dump_stack) from [] (__warn+0xd8/0x100) >> [ 1186.168284] [] (__warn) from [] (warn_slowpath_fmt+0x94/0xbc) >> [ 1186.175814] [] (warn_slowpath_fmt) from [] (counter_device_release+0x10/0x24 [counter]) >> [ 1186.185632] [] (counter_device_release [counter]) from [] (device_release+0x30/0xa4) >> [ 1186.195163] [] (device_release) from [] (kobject_put+0x94/0x104) >> [ 1186.202944] [] (kobject_put) from [] (kobject_put+0x94/0x104) >> [ 1186.210472] [] (kobject_put) from [] (ti_eqep_remove+0x10/0x30 [ti_eqep]) >> [ 1186.219047] [] (ti_eqep_remove [ti_eqep]) from [] (platform_drv_remove+0x24/0x3c) >> [ 1186.228313] [] (platform_drv_remove) from [] (device_release_driver_internal+0xfc/0x1d0) >> [ 1186.238187] [] (device_release_driver_internal) from [] (driver_detach+0x58/0xa8) >> [ 1186.247456] [] (driver_detach) from [] (bus_remove_driver+0x4c/0xa0) >> [ 1186.255594] [] (bus_remove_driver) from [] (sys_delete_module+0x180/0x264) >> [ 1186.264250] [] (sys_delete_module) from [] (ret_fast_syscall+0x0/0x54) >> [ 1186.272551] Exception stack(0xd247ffa8 to 0xd247fff0) >> [ 1186.277629] ffa0: 004fb478 004fb478 004fb4b4 00000800 b3bfcf00 00000000 >> [ 1186.285847] ffc0: 004fb478 004fb478 004fb478 00000081 00000000 be974900 be974a55 004fb478 >> [ 1186.294062] ffe0: 004f8f5c be97352c 004ddd97 b6d11d68 >> [ 1186.299253] ---[ end trace e1c61dea091f1078 ]--- > > I noticed that I'm calling counter_chrdev_remove() twice: once in > counter_unregister(), and again in counter_device_release(). I suspect > this is what's causing the refcount to underflow. I'll test and verify > that this is the culprit. > > In fact, I don't think I need to define a counter_device_release() > callback at all, would I? These cleanup function calls could be moved to > counter_unregister() instead. As long as a user program keeps a chrdev open, it holds a reference to the device, so I think it needs to be the other way around. (Unless it is impossible to call counter_unregister() before all references have been released - but I don't think that is the case - not 100% sure.) _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel