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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9D150C43217 for ; Tue, 15 Mar 2022 18:01:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1350871AbiCOSCZ (ORCPT ); Tue, 15 Mar 2022 14:02:25 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41014 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S243063AbiCOSCY (ORCPT ); Tue, 15 Mar 2022 14:02:24 -0400 Received: from mail-ed1-x52f.google.com (mail-ed1-x52f.google.com [IPv6:2a00:1450:4864:20::52f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A329D58E6E; Tue, 15 Mar 2022 11:01:11 -0700 (PDT) Received: by mail-ed1-x52f.google.com with SMTP id h13so25237638ede.5; Tue, 15 Mar 2022 11:01:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=9egEby/kGDQx/H2AywacJdMEq9UWb27T57iBP6MKY/M=; b=X459zaz8rBpHmz1eWRM/nXKPoCcWwgv93mgo1kUIqKJmmrFSVbFmtlBkSyc7oZbqEA 9iE/wpN2ldzWFyDARevE8q4aWwftUcTWrj/hZGCp5c/nWOErfTWiotj502O2E3wRub8x fAvwgbWwEpPPMZKlHleAIZ5b3NuCDHQaUCu3o6zMz3Pk9oBOWUhxp7FqU8z4OYlvff33 vjwTCIZda48THwgM2za8HjIs/dPh3/+wx8u7uUvYGGau/xbCmqh8QSdYU72RKq3dM+TV 7s/9EfIhX6BorS0N3xa1MT7AdVO4JDq0rPwvec+a1/gkrmCnUuEgVF5WsgvwobwSbCSf ZFuQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=9egEby/kGDQx/H2AywacJdMEq9UWb27T57iBP6MKY/M=; b=XzfM2y1/piwXOHyvUJCRTcLMyV5F25NwyAJ3OpzE3GPeEYCL6U9QtqD23JMPJV0Dlm 7Cd9roX0kyKTR3QEyPNBz7P1CTQQdguanEMw4FSfhmX7UWjfUgLeQiCFa+zIQZPXBRbP fVruSaKpdK9Nj2pQUqrEVlgMh/AZYNyEU8/ABFX10M8ukj2vAlNcIGthtma/HVPMpqFG eqHaWFywtWxx7W7kKn/3gr//Ca7cgl6MKQVOIjdnR0sT7WwYJmwA+p+LxEmrnryMr8zr 3zGqTbm/cgWVzJypTcsv1YoAaYPodndYKENU1w65gMnmadBR4NPRWrr/poSTTz9kAVwH +/lQ== X-Gm-Message-State: AOAM530LkyqLKpknuIcsqFSJWgP9d+kI2fTmX20AJM3dDQJS36PD0aSm M3ETKmWVYbHCuaFa4FQN0K7Dpq3rsDCIzIWhgY0= X-Google-Smtp-Source: ABdhPJz09b9Fm75ixFJtzUGUkS6dUid3EAUCrWF+jfN6zd2zyejpSwybW8/2BkPFf2PjWP6mgqXTNKVgn2eTNF92NTk= X-Received: by 2002:a05:6402:3589:b0:416:7de7:cdde with SMTP id y9-20020a056402358900b004167de7cddemr26440456edc.218.1647367269998; Tue, 15 Mar 2022 11:01:09 -0700 (PDT) MIME-Version: 1.0 References: <20220312132856.65163-1-krzysztof.kozlowski@canonical.com> <20220312132856.65163-2-krzysztof.kozlowski@canonical.com> In-Reply-To: <20220312132856.65163-2-krzysztof.kozlowski@canonical.com> From: Andy Shevchenko Date: Tue, 15 Mar 2022 19:59:56 +0200 Message-ID: Subject: Re: [PATCH v4 01/11] driver: platform: Add helper for safer setting of driver_override To: Krzysztof Kozlowski Cc: Greg Kroah-Hartman , "Rafael J. Wysocki" , Stuart Yoder , Laurentiu Tudor , Abel Vesa , Shawn Guo , Sascha Hauer , Fabio Estevam , "K. Y. Srinivasan" , Haiyang Zhang , Stephen Hemminger , Wei Liu , Dexuan Cui , Bjorn Helgaas , Bjorn Andersson , Mathieu Poirier , Vineeth Vijayan , Peter Oberparleiter , Heiko Carstens , Vasily Gorbik , Alexander Gordeev , Christian Borntraeger , Sven Schnelle , Andy Gross , Srinivas Kandagatla , Mark Brown , "Michael S. Tsirkin" , Jason Wang , Linux Kernel Mailing List , linux-clk , NXP Linux Team , linux-arm Mailing List , Linux on Hyper-V List , linux-pci , linux-remoteproc@vger.kernel.org, linux-s390@vger.kernel.org, linux-arm-msm@vger.kernel.org, ALSA Development Mailing List , linux-spi , virtualization@lists.linux-foundation.org, Linus Torvalds , Rasmus Villemoes Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-remoteproc@vger.kernel.org On Sat, Mar 12, 2022 at 5:16 PM Krzysztof Kozlowski wrote: > > Several core drivers and buses expect that driver_override is a > dynamically allocated memory thus later they can kfree() it. > > However such assumption is not documented, there were in the past and > there are already users setting it to a string literal. This leads to > kfree() of static memory during device release (e.g. in error paths or > during unbind): > > kernel BUG at ../mm/slub.c:3960! > Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM > ... > (kfree) from [] (platform_device_release+0x88/0xb4) > (platform_device_release) from [] (device_release+0x2c/0x90) > (device_release) from [] (kobject_put+0xec/0x20c) > (kobject_put) from [] (exynos5_clk_probe+0x154/0x18c) > (exynos5_clk_probe) from [] (platform_drv_probe+0x6c/0xa4) > (platform_drv_probe) from [] (really_probe+0x280/0x414) > (really_probe) from [] (driver_probe_device+0x78/0x1c4) > (driver_probe_device) from [] (bus_for_each_drv+0x74/0xb8) > (bus_for_each_drv) from [] (__device_attach+0xd4/0x16c) > (__device_attach) from [] (bus_probe_device+0x88/0x90) > (bus_probe_device) from [] (device_add+0x3dc/0x62c) > (device_add) from [] (of_platform_device_create_pdata+0x94/0xbc) > (of_platform_device_create_pdata) from [] (of_platform_bus_create+0x1a8/0x4fc) > (of_platform_bus_create) from [] (of_platform_bus_create+0x20c/0x4fc) > (of_platform_bus_create) from [] (of_platform_populate+0x84/0x118) > (of_platform_populate) from [] (of_platform_default_populate_init+0xa0/0xb8) > (of_platform_default_populate_init) from [] (do_one_initcall+0x8c/0x404) > (do_one_initcall) from [] (kernel_init_freeable+0x3d0/0x4d8) > (kernel_init_freeable) from [] (kernel_init+0x8/0x114) > (kernel_init) from [] (ret_from_fork+0x14/0x20) I believe you may remove these three. > Provide a helper which clearly documents the usage of driver_override. > This will allow later to reuse the helper and reduce amount of the amount > duplicated code. > Convert the platform driver to use new helper and make the a new > driver_override field const char (it is not modified by the core). ... > +/** > + * driver_set_override() - Helper to set or clear driver override. > + * @dev: Device to change > + * @override: Address of string to change (e.g. &device->driver_override); > + * The contents will be freed and hold newly allocated override. > + * @s: NUL terminated string, new driver name to force a match, pass empty NUL-terminated? (44 vs 115 occurrences) > + * string to clear it > + * @len: length of @s > + * > + * Helper to set or clear driver override in a device, intended for the cases > + * when the driver_override field is allocated by driver/bus code. > + * > + * Returns: 0 on success or a negative error code on failure. > + */ > +int driver_set_override(struct device *dev, const char **override, > + const char *s, size_t len) > +{ > + const char *new, *old; > + char *cp; > + > + if (!dev || !override || !s) > + return -EINVAL; > + > + /* > + * The stored value will be used in sysfs show callback (sysfs_emit()), > + * which has a length limit of PAGE_SIZE and adds a trailing newline. > + * Thus we can store one character less to avoid truncation during sysfs > + * show. > + */ > + if (len >= (PAGE_SIZE - 1)) > + return -EINVAL; > + > + new = kstrndup(s, len, GFP_KERNEL); > + if (!new) > + return -ENOMEM; > + > + cp = strchr(new, '\n'); > + if (cp) > + *cp = '\0'; AFAIU you may reduce memory footprint by cp = strnchr(new, len, '\n'); if (cp) len = s - cp; new = kstrndup(...); > + device_lock(dev); > + old = *override; > + if (cp != new) { > + *override = new; > + } else { > + kfree(new); > + *override = NULL; > + } > + device_unlock(dev); > + > + kfree(old); > + > + return 0; > +} -- With Best Regards, Andy Shevchenko 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 Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id C5532C433FE for ; Tue, 15 Mar 2022 18:01:16 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 69C00842EA; Tue, 15 Mar 2022 18:01:16 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 6NHd0_TTCncz; Tue, 15 Mar 2022 18:01:15 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp1.osuosl.org (Postfix) with ESMTPS id E906C842FB; Tue, 15 Mar 2022 18:01:14 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id B16D2C0012; Tue, 15 Mar 2022 18:01:14 +0000 (UTC) Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 34336C000B for ; Tue, 15 Mar 2022 18:01:13 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 1C662416EC for ; Tue, 15 Mar 2022 18:01:13 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Authentication-Results: smtp4.osuosl.org (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id mNRhA6qqRSWF for ; Tue, 15 Mar 2022 18:01:12 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.8.0 Received: from mail-ed1-x52b.google.com (mail-ed1-x52b.google.com [IPv6:2a00:1450:4864:20::52b]) by smtp4.osuosl.org (Postfix) with ESMTPS id 32AE9416EB for ; Tue, 15 Mar 2022 18:01:12 +0000 (UTC) Received: by mail-ed1-x52b.google.com with SMTP id e22so2520357edc.13 for ; Tue, 15 Mar 2022 11:01:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=9egEby/kGDQx/H2AywacJdMEq9UWb27T57iBP6MKY/M=; b=X459zaz8rBpHmz1eWRM/nXKPoCcWwgv93mgo1kUIqKJmmrFSVbFmtlBkSyc7oZbqEA 9iE/wpN2ldzWFyDARevE8q4aWwftUcTWrj/hZGCp5c/nWOErfTWiotj502O2E3wRub8x fAvwgbWwEpPPMZKlHleAIZ5b3NuCDHQaUCu3o6zMz3Pk9oBOWUhxp7FqU8z4OYlvff33 vjwTCIZda48THwgM2za8HjIs/dPh3/+wx8u7uUvYGGau/xbCmqh8QSdYU72RKq3dM+TV 7s/9EfIhX6BorS0N3xa1MT7AdVO4JDq0rPwvec+a1/gkrmCnUuEgVF5WsgvwobwSbCSf ZFuQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=9egEby/kGDQx/H2AywacJdMEq9UWb27T57iBP6MKY/M=; b=cTNxq2BiBBy/uFVZMfSghZzf2+/OSQiNU9fyu7yGGMdZWtNkaA3hvquaE/K+BfI8Qs 6Bl++OHMKnOLJvrZ/3BinGztyn7Yk7USOp1PH/q5MlXCnRuZuwC5piElThsOBTIUs4sN 1+u/uGWULzv8jpQ0qh1KMnqb2/eV5zGV1JROsmDW7ffG2bbnTSOzV59AmhDA1HEyIix/ ADDK7IJ2AjuiJYdPj5rGJGxsPE31C7FF97oXGFb+r0sCSNU/rzlwaq9OE4/RZ2YdIWNC omNG+qJAWPvmC9KG7VTrDahQsfyJiXLkMlvmMW+02f4q+IYDHwFBM2YaXegUkV3f29Su spyg== X-Gm-Message-State: AOAM532XdR3cHrpXFjb/tBTBTq+cI+eMAi9wB3mnsQoUQYGnz5jQrJsR q/bnCrfTfYG3UuWNrUfwnGjStxIC7oP/yBcjSEk= X-Google-Smtp-Source: ABdhPJz09b9Fm75ixFJtzUGUkS6dUid3EAUCrWF+jfN6zd2zyejpSwybW8/2BkPFf2PjWP6mgqXTNKVgn2eTNF92NTk= X-Received: by 2002:a05:6402:3589:b0:416:7de7:cdde with SMTP id y9-20020a056402358900b004167de7cddemr26440456edc.218.1647367269998; Tue, 15 Mar 2022 11:01:09 -0700 (PDT) MIME-Version: 1.0 References: <20220312132856.65163-1-krzysztof.kozlowski@canonical.com> <20220312132856.65163-2-krzysztof.kozlowski@canonical.com> In-Reply-To: <20220312132856.65163-2-krzysztof.kozlowski@canonical.com> From: Andy Shevchenko Date: Tue, 15 Mar 2022 19:59:56 +0200 Message-ID: Subject: Re: [PATCH v4 01/11] driver: platform: Add helper for safer setting of driver_override To: Krzysztof Kozlowski Cc: Linux on Hyper-V List , Stuart Yoder , "Rafael J. Wysocki" , linux-pci , linux-remoteproc@vger.kernel.org, ALSA Development Mailing List , Bjorn Andersson , Srinivas Kandagatla , Vineeth Vijayan , Alexander Gordeev , Fabio Estevam , linux-clk , linux-s390@vger.kernel.org, Wei Liu , Stephen Hemminger , Abel Vesa , "Michael S. Tsirkin" , Dexuan Cui , Linus Torvalds , Andy Gross , NXP Linux Team , Heiko Carstens , Vasily Gorbik , linux-arm-msm@vger.kernel.org, Sascha Hauer , linux-spi , Mark Brown , Rasmus Villemoes , Bjorn Helgaas , virtualization@lists.linux-foundation.org, linux-arm Mailing List , Laurentiu Tudor , Mathieu Poirier , Greg Kroah-Hartman , Haiyang Zhang , Peter Oberparleiter , Linux Kernel Mailing List , Sven Schnelle , Shawn Guo X-BeenThere: virtualization@lists.linux-foundation.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Linux virtualization List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: virtualization-bounces@lists.linux-foundation.org Sender: "Virtualization" On Sat, Mar 12, 2022 at 5:16 PM Krzysztof Kozlowski wrote: > > Several core drivers and buses expect that driver_override is a > dynamically allocated memory thus later they can kfree() it. > > However such assumption is not documented, there were in the past and > there are already users setting it to a string literal. This leads to > kfree() of static memory during device release (e.g. in error paths or > during unbind): > > kernel BUG at ../mm/slub.c:3960! > Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM > ... > (kfree) from [] (platform_device_release+0x88/0xb4) > (platform_device_release) from [] (device_release+0x2c/0x90) > (device_release) from [] (kobject_put+0xec/0x20c) > (kobject_put) from [] (exynos5_clk_probe+0x154/0x18c) > (exynos5_clk_probe) from [] (platform_drv_probe+0x6c/0xa4) > (platform_drv_probe) from [] (really_probe+0x280/0x414) > (really_probe) from [] (driver_probe_device+0x78/0x1c4) > (driver_probe_device) from [] (bus_for_each_drv+0x74/0xb8) > (bus_for_each_drv) from [] (__device_attach+0xd4/0x16c) > (__device_attach) from [] (bus_probe_device+0x88/0x90) > (bus_probe_device) from [] (device_add+0x3dc/0x62c) > (device_add) from [] (of_platform_device_create_pdata+0x94/0xbc) > (of_platform_device_create_pdata) from [] (of_platform_bus_create+0x1a8/0x4fc) > (of_platform_bus_create) from [] (of_platform_bus_create+0x20c/0x4fc) > (of_platform_bus_create) from [] (of_platform_populate+0x84/0x118) > (of_platform_populate) from [] (of_platform_default_populate_init+0xa0/0xb8) > (of_platform_default_populate_init) from [] (do_one_initcall+0x8c/0x404) > (do_one_initcall) from [] (kernel_init_freeable+0x3d0/0x4d8) > (kernel_init_freeable) from [] (kernel_init+0x8/0x114) > (kernel_init) from [] (ret_from_fork+0x14/0x20) I believe you may remove these three. > Provide a helper which clearly documents the usage of driver_override. > This will allow later to reuse the helper and reduce amount of the amount > duplicated code. > Convert the platform driver to use new helper and make the a new > driver_override field const char (it is not modified by the core). ... > +/** > + * driver_set_override() - Helper to set or clear driver override. > + * @dev: Device to change > + * @override: Address of string to change (e.g. &device->driver_override); > + * The contents will be freed and hold newly allocated override. > + * @s: NUL terminated string, new driver name to force a match, pass empty NUL-terminated? (44 vs 115 occurrences) > + * string to clear it > + * @len: length of @s > + * > + * Helper to set or clear driver override in a device, intended for the cases > + * when the driver_override field is allocated by driver/bus code. > + * > + * Returns: 0 on success or a negative error code on failure. > + */ > +int driver_set_override(struct device *dev, const char **override, > + const char *s, size_t len) > +{ > + const char *new, *old; > + char *cp; > + > + if (!dev || !override || !s) > + return -EINVAL; > + > + /* > + * The stored value will be used in sysfs show callback (sysfs_emit()), > + * which has a length limit of PAGE_SIZE and adds a trailing newline. > + * Thus we can store one character less to avoid truncation during sysfs > + * show. > + */ > + if (len >= (PAGE_SIZE - 1)) > + return -EINVAL; > + > + new = kstrndup(s, len, GFP_KERNEL); > + if (!new) > + return -ENOMEM; > + > + cp = strchr(new, '\n'); > + if (cp) > + *cp = '\0'; AFAIU you may reduce memory footprint by cp = strnchr(new, len, '\n'); if (cp) len = s - cp; new = kstrndup(...); > + device_lock(dev); > + old = *override; > + if (cp != new) { > + *override = new; > + } else { > + kfree(new); > + *override = NULL; > + } > + device_unlock(dev); > + > + kfree(old); > + > + return 0; > +} -- With Best Regards, Andy Shevchenko _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization 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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id AB63FC433F5 for ; Tue, 15 Mar 2022 18:02:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=bCm4KbRopGOfBd3DO0VxdLFyDWi//LXR8k8MazBFZa4=; b=irH7fSt9LNvwJF Rfnf3W1/4bW47oexyXjqWKOm1EVLKT5ZRPUcecgjnD1/f9Xsf++lmd/EJTJgnVTtMbM7zt1h2L7wu ybcXFRerX/PzFEDtL2o+ZBsz1Gvf/EM7vgWjMDajTn+kodw6YX0FND7ZSlx/14sK9L3RAqYXFLpGd mHBGe/EwELtJSMzFW2P43QMHgMoXev4n7fChU/DZLGcVjN7hY2+C4Vg/hfnHw0l+EeAQJQxYx1Ckn ZDgUBgQ7pud/xMTJi6+qKvjXZ/pXFN9Wxd0HTslYwZe/s45MZH+snyKgSzOzXuAEUKxoAxTAM9nDr msYRWKjKflVn7g3KQa4g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nUBTv-00A8yT-7i; Tue, 15 Mar 2022 18:01:15 +0000 Received: from mail-ed1-x52f.google.com ([2a00:1450:4864:20::52f]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nUBTr-00A8xk-Ns for linux-arm-kernel@lists.infradead.org; Tue, 15 Mar 2022 18:01:13 +0000 Received: by mail-ed1-x52f.google.com with SMTP id b24so25204232edu.10 for ; Tue, 15 Mar 2022 11:01:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=9egEby/kGDQx/H2AywacJdMEq9UWb27T57iBP6MKY/M=; b=X459zaz8rBpHmz1eWRM/nXKPoCcWwgv93mgo1kUIqKJmmrFSVbFmtlBkSyc7oZbqEA 9iE/wpN2ldzWFyDARevE8q4aWwftUcTWrj/hZGCp5c/nWOErfTWiotj502O2E3wRub8x fAvwgbWwEpPPMZKlHleAIZ5b3NuCDHQaUCu3o6zMz3Pk9oBOWUhxp7FqU8z4OYlvff33 vjwTCIZda48THwgM2za8HjIs/dPh3/+wx8u7uUvYGGau/xbCmqh8QSdYU72RKq3dM+TV 7s/9EfIhX6BorS0N3xa1MT7AdVO4JDq0rPwvec+a1/gkrmCnUuEgVF5WsgvwobwSbCSf ZFuQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=9egEby/kGDQx/H2AywacJdMEq9UWb27T57iBP6MKY/M=; b=wu9vvMd8M5kPdn/wmQR8riBosVyFonur7A4CoQZWxgIe/zoUh8CXFS+t2IG1MYS6Xk ygeB50lIXFRKU3f3fMARkrAFWTyqkKVNBBCHbS5ZqyjLQKMd9Cv+J5syOReCpFdVHa6Z wknor1zLgY4Yd+xnQy2nGk7zRlnq3LlH/n1IL6bXB+7Fh/nnV9CBYrbq8PKbcGMpdro6 5mo6ZnsYzenZukMmms8oFOoaUYAju2g1u1hpZGq/tpcs3BZ19ZdQqdE8IRwVFpv0RAjd WX62IQpbfwav0IawDaUkN7cJwLDoGtZGUJ87Lyh7nRBx2oWi3ELIRinrq7BNaTkvbFK1 N/+w== X-Gm-Message-State: AOAM533HNvhx/c5mgrrb6ND3RDInn4OSqAplMFxQi4nPz8f2qa1fvLKs xHBe2qGqW/W90rvXUdwiJsmchyL647rsc+rDeq0= X-Google-Smtp-Source: ABdhPJz09b9Fm75ixFJtzUGUkS6dUid3EAUCrWF+jfN6zd2zyejpSwybW8/2BkPFf2PjWP6mgqXTNKVgn2eTNF92NTk= X-Received: by 2002:a05:6402:3589:b0:416:7de7:cdde with SMTP id y9-20020a056402358900b004167de7cddemr26440456edc.218.1647367269998; Tue, 15 Mar 2022 11:01:09 -0700 (PDT) MIME-Version: 1.0 References: <20220312132856.65163-1-krzysztof.kozlowski@canonical.com> <20220312132856.65163-2-krzysztof.kozlowski@canonical.com> In-Reply-To: <20220312132856.65163-2-krzysztof.kozlowski@canonical.com> From: Andy Shevchenko Date: Tue, 15 Mar 2022 19:59:56 +0200 Message-ID: Subject: Re: [PATCH v4 01/11] driver: platform: Add helper for safer setting of driver_override To: Krzysztof Kozlowski Cc: Greg Kroah-Hartman , "Rafael J. Wysocki" , Stuart Yoder , Laurentiu Tudor , Abel Vesa , Shawn Guo , Sascha Hauer , Fabio Estevam , "K. Y. Srinivasan" , Haiyang Zhang , Stephen Hemminger , Wei Liu , Dexuan Cui , Bjorn Helgaas , Bjorn Andersson , Mathieu Poirier , Vineeth Vijayan , Peter Oberparleiter , Heiko Carstens , Vasily Gorbik , Alexander Gordeev , Christian Borntraeger , Sven Schnelle , Andy Gross , Srinivas Kandagatla , Mark Brown , "Michael S. Tsirkin" , Jason Wang , Linux Kernel Mailing List , linux-clk , NXP Linux Team , linux-arm Mailing List , Linux on Hyper-V List , linux-pci , linux-remoteproc@vger.kernel.org, linux-s390@vger.kernel.org, linux-arm-msm@vger.kernel.org, ALSA Development Mailing List , linux-spi , virtualization@lists.linux-foundation.org, Linus Torvalds , Rasmus Villemoes X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220315_110111_813508_1785737F X-CRM114-Status: GOOD ( 27.98 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Sat, Mar 12, 2022 at 5:16 PM Krzysztof Kozlowski wrote: > > Several core drivers and buses expect that driver_override is a > dynamically allocated memory thus later they can kfree() it. > > However such assumption is not documented, there were in the past and > there are already users setting it to a string literal. This leads to > kfree() of static memory during device release (e.g. in error paths or > during unbind): > > kernel BUG at ../mm/slub.c:3960! > Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM > ... > (kfree) from [] (platform_device_release+0x88/0xb4) > (platform_device_release) from [] (device_release+0x2c/0x90) > (device_release) from [] (kobject_put+0xec/0x20c) > (kobject_put) from [] (exynos5_clk_probe+0x154/0x18c) > (exynos5_clk_probe) from [] (platform_drv_probe+0x6c/0xa4) > (platform_drv_probe) from [] (really_probe+0x280/0x414) > (really_probe) from [] (driver_probe_device+0x78/0x1c4) > (driver_probe_device) from [] (bus_for_each_drv+0x74/0xb8) > (bus_for_each_drv) from [] (__device_attach+0xd4/0x16c) > (__device_attach) from [] (bus_probe_device+0x88/0x90) > (bus_probe_device) from [] (device_add+0x3dc/0x62c) > (device_add) from [] (of_platform_device_create_pdata+0x94/0xbc) > (of_platform_device_create_pdata) from [] (of_platform_bus_create+0x1a8/0x4fc) > (of_platform_bus_create) from [] (of_platform_bus_create+0x20c/0x4fc) > (of_platform_bus_create) from [] (of_platform_populate+0x84/0x118) > (of_platform_populate) from [] (of_platform_default_populate_init+0xa0/0xb8) > (of_platform_default_populate_init) from [] (do_one_initcall+0x8c/0x404) > (do_one_initcall) from [] (kernel_init_freeable+0x3d0/0x4d8) > (kernel_init_freeable) from [] (kernel_init+0x8/0x114) > (kernel_init) from [] (ret_from_fork+0x14/0x20) I believe you may remove these three. > Provide a helper which clearly documents the usage of driver_override. > This will allow later to reuse the helper and reduce amount of the amount > duplicated code. > Convert the platform driver to use new helper and make the a new > driver_override field const char (it is not modified by the core). ... > +/** > + * driver_set_override() - Helper to set or clear driver override. > + * @dev: Device to change > + * @override: Address of string to change (e.g. &device->driver_override); > + * The contents will be freed and hold newly allocated override. > + * @s: NUL terminated string, new driver name to force a match, pass empty NUL-terminated? (44 vs 115 occurrences) > + * string to clear it > + * @len: length of @s > + * > + * Helper to set or clear driver override in a device, intended for the cases > + * when the driver_override field is allocated by driver/bus code. > + * > + * Returns: 0 on success or a negative error code on failure. > + */ > +int driver_set_override(struct device *dev, const char **override, > + const char *s, size_t len) > +{ > + const char *new, *old; > + char *cp; > + > + if (!dev || !override || !s) > + return -EINVAL; > + > + /* > + * The stored value will be used in sysfs show callback (sysfs_emit()), > + * which has a length limit of PAGE_SIZE and adds a trailing newline. > + * Thus we can store one character less to avoid truncation during sysfs > + * show. > + */ > + if (len >= (PAGE_SIZE - 1)) > + return -EINVAL; > + > + new = kstrndup(s, len, GFP_KERNEL); > + if (!new) > + return -ENOMEM; > + > + cp = strchr(new, '\n'); > + if (cp) > + *cp = '\0'; AFAIU you may reduce memory footprint by cp = strnchr(new, len, '\n'); if (cp) len = s - cp; new = kstrndup(...); > + device_lock(dev); > + old = *override; > + if (cp != new) { > + *override = new; > + } else { > + kfree(new); > + *override = NULL; > + } > + device_unlock(dev); > + > + kfree(old); > + > + return 0; > +} -- With Best Regards, Andy Shevchenko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel 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 Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 67821C433F5 for ; Thu, 17 Mar 2022 06:58:27 +0000 (UTC) Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id A8BB9183B; Thu, 17 Mar 2022 07:57:35 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz A8BB9183B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1647500305; bh=mQkn9o/oTYSn/qPED3CZ6MW7DvfgJAtJBi7yHXxbwBw=; h=References:In-Reply-To:From:Date:Subject:To:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=K7+FLGITUZ1JhCtkCBz7qcdBYeukcJIRqTiauHGZO+mlN0oUjqM2I/52ujeWhHwn1 NUK54Befy3BlFlR7R9r1m+vslIzvvuA0IZp1cElKTWRgvJQbSus7LosWBaZjnDOX+m psxGrFCewLhXxPciMEu18OPPf5NIdKDpg4d68wuY= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id A95DEF8053D; Thu, 17 Mar 2022 07:55:20 +0100 (CET) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 8E43CF80162; Tue, 15 Mar 2022 19:01:18 +0100 (CET) Received: from mail-ed1-x52a.google.com (mail-ed1-x52a.google.com [IPv6:2a00:1450:4864:20::52a]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 054DDF80095 for ; Tue, 15 Mar 2022 19:01:11 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 054DDF80095 Authentication-Results: alsa1.perex.cz; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="X459zaz8" Received: by mail-ed1-x52a.google.com with SMTP id m12so25185936edc.12 for ; Tue, 15 Mar 2022 11:01:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=9egEby/kGDQx/H2AywacJdMEq9UWb27T57iBP6MKY/M=; b=X459zaz8rBpHmz1eWRM/nXKPoCcWwgv93mgo1kUIqKJmmrFSVbFmtlBkSyc7oZbqEA 9iE/wpN2ldzWFyDARevE8q4aWwftUcTWrj/hZGCp5c/nWOErfTWiotj502O2E3wRub8x fAvwgbWwEpPPMZKlHleAIZ5b3NuCDHQaUCu3o6zMz3Pk9oBOWUhxp7FqU8z4OYlvff33 vjwTCIZda48THwgM2za8HjIs/dPh3/+wx8u7uUvYGGau/xbCmqh8QSdYU72RKq3dM+TV 7s/9EfIhX6BorS0N3xa1MT7AdVO4JDq0rPwvec+a1/gkrmCnUuEgVF5WsgvwobwSbCSf ZFuQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=9egEby/kGDQx/H2AywacJdMEq9UWb27T57iBP6MKY/M=; b=R0DaBwqbzx4gPN+dmJCxKpClCZFTCELTsQevtXwPmQ8QcqIMcvbyIi+0rvoGepTO/E 2nv9F2Ee9YnOp5//L9n8UfKBHFvTo3kuJhh6qkb/QrsOi0Z22borr3dHv5cGkxplerhK wZCiBKUU/T4QKEWJ/lukO2SqWY1BWCw1rNQGg8QRu1cxK2bZccwPRR20xwGihoZwtsDB ouWz7qiLKN2x+cfPmNQjZg5FbdttcdoW0ujVLJcDdqJyqD2mv8r4I1TXR3bwarjiYy7b DbxeLXad0Ph4uuYnHn+Q+Iw96GHJDzs1/I3daTfGtdXCziDpJBvBiqpWymbeVfvAAirG grWQ== X-Gm-Message-State: AOAM530y81+exCHxoAV62+Ukevto2sAmmM6aadiZEfyNU1mLiATZhRlW bYiWDcvxCARDNuYkgoWRPaiwvVWHyRwQfXTOFQ0= X-Google-Smtp-Source: ABdhPJz09b9Fm75ixFJtzUGUkS6dUid3EAUCrWF+jfN6zd2zyejpSwybW8/2BkPFf2PjWP6mgqXTNKVgn2eTNF92NTk= X-Received: by 2002:a05:6402:3589:b0:416:7de7:cdde with SMTP id y9-20020a056402358900b004167de7cddemr26440456edc.218.1647367269998; Tue, 15 Mar 2022 11:01:09 -0700 (PDT) MIME-Version: 1.0 References: <20220312132856.65163-1-krzysztof.kozlowski@canonical.com> <20220312132856.65163-2-krzysztof.kozlowski@canonical.com> In-Reply-To: <20220312132856.65163-2-krzysztof.kozlowski@canonical.com> From: Andy Shevchenko Date: Tue, 15 Mar 2022 19:59:56 +0200 Message-ID: Subject: Re: [PATCH v4 01/11] driver: platform: Add helper for safer setting of driver_override To: Krzysztof Kozlowski Content-Type: text/plain; charset="UTF-8" X-Mailman-Approved-At: Thu, 17 Mar 2022 07:55:05 +0100 Cc: Linux on Hyper-V List , Stuart Yoder , "Rafael J. Wysocki" , linux-pci , Jason Wang , linux-remoteproc@vger.kernel.org, ALSA Development Mailing List , Bjorn Andersson , Srinivas Kandagatla , Vineeth Vijayan , Alexander Gordeev , "K. Y. Srinivasan" , Fabio Estevam , linux-clk , linux-s390@vger.kernel.org, Wei Liu , Stephen Hemminger , Abel Vesa , "Michael S. Tsirkin" , Dexuan Cui , Linus Torvalds , Andy Gross , NXP Linux Team , Christian Borntraeger , Heiko Carstens , Vasily Gorbik , linux-arm-msm@vger.kernel.org, Sascha Hauer , linux-spi , Mark Brown , Rasmus Villemoes , Bjorn Helgaas , virtualization@lists.linux-foundation.org, linux-arm Mailing List , Laurentiu Tudor , Mathieu Poirier , Greg Kroah-Hartman , Haiyang Zhang , Peter Oberparleiter , Linux Kernel Mailing List , Sven Schnelle , Shawn Guo X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" On Sat, Mar 12, 2022 at 5:16 PM Krzysztof Kozlowski wrote: > > Several core drivers and buses expect that driver_override is a > dynamically allocated memory thus later they can kfree() it. > > However such assumption is not documented, there were in the past and > there are already users setting it to a string literal. This leads to > kfree() of static memory during device release (e.g. in error paths or > during unbind): > > kernel BUG at ../mm/slub.c:3960! > Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM > ... > (kfree) from [] (platform_device_release+0x88/0xb4) > (platform_device_release) from [] (device_release+0x2c/0x90) > (device_release) from [] (kobject_put+0xec/0x20c) > (kobject_put) from [] (exynos5_clk_probe+0x154/0x18c) > (exynos5_clk_probe) from [] (platform_drv_probe+0x6c/0xa4) > (platform_drv_probe) from [] (really_probe+0x280/0x414) > (really_probe) from [] (driver_probe_device+0x78/0x1c4) > (driver_probe_device) from [] (bus_for_each_drv+0x74/0xb8) > (bus_for_each_drv) from [] (__device_attach+0xd4/0x16c) > (__device_attach) from [] (bus_probe_device+0x88/0x90) > (bus_probe_device) from [] (device_add+0x3dc/0x62c) > (device_add) from [] (of_platform_device_create_pdata+0x94/0xbc) > (of_platform_device_create_pdata) from [] (of_platform_bus_create+0x1a8/0x4fc) > (of_platform_bus_create) from [] (of_platform_bus_create+0x20c/0x4fc) > (of_platform_bus_create) from [] (of_platform_populate+0x84/0x118) > (of_platform_populate) from [] (of_platform_default_populate_init+0xa0/0xb8) > (of_platform_default_populate_init) from [] (do_one_initcall+0x8c/0x404) > (do_one_initcall) from [] (kernel_init_freeable+0x3d0/0x4d8) > (kernel_init_freeable) from [] (kernel_init+0x8/0x114) > (kernel_init) from [] (ret_from_fork+0x14/0x20) I believe you may remove these three. > Provide a helper which clearly documents the usage of driver_override. > This will allow later to reuse the helper and reduce amount of the amount > duplicated code. > Convert the platform driver to use new helper and make the a new > driver_override field const char (it is not modified by the core). ... > +/** > + * driver_set_override() - Helper to set or clear driver override. > + * @dev: Device to change > + * @override: Address of string to change (e.g. &device->driver_override); > + * The contents will be freed and hold newly allocated override. > + * @s: NUL terminated string, new driver name to force a match, pass empty NUL-terminated? (44 vs 115 occurrences) > + * string to clear it > + * @len: length of @s > + * > + * Helper to set or clear driver override in a device, intended for the cases > + * when the driver_override field is allocated by driver/bus code. > + * > + * Returns: 0 on success or a negative error code on failure. > + */ > +int driver_set_override(struct device *dev, const char **override, > + const char *s, size_t len) > +{ > + const char *new, *old; > + char *cp; > + > + if (!dev || !override || !s) > + return -EINVAL; > + > + /* > + * The stored value will be used in sysfs show callback (sysfs_emit()), > + * which has a length limit of PAGE_SIZE and adds a trailing newline. > + * Thus we can store one character less to avoid truncation during sysfs > + * show. > + */ > + if (len >= (PAGE_SIZE - 1)) > + return -EINVAL; > + > + new = kstrndup(s, len, GFP_KERNEL); > + if (!new) > + return -ENOMEM; > + > + cp = strchr(new, '\n'); > + if (cp) > + *cp = '\0'; AFAIU you may reduce memory footprint by cp = strnchr(new, len, '\n'); if (cp) len = s - cp; new = kstrndup(...); > + device_lock(dev); > + old = *override; > + if (cp != new) { > + *override = new; > + } else { > + kfree(new); > + *override = NULL; > + } > + device_unlock(dev); > + > + kfree(old); > + > + return 0; > +} -- With Best Regards, Andy Shevchenko