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 1A99CC4332F for ; Sun, 3 Apr 2022 18:27:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1353574AbiDCS2w (ORCPT ); Sun, 3 Apr 2022 14:28:52 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54104 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1359739AbiDCS2v (ORCPT ); Sun, 3 Apr 2022 14:28:51 -0400 Received: from mail-wr1-x42e.google.com (mail-wr1-x42e.google.com [IPv6:2a00:1450:4864:20::42e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2F28839168 for ; Sun, 3 Apr 2022 11:26:55 -0700 (PDT) Received: by mail-wr1-x42e.google.com with SMTP id w4so11312370wrg.12 for ; Sun, 03 Apr 2022 11:26:55 -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=8RrZNQo5Lb6imCjSfJ8rtvqeIbeVOjraIw0CJGdQLTt9EjjcfdK+PBN4M6DKD+t5ON DunsRUkfQyWV/nXH6W2z6hdbJesJhq7KEt+fKZMrNKH1IuzuI3LxzgAGlodD8T/uoUkn 6EK80wbVe9Im3L6RtnCth+aJK39q3i5diaeKazVGfRTSQoKwKgREPc8Q9cBT7w1kH+mp pwCx0cB6KWS6pP4VDNlQwJYA1nZVEfEjMT1GXHXT+PoPFRIzm0JpZB3S+yXEXQzgaOCy cW4fjXvPNsojzoYxAv/taR8JzenpoZ1EzI/f/fcTUdLCnRnPI38swCFkWra/qZKzGfCv iXAA== X-Gm-Message-State: AOAM530k/p+nhb4/LmyXhyajqZ1enXX4ALn/EUpc38MDQXKBD4S1nKhv NNdSb5xD+CZNlN2XIj6ktTMnBQ== 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-spi@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