netdev - RE: [PATCH net-next 01/15] devlink: add value check to devlink_info_version

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <DS0PR11MB7785C2BC22AE770A31D7427AF0B52@DS0PR11MB7785.namprd11.prod.outlook.com>
Date: Tue, 8 Apr 2025 11:00:55 +0000
From: "Jagielski, Jedrzej" <jedrzej.jagielski@...el.com>
To: "Nelson, Shannon" <shannon.nelson@....com>, "Nguyen, Anthony L"
	<anthony.l.nguyen@...el.com>, "davem@...emloft.net" <davem@...emloft.net>,
	"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"Dumazet, Eric" <edumazet@...gle.com>, "andrew+netdev@...n.ch"
	<andrew+netdev@...n.ch>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC: "Kitszel, Przemyslaw" <przemyslaw.kitszel@...el.com>, "jiri@...nulli.us"
	<jiri@...nulli.us>, "horms@...nel.org" <horms@...nel.org>, "corbet@....net"
	<corbet@....net>, "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	Kalesh AP <kalesh-anakkur.purayil@...adcom.com>, "R, Bharath"
	<bharath.r@...el.com>
Subject: RE: [PATCH net-next 01/15] devlink: add value check to
 devlink_info_version_put()

From: Nelson, Shannon <shannon.nelson@....com> 
Sent: Tuesday, April 8, 2025 2:31 AM

>On 4/7/2025 2:51 PM, Tony Nguyen wrote:
>> From: Jedrzej Jagielski <jedrzej.jagielski@...el.com>
>> 
>> Prevent from proceeding if there's nothing to print.
>> 
>> Suggested-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
>> Reviewed-by: Jiri Pirko <jiri@...dia.com>
>> Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@...adcom.com>
>> Tested-by: Bharath R <bharath.r@...el.com>
>> Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@...el.com>
>> Signed-off-by: Tony Nguyen <anthony.l.nguyen@...el.com>
>> ---
>>   net/devlink/dev.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/net/devlink/dev.c b/net/devlink/dev.c
>> index d6e3db300acb..02602704bdea 100644
>> --- a/net/devlink/dev.c
>> +++ b/net/devlink/dev.c
>> @@ -775,7 +775,7 @@ static int devlink_info_version_put(struct devlink_info_req *req, int attr,
>>                  req->version_cb(version_name, version_type,
>>                                  req->version_cb_priv);
>> 
>> -       if (!req->msg)
>> +       if (!req->msg || !*version_value)
>
>Personally, I'd like to know that the value was blank if there was 
>normally a value to be printed.  This is removing a useful indicator of 
>something that might be wrong.
>
>sln


Actually this still works the same - when there is no entry that means
that the input was blank, so it still gives you some message.
From my standpoint that's some sort of nice-to-have preventing from printing
the data which has not been inited which most likely is not intentional and
doesn't look good imho.

>
>>                  return 0;
>> 
>>          nest = nla_nest_start_noflag(req->msg, attr);
>> --
>> 2.47.1
>> 
>>