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 048DDC433F5 for ; Sun, 3 Apr 2022 18:27:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1359757AbiDCS2w (ORCPT ); Sun, 3 Apr 2022 14:28:52 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54084 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1351823AbiDCS2t (ORCPT ); Sun, 3 Apr 2022 14:28:49 -0400 Received: from mail-wr1-x42a.google.com (mail-wr1-x42a.google.com [IPv6:2a00:1450:4864:20::42a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 21B1C39161 for ; Sun, 3 Apr 2022 11:26:55 -0700 (PDT) Received: by mail-wr1-x42a.google.com with SMTP id v30so3512613wra.8 for ; Sun, 03 Apr 2022 11:26:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=message-id:date:mime-version:user-agent:subject:content-language:to :cc:references:from:in-reply-to:content-transfer-encoding; bh=bdNSHlvxbLgeS5OtknPrJD9scGCCHGJswcOxfjpkj8k=; b=HJPxauClsE0np7DjAvay8A3qNO9yH3dZHFeONBhunsAye2QJjEwG8hC0V131BjTbQw vtyLN7WthuLgwwNhdCY6aF/k7R8txSC/qRMJ8xCwemdVY+KaeRBkvHINcZa7pp8B9n/4 psP1HG8tSSfUDJ2zS0n2xwmmwSuh7hDUpNROy3vYRAbyHR3Q57BU0DgYPwekeLzw8ODZ H+QsFAuIO1JiJhRZbkQKkU+MKR7bpvvFeUkaLl+jN0kxpw3dXUqm7FpniWC9vVy3Mtjb RbhlTctNKmMP8B2cuuCA2Qn5LglXfvS2+fDopX/AruCsvwpY0kuwoNM7/EI3sZhhp1GW 65HA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=bdNSHlvxbLgeS5OtknPrJD9scGCCHGJswcOxfjpkj8k=; b=oi7njJ2zcc3Zd7gqIeQUtpE9+6JySC/kRyPHObj48IGKNMeSZVfk0S6zhgGafVK8uz 7g3joZvc/zoerJ2i8XjCJBT0RnWXwm+vvxG+FT0Pft62GADhrZlpv9nZBdrEjNS7dunR aUWJv/OhNws52ODcVgXTwQv8Q9wBPJKs3KADiXyaTlVx8ATjLsWWG9WOL8SkAax0VBec l37q3lHoLruZrG/nkDv73r68euyGmLQjN6BXxpICPRjrojBuVMIHGyg0MmYkmbbcohpx 3XYpPU/GR6Kge8v52arDu+jKGvYajaIYkEMZNdpcR6NTqfW5GcadPs2X/VQFjIp1aXNE dsRQ== X-Gm-Message-State: AOAM530Q7SJzeUyzw4op7KymqlHLgSzKcKtvbCwuFfomFwBv8KrgJxRO My4K4foO6Lt2GcHHKulTPK3xHA== X-Google-Smtp-Source: ABdhPJxZiOGK2egxD/5OYMyK+G/Qvkwdu5zqnF3HM5t2oE8lWspEjCw8qRMqkAP48HNYh0SvcBxiOQ== X-Received: by 2002:a05:6000:1547:b0:204:213:ae28 with SMTP id 7-20020a056000154700b002040213ae28mr14429429wry.114.1649010413589; Sun, 03 Apr 2022 11:26:53 -0700 (PDT) Received: from [192.168.0.172] (xdsl-188-155-201-27.adslplus.ch. [188.155.201.27]) by smtp.gmail.com with ESMTPSA id f1-20020a1c6a01000000b0038c9f6a3634sm14881579wmc.7.2022.04.03.11.26.51 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 03 Apr 2022 11:26:52 -0700 (PDT) Message-ID: Date: Sun, 3 Apr 2022 20:26:51 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 Subject: Re: [PATCH v5 01/11] driver: platform: Add helper for safer setting of driver_override Content-Language: en-US To: Andy Shevchenko , Krzysztof Kozlowski Cc: Greg Kroah-Hartman , "Rafael J. Wysocki" , Stuart Yoder , "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 , 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 References: <20220316150533.421349-1-krzysztof.kozlowski@canonical.com> <20220316150533.421349-2-krzysztof.kozlowski@canonical.com> From: Krzysztof Kozlowski In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-remoteproc@vger.kernel.org On 16/03/2022 16:54, Andy Shevchenko wrote: > On Wed, Mar 16, 2022 at 5:06 PM Krzysztof Kozlowski > wrote: > > ... > >> +int driver_set_override(struct device *dev, const char **override, >> + const char *s, size_t len) >> +{ >> + const char *new, *old; >> + char *cp; I focused on some other topics, so I did not respond to your questions for some time. > >> + if (!dev || !override || !s) >> + return -EINVAL; > > Sorry, I didn't pay much attention on this. First of all, I would drop > dev checks and simply require that dev should be valid. Do you expect > this can be called when dev is invalid? No, I can skip the check. > I would like to hear if it's > anything but theoretical. Second one, is the !s requirement. Do I > understand correctly that the string must be always present? But then > how we NULify the override? Is it possible? I did not change the convention of this sysfs hook, so removing of override is passing empty string "". Have in mind that his interface is mainly for sysfs, not for other drivers. > Third one is absence of > len check. See below. > >> + /* >> + * 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; > > I would relax this to make sure we can use it if \n is within this limit. Relax in what way? Store more than PAGE_SIZE? This is a user-visible string and the name of a driver. > >> + cp = strnchr(s, len, '\n'); >> + if (cp) >> + len = cp - s; >> + >> + new = kstrndup(s, len, GFP_KERNEL); > > Here is a word about the len check. > >> + if (!new) > > If len == 0, this won't trigger and you have something very > interesting as a result. True, empty string would be set as override. The API says that empty string clears the override, so len==0 should be valid (just like "\n" is ok). > > One way is to use ZERO_PTR_OR_NULL() another is explicitly check for 0 > and issue a (different?) error code. Best regards, Krzysztof 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 76FD2C433EF for ; Sun, 3 Apr 2022 18:28:32 +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:In-Reply-To:From:References:Cc:To: Subject:MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Y171YRlEcfTu8ZbuUCC5vtJCGSLrahfRuUifZARoyxA=; b=he2ln8xm1DsvJM VJNqsoRWxBUQFvDF5i2ZtcrfonTUcVZMhutlFagQElhub3nV5VxF/kqqtj12Cg6OWkfhHa/XcUM8u GKokLZMBOIKfiwtji3+G0HVgZP0h68TOTM+2TUKxsztOO5AZKtbTZpJIG4JxfEECb/mNasIQvRPcu Swt08oA6fHBMF+lT8Cj+KzV31aY07KbVZiqgnLBZ2hIByj//EJkynLVGPZR0Uomz9croUCGt52p0B kpsSet+V70KZvku88SFj9oIIRkfK1/+eHK94A4I7GNjw6D55grO1hk+PxIJkGF7SDNCYYHvqBCqQa T4p4trQRqYKDtKcuDEjw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nb4wK-00C6rE-Hu; Sun, 03 Apr 2022 18:27:04 +0000 Received: from mail-wr1-x42e.google.com ([2a00:1450:4864:20::42e]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nb4wG-00C6pw-G1 for linux-arm-kernel@lists.infradead.org; Sun, 03 Apr 2022 18:27:01 +0000 Received: by mail-wr1-x42e.google.com with SMTP id r13so11330671wrr.9 for ; Sun, 03 Apr 2022 11:26:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=message-id:date:mime-version:user-agent:subject:content-language:to :cc:references:from:in-reply-to:content-transfer-encoding; bh=bdNSHlvxbLgeS5OtknPrJD9scGCCHGJswcOxfjpkj8k=; b=HJPxauClsE0np7DjAvay8A3qNO9yH3dZHFeONBhunsAye2QJjEwG8hC0V131BjTbQw vtyLN7WthuLgwwNhdCY6aF/k7R8txSC/qRMJ8xCwemdVY+KaeRBkvHINcZa7pp8B9n/4 psP1HG8tSSfUDJ2zS0n2xwmmwSuh7hDUpNROy3vYRAbyHR3Q57BU0DgYPwekeLzw8ODZ H+QsFAuIO1JiJhRZbkQKkU+MKR7bpvvFeUkaLl+jN0kxpw3dXUqm7FpniWC9vVy3Mtjb RbhlTctNKmMP8B2cuuCA2Qn5LglXfvS2+fDopX/AruCsvwpY0kuwoNM7/EI3sZhhp1GW 65HA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=bdNSHlvxbLgeS5OtknPrJD9scGCCHGJswcOxfjpkj8k=; b=bSgrF5zPzNwU0OhZhCWTw5M9tHdD/qBJD8xYFJv+HwfvRJKR0ptWjLr9ZSqio3NBd2 IDLtO1x/e12SL2wEqa3G6Le9jR+hR6P3e5SNtX3YHSVbuoCNA1uLki1whBoqUDUjjK+M 9PtwP0kob9yYol+oJe1RRaVJlBNIuXXIx1Ywwp2uo9kapySUcVCWya4SXyRmN4mLob/D h9OXMPJ/1Omu+8yo3tlzTwafJBa7YZr+SMMAprzev2mGDh+Za/iHoMZdwi1p/FCwKh/Q 5p11mcrrDXlOA9NG3exXN9/AmVw/Umah8SStbpu0NMmtqh3KeUPyM1iZmA3YQ9Zi+s8B IVag== X-Gm-Message-State: AOAM533ItE0H81g+g58agBiuRM649m1u/0DKZAGMpH4oSYgsdNVL2Ab5 ehXjykmWlKHsYm0aujW++oOutA== X-Google-Smtp-Source: ABdhPJxZiOGK2egxD/5OYMyK+G/Qvkwdu5zqnF3HM5t2oE8lWspEjCw8qRMqkAP48HNYh0SvcBxiOQ== X-Received: by 2002:a05:6000:1547:b0:204:213:ae28 with SMTP id 7-20020a056000154700b002040213ae28mr14429429wry.114.1649010413589; Sun, 03 Apr 2022 11:26:53 -0700 (PDT) Received: from [192.168.0.172] (xdsl-188-155-201-27.adslplus.ch. [188.155.201.27]) by smtp.gmail.com with ESMTPSA id f1-20020a1c6a01000000b0038c9f6a3634sm14881579wmc.7.2022.04.03.11.26.51 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 03 Apr 2022 11:26:52 -0700 (PDT) Message-ID: Date: Sun, 3 Apr 2022 20:26:51 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 Subject: Re: [PATCH v5 01/11] driver: platform: Add helper for safer setting of driver_override Content-Language: en-US To: Andy Shevchenko , Krzysztof Kozlowski Cc: Greg Kroah-Hartman , "Rafael J. Wysocki" , Stuart Yoder , "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 , 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 References: <20220316150533.421349-1-krzysztof.kozlowski@canonical.com> <20220316150533.421349-2-krzysztof.kozlowski@canonical.com> From: Krzysztof Kozlowski In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220403_112700_625488_52A20B46 X-CRM114-Status: GOOD ( 27.58 ) 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 16/03/2022 16:54, Andy Shevchenko wrote: > On Wed, Mar 16, 2022 at 5:06 PM Krzysztof Kozlowski > wrote: > > ... > >> +int driver_set_override(struct device *dev, const char **override, >> + const char *s, size_t len) >> +{ >> + const char *new, *old; >> + char *cp; I focused on some other topics, so I did not respond to your questions for some time. > >> + if (!dev || !override || !s) >> + return -EINVAL; > > Sorry, I didn't pay much attention on this. First of all, I would drop > dev checks and simply require that dev should be valid. Do you expect > this can be called when dev is invalid? No, I can skip the check. > I would like to hear if it's > anything but theoretical. Second one, is the !s requirement. Do I > understand correctly that the string must be always present? But then > how we NULify the override? Is it possible? I did not change the convention of this sysfs hook, so removing of override is passing empty string "". Have in mind that his interface is mainly for sysfs, not for other drivers. > Third one is absence of > len check. See below. > >> + /* >> + * 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; > > I would relax this to make sure we can use it if \n is within this limit. Relax in what way? Store more than PAGE_SIZE? This is a user-visible string and the name of a driver. > >> + cp = strnchr(s, len, '\n'); >> + if (cp) >> + len = cp - s; >> + >> + new = kstrndup(s, len, GFP_KERNEL); > > Here is a word about the len check. > >> + if (!new) > > If len == 0, this won't trigger and you have something very > interesting as a result. True, empty string would be set as override. The API says that empty string clears the override, so len==0 should be valid (just like "\n" is ok). > > One way is to use ZERO_PTR_OR_NULL() another is explicitly check for 0 > and issue a (different?) error code. Best regards, Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel