netdev - Re: [PATCH v3 net-next 8/8] ionic: drop ethtool driver 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]

Date:   Wed, 4 Mar 2020 23:20:11 -0800
From:   Shannon Nelson <snelson@...sando.io>
To:     Leon Romanovsky <leonro@...lanox.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net
Subject: Re: [PATCH v3 net-next 8/8] ionic: drop ethtool driver version

On 3/4/20 10:10 PM, Leon Romanovsky wrote:
> On Wed, Mar 04, 2020 at 09:23:19PM -0800, Shannon Nelson wrote:
>> Use the default kernel version in ethtool drv_info output
>> and drop the module version.
>>
>> Cc: Leon Romanovsky <leonro@...lanox.com>
>> Signed-off-by: Shannon Nelson <snelson@...sando.io>
>> ---
>>   drivers/net/ethernet/pensando/ionic/ionic.h         | 1 -
>>   drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 1 -
>>   drivers/net/ethernet/pensando/ionic/ionic_main.c    | 6 ++----
>>   3 files changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h
>> index c8ff33da243a..1c720759fd80 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic.h
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic.h
>> @@ -12,7 +12,6 @@ struct ionic_lif;
>>
>>   #define IONIC_DRV_NAME		"ionic"
>>   #define IONIC_DRV_DESCRIPTION	"Pensando Ethernet NIC Driver"
>> -#define IONIC_DRV_VERSION	"0.20.0-k"
>>
>>   #define PCI_VENDOR_ID_PENSANDO			0x1dd8
>>
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
>> index acd53e27d1ec..bea9b78e0189 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
>> @@ -86,7 +86,6 @@ static void ionic_get_drvinfo(struct net_device *netdev,
>>   	struct ionic *ionic = lif->ionic;
>>
>>   	strlcpy(drvinfo->driver, IONIC_DRV_NAME, sizeof(drvinfo->driver));
>> -	strlcpy(drvinfo->version, IONIC_DRV_VERSION, sizeof(drvinfo->version));
>>   	strlcpy(drvinfo->fw_version, ionic->idev.dev_info.fw_version,
>>   		sizeof(drvinfo->fw_version));
>>   	strlcpy(drvinfo->bus_info, ionic_bus_info(ionic),
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
>> index a8e3fb73b465..e4a76e66f542 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
>> @@ -6,6 +6,7 @@
>>   #include <linux/module.h>
>>   #include <linux/netdevice.h>
>>   #include <linux/utsname.h>
>> +#include <linux/vermagic.h>
>>
>>   #include "ionic.h"
>>   #include "ionic_bus.h"
>> @@ -15,7 +16,6 @@
>>   MODULE_DESCRIPTION(IONIC_DRV_DESCRIPTION);
>>   MODULE_AUTHOR("Pensando Systems, Inc");
>>   MODULE_LICENSE("GPL");
>> -MODULE_VERSION(IONIC_DRV_VERSION);
>>
>>   static const char *ionic_error_to_str(enum ionic_status_code code)
>>   {
>> @@ -414,7 +414,7 @@ int ionic_identify(struct ionic *ionic)
>>   	memset(ident, 0, sizeof(*ident));
>>
>>   	ident->drv.os_type = cpu_to_le32(IONIC_OS_TYPE_LINUX);
>> -	strncpy(ident->drv.driver_ver_str, IONIC_DRV_VERSION,
>> +	strncpy(ident->drv.driver_ver_str, UTS_RELEASE,
>>   		sizeof(ident->drv.driver_ver_str) - 1);
> i see that you responded to my question about usage of this string [1]
> and I can't say anything about netdev policy on that, but in other
> subsystems, the idea that driver has duplicated debug functionalities
> to the general kernel code is not welcomed.
>
> [1] https://lore.kernel.org/netdev/e0cbc84c-7860-abf2-a622-4035be1479dc@pensando.io

This DSC (Distributed Services Card) is more than a simple NIC, and in 
several use cases is intended to be managed centrally and installed in 
hosts that can be handed out to customers as bare-metal machines to 
which the datacenter personnel cannot access.  The device can be 
accessed through a separate management network port, similar to an ilom 
or cimc other similar host management gizmo. Getting a little 
information about the driver into the card's logfiles allows for a 
little better debugging context from the management side without having 
access to the host.

Yes, we want to keep functionality duplication to a minimum, but I think 
this is a different case.  We also want to keep customer information 
leakage to a minimum, which is why we were using only the individual 
driver version info before it was replaced with the kernel version.  I'd 
like to keep at least some bit of driver context information available 
to those on the management side of this PCI device.

sln

>>   	mutex_lock(&ionic->dev_cmd_lock);
>> @@ -558,8 +558,6 @@ int ionic_port_reset(struct ionic *ionic)
>>
>>   static int __init ionic_init_module(void)
>>   {
>> -	pr_info("%s %s, ver %s\n",
>> -		IONIC_DRV_NAME, IONIC_DRV_DESCRIPTION, IONIC_DRV_VERSION);
>>   	ionic_debugfs_create();
>>   	return ionic_bus_register_driver();
>>   }
>> --
>> 2.17.1
>>