Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Message-ID: <7c48dc5b-40b9-0556-e4d2-6aca78886ded@intel.com>
Date:   Tue, 10 Oct 2023 14:33:49 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.15.1
Subject: Re: [PATCH] igbvf: replace deprecated strncpy with strscpy
Content-Language: en-US
From:   Jesse Brandeburg <jesse.brandeburg@intel.com>
To:     Justin Stitt <justinstitt@google.com>,
        Tony Nguyen <anthony.l.nguyen@intel.com>,
        "David S. Miller" <davem@davemloft.net>,
        "Eric Dumazet" <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>
CC:     <intel-wired-lan@lists.osuosl.org>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <linux-hardening@vger.kernel.org>
References: <20231010-strncpy-drivers-net-ethernet-intel-igbvf-netdev-c-v1-1-69ccfb2c2aa5@google.com>
 <5dc78e2f-62c1-083a-387f-9afabac02007@intel.com>
In-Reply-To: <5dc78e2f-62c1-083a-387f-9afabac02007@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dVcya3V6cTZmTGN3VmdMVnlhanlhMi95WWpWTGxqbW5aSTVFSExaYVRtOE9C?=
 =?utf-8?B?NUJXL2tlS0RzdHF5clZSTGtXV2hTUkRUUXFMRjJ3TTU3MDJMa1NaRDJxUXds?=
 =?utf-8?B?Uk5jOUR5YlJnQzRFOEhGblVhck5NMWw1ZXlRR2t0Uk1GcTNYaHRITmVlVEpW?=
 =?utf-8?B?Rkd0TWhta3ozbDgvM0s3ajdQbUFvdjJYK2k1eHhnY0tocUljTmZMTFR1c1Na?=
 =?utf-8?B?SkNFWElCUC8zeDBITTRvOEtraWVVMjhXeVh2OS9sTFRlQVl6Q3hwN2Zid1Zt?=
 =?utf-8?B?MTlSak9STmZIR05NbVQ5aWhreWlUYUx1TFdpS092ZU83RnhkS3czdm04dVpF?=
 =?utf-8?B?ZWQzUnNrYldBeEpxQk51bGZ0MXFNV29ENm16VlRJM1lGMXRqMmVFVE1GRldW?=
 =?utf-8?B?QjJRaXZiRTVxTG8vQ0JDNTBHVmZMeU5OTmxFT2E3ZFlCTDZFNnNHUWtIZHVa?=
 =?utf-8?B?UTVzSzZuKzFnRThWVHVtbE5kTUZESndjNHNvd3BPOWpBVmpkc01Sdkw0Zmor?=
 =?utf-8?B?eTdZYTcrczRBMFhYdVJOR0kydkI0U24rYTRaMmQ3cDJ1bmZaM1hTWDJXV3Jt?=
 =?utf-8?B?bm1WTWJGOVhYVUo3SnlKYm9VSXR2NG1namFPcWNEeE1XeTVmL0E2cjUzdm44?=
 =?utf-8?B?clJRTmVzN2d5ZlcvVURkWlN2YjhFYU5iekdhWVVoSldqa1JRdHpYT25ESFk3?=
 =?utf-8?B?SlQzS0lDekNrOG14ZXpNK2RBTGF2S1VqeFdMSm5VdjhqNGZYb0IrdkN6MjhO?=
 =?utf-8?B?bXdqK2xPZ2EzNDJmNkFZMklLbVpRV3ZJbXE2ZWkrWkFIdlI3UjNWTzNoNWpt?=
 =?utf-8?B?YzhVbEpVTTBmaFo3UjIveHVnRDVoWFZEMjRpODc3WnkyVVBKUFN2RFhBRHB0?=
 =?utf-8?B?THpkL3EwcGlxd0lwVG5GVGhQTWQvV09XSHUrb2EyWWV1YVpWUTNnRzRCaEZ6?=
 =?utf-8?B?bCtZWU9kNk01MFBwanp5bmt3SER2UEE5WEdMRjQ5c1k1OHFma3pSbEtBVjZl?=
 =?utf-8?B?NkdrUjFiRlVXemxBNE01ZnI5ejh3T3hUQWlqUEY0cDY2SkZxa3lXQlRXZUlV?=
 =?utf-8?B?SnVnSVc1NjRHNnZGYXdtL2VUYTBmUnBrb0IvcGYycFhKWGJIS2NTQWpwT3Vw?=
 =?utf-8?B?RU9GZFAzVUNsQnpjTlNtSUZ3bU1GeXc1cUxzazQvSm4vTEpSaC9rZVZIMFVN?=
 =?utf-8?B?bXVYTHdlS21aRS9pRGEvMVIyRzEyeDJHa1ZVTVRsNG0xd2tOdXVQdkNvT3lL?=
 =?utf-8?B?YUJJSkgyWVN1S2hlNVVKQjA3V1hKZk9SS3ZtWjlERlFXU3JDUG04NXlpRU9y?=
 =?utf-8?B?bWZYMjVlMDltMVo2dGMrWXowTEpCMFN4dUVqdFNseTdvdTZLU2VDcWY2L3Z3?=
 =?utf-8?B?Z1g4V1c4WDNNOTF0dGdYbzA0UFNNUVJxSlp2ZXJjN0Z2N0RseGxKR0tsZ1Bj?=
 =?utf-8?B?U2lGbEJhWVZRcHQzaUY4Yjl3ekt3TCtqM3VuUWJKamVlMlpZRFpQTEpJTnUx?=
 =?utf-8?B?cTE5dHk4TC9WdE9aSWs3KzdTL2l0QlpNVVFRakJmMU5XMFV2ay8vVmwxSDhl?=
 =?utf-8?B?VFMxMGdxM2UvNWhnbDVtbUNmUnBEL3k1aWVMcjJnMFZ3S09SRC9SdzMrUVRx?=
 =?utf-8?B?YTJsRjhjZVFQdjVINWVNZzBOazVyd3Q4M3V0djVxTE53QTdvMHE5SURqZkdX?=
 =?utf-8?B?UDU5UGxvclphTldjb1BzczVLd1kwMTZoRHJ1QmVHWDRtQnpyTE9uZmRDNjRG?=
 =?utf-8?B?cGlWT2JMbG5tNXllbXVSOFFUMEVNTjl0T0g5c0FpQWR1eUoxM0ltb1JLcGlH?=
 =?utf-8?B?YXRpalBpc1B5ZDNBYUJpL2VZWVpra0JSTTU3QyszQVpibVBhdTBVKzIwQVhC?=
 =?utf-8?B?M09MazYwbm9yWWJPNktncDhhYVo1WWF1QTBOYnpEYXFIcFFDZHBiazRjVEJh?=
 =?utf-8?B?ZXAzalIyUVBUb2JLL3JlOThuQ3BISGoyMlVsbW1QUkJKcW05dEdjSVprcUEv?=
 =?utf-8?B?ZERiOExWSXJQM3FFNG1hc01zOGlZc09HYVhzbTV4elhSNmRJTURLcW9kMjRX?=
 =?utf-8?B?N3VML3lhd1kvVDAxVDZBdXZqQnhqNkJ5eEx3M0ZrUWJzcW1KdUFRMU81NW5Q?=
 =?utf-8?B?UmhNSlpWTXVwd3JTNGFXUFR0SDJmREJPc2ZXblpweVBDbDBjNm4wK3BuUlB6?=
 =?utf-8?B?cVE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 697abb8b-027c-4cec-569e-08dbc9d89871
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4914.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Oct 2023 21:33:51.1675
 (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: MxXFWfppmq8q/cCtwG8lIW4reCbsl23JlREhjyQ0s1NdQk9C+kts1zJUy0Si0MuPBRxt62Wu5+hHrCsp+Qf3VKwrDYzq1d/oXgvd5tin3uI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB7709
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.6 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,RCVD_IN_SBL_CSS,SPF_HELO_NONE,SPF_PASS,
	URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on pete.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 (pete.vger.email [0.0.0.0]); Tue, 10 Oct 2023 14:34:07 -0700 (PDT)

On 10/10/2023 2:20 PM, Jesse Brandeburg wrote:
> On 10/10/2023 2:12 PM, Justin Stitt wrote:
>> `strncpy` is deprecated for use on NUL-terminated destination strings
>> [1] and as such we should prefer more robust and less ambiguous string
>> interfaces.
>>
>> We expect netdev->name to be NUL-terminated based on its usage with
>> `strlen` and format strings:
>> |       if (strlen(netdev->name) < (IFNAMSIZ - 5)) {
>> |               sprintf(adapter->tx_ring->name, "%s-tx-0", netdev->name);
>>
>> Moreover, we do not need NUL-padding as netdev is already
>> zero-allocated:
>> |       netdev = alloc_etherdev(sizeof(struct igbvf_adapter));
>> ...
>> alloc_etherdev() -> alloc_etherdev_mq() -> alloc_etherdev_mqs() ->
>> alloc_netdev_mqs() ...
>> |       p = kvzalloc(alloc_size, GFP_KERNEL_ACCOUNT | __GFP_RETRY_MAYFAIL);
>>
>> Considering the above, a suitable replacement is `strscpy` [2] due to
>> the fact that it guarantees NUL-termination on the destination buffer
>> without unnecessarily NUL-padding.
>>
>> Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
>> Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
>> Link: https://github.com/KSPP/linux/issues/90
>> Cc: linux-hardening@vger.kernel.org
>> Signed-off-by: Justin Stitt <justinstitt@google.com>
>> ---
> 
> Thanks Justin for these patches, please make sure you mark the subject
> line as per the netdev rules:
> [PATCH net-next v1] etc etc
> 
> I'd also prefer they came in as part of one series with a good cover
> letter, at the very least for the Intel drivers, and you probably could
> combine any others (netdev) together up to the 15 patch limit.
> 
> Please mention how you found these issues, via automated tool or via
> coccinelle script, manual grepping, etc?
> 
> Thanks,
> Jesse

netdev rules: https://docs.kernel.org/process/maintainer-netdev.html

Also, please see my related series, that will probably conflict with
your changes but the two are making different changes.

https://lore.kernel.org/netdev/20231003183603.3887546-1-jesse.brandeburg@intel.com/