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]
Message-ID: <20200305075216.GA184088@unreal>
Date:   Thu, 5 Mar 2020 09:52:16 +0200
From:   Leon Romanovsky <leonro@...lanox.com>
To:     Shannon Nelson <snelson@...sando.io>
Cc:     netdev@...r.kernel.org, davem@...emloft.net
Subject: Re: [PATCH v3 net-next 8/8] ionic: drop ethtool driver version

On Wed, Mar 04, 2020 at 11:20:11PM -0800, Shannon Nelson wrote:
> 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.

Again, I'm not netdev person and say nothing about if it is ok or not.
However, this string is completely unreliable, especially if user
changes your driver in his bare-metal machine.

Thanks

>
> 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
> > >
>