Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Message-ID: <cb83107f-f022-86c6-b463-a1eee4936967@intel.com>
Date:   Thu, 26 Oct 2023 11:29:47 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.3.1
Subject: Re: [PATCH 3/3] checkpatch: add ethtool_sprintf rules
Content-Language: en-US
To:     Justin Stitt <justinstitt@google.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Shay Agroskin <shayagr@amazon.com>,
        Arthur Kiyanovski <akiyano@amazon.com>,
        David Arinzon <darinzon@amazon.com>,
        Noam Dagan <ndagan@amazon.com>,
        Saeed Bishara <saeedb@amazon.com>,
        Rasesh Mody <rmody@marvell.com>,
        Sudarsana Kalluru <skalluru@marvell.com>,
        <GR-Linux-NIC-Dev@marvell.com>,
        Dimitris Michailidis <dmichail@fungible.com>,
        Yisen Zhuang <yisen.zhuang@huawei.com>,
        Salil Mehta <salil.mehta@huawei.com>,
        Jesse Brandeburg <jesse.brandeburg@intel.com>,
        Tony Nguyen <anthony.l.nguyen@intel.com>,
        Louis Peens <louis.peens@corigine.com>,
        Shannon Nelson <shannon.nelson@amd.com>,
        "Brett Creeley" <brett.creeley@amd.com>, <drivers@pensando.io>,
        "K. Y. Srinivasan" <kys@microsoft.com>,
        Haiyang Zhang <haiyangz@microsoft.com>,
        Wei Liu <wei.liu@kernel.org>, Dexuan Cui <decui@microsoft.com>,
        Ronak Doshi <doshir@vmware.com>,
        VMware PV-Drivers Reviewers <pv-drivers@vmware.com>,
        Andy Whitcroft <apw@canonical.com>,
        Joe Perches <joe@perches.com>,
        "Dwaipayan Ray" <dwaipayanray1@gmail.com>,
        Lukas Bulwahn <lukas.bulwahn@gmail.com>
CC:     <linux-kernel@vger.kernel.org>, <netdev@vger.kernel.org>,
        Nick Desaulniers <ndesaulniers@google.com>,
        Nathan Chancellor <nathan@kernel.org>,
        Kees Cook <keescook@chromium.org>,
        <intel-wired-lan@lists.osuosl.org>, <oss-drivers@corigine.com>,
        <linux-hyperv@vger.kernel.org>
References: <20231025-ethtool_puts_impl-v1-0-6a53a93d3b72@google.com>
 <20231025-ethtool_puts_impl-v1-3-6a53a93d3b72@google.com>
From:   Przemek Kitszel <przemyslaw.kitszel@intel.com>
In-Reply-To: <20231025-ethtool_puts_impl-v1-3-6a53a93d3b72@google.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?K1FIVk5iY2JNVWExRE0wT1JSUVlZeThFQ0xvMXFGNlFDRHgyajJqM1gyUElJ?=
 =?utf-8?B?eDI4a0tsNVhCcUlpTEpxZmNoMmFqWElCU1pwZ1RtTFY4bnVrYmc2cmw3MGRo?=
 =?utf-8?B?VnoyajdZWWttYlM4YmNyR2ZmdVc5OHpCZkhRL2U4UkJJRWloNk1ZSXJPVSt2?=
 =?utf-8?B?VTVqS3JGQTVUKzVZYkxJaVlLUi85dlFqc3N0ZmFIK1JsdVVORTJKUnJJVXZQ?=
 =?utf-8?B?N0h5T3EzbmNpVGRxTGhCcTJ4eEpmZ1I1M1daRm9KUjFvNGtGTGI2WGluVE5X?=
 =?utf-8?B?YS8zSGFvNXVJVGtBeG5wYTd1bTZ4djhkY08rbmdkRnJLYm01MXRyK1pzTm1O?=
 =?utf-8?B?WFh6MmZwOVJmS0IwZXdCSWZwOExXNzFOS3BrWHpaWCsyODl3QVdwM0pOZlA5?=
 =?utf-8?B?M3hFMVRuWEJxVTI4V3Q3Y3Y3ZFVjNGRuM0NHS0d1c05LU3AwYnRuUnRqWUVh?=
 =?utf-8?B?NDdBR05ZZlFRNHdkZ2dwTTczamh3Wkw4dG1QdnJId3JoTEZTZzdVK0lzeDhZ?=
 =?utf-8?B?R2hNY0lVMXdnV0UrUHdGWXNINE02OTdFZmF5SEt2aDJCRm1HQUhhRHptZDNs?=
 =?utf-8?B?NjVuQ1VTbEdIT1RwMkN2RGNvOVpvNVlHeFF4dmcrRzVjYXJHQVZpSnIvSnds?=
 =?utf-8?B?QytVMlJreHgvZzl6M2xYTUR6STBCMHl6UGNtN2tQaUJSOURmTVEvMDFSdTlu?=
 =?utf-8?B?WjgvMGUrcVJlOTNvY3VIT0FDTWxGbWVjR3JvMlltSVZ6TjFkb1R4dlVJSThu?=
 =?utf-8?B?L0t1OWVHcFNrV2YyMW8vZEFyOWppRjdRQ3Y4MXdxK0VrMlpJL1JQNm1IOGJH?=
 =?utf-8?B?eEZoek1VdjMyc1ozbUp4T1dnUFFLazU3VitUNzBjdDlLM3ViTVczMmZGSmVa?=
 =?utf-8?B?OXBpRGVLQ2ZGc0w5ckVOMFZnVzRQTVJXdUZUZlpvb09qSmE4U0VjMzQzTVZP?=
 =?utf-8?B?bTd0NlFQdThwY2F4bzZDQ2FrV0p6NGw3NGRaRThxRTJnYjJsVG9TMDUzS0Fr?=
 =?utf-8?B?emM2SzhvdEx6MXVFVVRabGhjSEI0bEpsemFDWm15YTNLbnZOc2JrSm16dUR2?=
 =?utf-8?B?cFNUTXZ3b2lYVWJINEVxM3Y1N29nZlVDeGhNV0JGVWcxOWhSTWFKMVNNMXNn?=
 =?utf-8?B?d1h3VjgvbEFVRkJIM2VseWtpZGpDaWpndE5yOEQvSUI1VlJqTVB4N29SYVJI?=
 =?utf-8?B?V0RMQ2xLc0hWVjV4RWRNNnhYTDY5NUtZdUFTaWpTbU1rRVhUaStLck9MNlc3?=
 =?utf-8?B?VG9YMkllUTFvNms2cysvbE5uVjYzc2RpRG1uek83SFV1aWhMRTFEVkdYalJH?=
 =?utf-8?B?UDdDeFRoNVhWZjd2Nng3VWxFMjhlbmxVOGNoRnJOWll5S1k1dUt4V2NpMlVp?=
 =?utf-8?B?aHA4d3dVbTErRis5eXJvY0JSMS9zRWc0RTh2bTJFQk5zQ25ITmtjVUFOOHpr?=
 =?utf-8?B?VEt4bjY2Z2x3VXFYaUh6UFk2UkthODhQNkFHc3oyVy85QlQwcXFiWWdWZ1NY?=
 =?utf-8?B?U0N6M2FRT0FiQnhaSjVpN2VSOXZzdElmOU5MNW1QRUNpNlhDcHd6U3U5MUIv?=
 =?utf-8?B?cjQ3cnN5MS9ZdTcrMHdIWC9yc1dzeThNbE55amxUaXFMaERRdG5EN0pxQU5k?=
 =?utf-8?B?emtiNzhob29HS1NRWk5ZVExET2pQWVJjZyszL2Q4TDdjYU9YTUhRclR3RmI5?=
 =?utf-8?B?SnFLaGpYWXhjREMwN3pwekZSQnZlM25uTU42MTBjU2ZJb0RKY3crWndTNmdr?=
 =?utf-8?B?elYvNUk3SzJFbTNZVHZ3bVhsWjhpNUdPTnVlNGdIN0RrZE9SUndyVjNKQVhH?=
 =?utf-8?B?OU5UL01RSTVweld5WGpFK1E5cHIzRVdnTXQvTzhjdk1xb1ExcTFZSEpIVEk2?=
 =?utf-8?B?Wmpyb0FvMkRSUjBqRURSVW9uZkFWc09pZVU1cWtmZEd2Zjc1QVNkUzY2Uy9S?=
 =?utf-8?B?VzNuSDIrVUVNRHJZRlNVQ1g0a0RtalhXYUo2eHNSb2VVc1ozZlQzVzVrMDZ1?=
 =?utf-8?B?VFNzWDB2T2JHNjBuWVIyTXRpVTB4MmovYWhFN1BZTjZ1VlM2VkpXUmZyZE5P?=
 =?utf-8?B?ZlZ3Q0NUa0t0Vlc3VnQzT0hTQkt4VFVpUFMyTEFWa0ZkTVd3eGUvRHovQmhV?=
 =?utf-8?B?eUFpZUVTc1lnNElSbTVqdzIrTXU5T0FkQkN6SEpBWlVXY2tHTXdOc1ZydzV6?=
 =?utf-8?B?UEE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 80fad5c7-95f7-422a-0096-08dbd60622fc
X-MS-Exchange-CrossTenant-AuthSource: BYAPR11MB3672.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Oct 2023 09:30:05.4387
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: SuqanZeHvsNTs5ZYExbL+fjApm/rpyaw43UO8EnzIKTJc9B3h47sQerX/P1O1doBqJPgacq0rFGVTHQQdLkdf9u8qjRovGbLiHJ5bBtGKVc=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR11MB6880
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.2 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Thu, 26 Oct 2023 02:31:07 -0700 (PDT)

On 10/26/23 01:40, Justin Stitt wrote:
> Add some warnings for using ethtool_sprintf() where a simple
> ethtool_puts() would suffice.
> 
> The two cases are:
> 
> 1) Use ethtool_sprintf() with just two arguments:
> |       ethtool_sprintf(&data, driver[i].name);
> or
> 2) Use ethtool_sprintf() with a standalone "%s" fmt string:
> |       ethtool_sprintf(&data, "%s", driver[i].name);
> 
> The former may cause -Wformat-security warnings while the latter is just
> not preferred. Both are safely in the category of warnings, not errors.
> 
> Signed-off-by: Justin Stitt <justinstitt@google.com>
> ---
>   scripts/checkpatch.pl | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7d16f863edf1..1ba9ce778746 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -7020,6 +7020,19 @@ sub process {
>   			     "Prefer strscpy, strscpy_pad, or __nonstring over strncpy - see: https://github.com/KSPP/linux/issues/90\n" . $herecurr);
>   		}
>   
> +# ethtool_sprintf uses that should likely be ethtool_puts
> +		if (   $line =~ /\bethtool_sprintf\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\)/   ) {

no need for whitespace right after opening parenthesis, same at the end

Does it work for ethtool_sprintf(calls broken
				 into multiple lines)?

BTW, I really like this series!

> +			WARN("ETHTOOL_SPRINTF",
> +			     "Prefer ethtool_puts over ethtool_sprintf with only two arguments" . $herecurr);
> +		}
> +
> +		# use $rawline because $line loses %s via sanitization and thus we can't match against it.
> +		if (   $rawline =~ /\bethtool_sprintf\s*\(\s*$FuncArg\s*,\s*\"\%s\"\s*,\s*$FuncArg\s*\)/   ) {
> +			WARN("ETHTOOL_SPRINTF2",
> +			     "Prefer ethtool_puts over ethtool_sprintf with standalone \"%s\" specifier" . $herecurr);
> +		}
> +
> +
>   # typecasts on min/max could be min_t/max_t
>   		if ($perl_version_ok &&
>   		    defined $stat &&
>