netdev - Re: [PATCH net-next 2/3] nfp: look for firmware image by device serial number and PCI name

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: <20170727022546.1fe8d1ea@cakuba.netronome.com>
Date:   Thu, 27 Jul 2017 02:25:46 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     David Laight <David.Laight@...LAB.COM>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "oss-drivers@...ronome.com" <oss-drivers@...ronome.com>
Subject: Re: [PATCH net-next 2/3] nfp: look for firmware image by device
 serial number and PCI name

On Thu, 27 Jul 2017 08:20:22 +0000, David Laight wrote:
> From: Jakub Kicinski
> > Sent: 26 July 2017 19:10
> > We generally look up firmware by card type, but that doesn't allow
> > users who have more than one card of the same type in their system
> > to select firmware per adapter.
> > 
> > Unfortunately user space firmware helper seems fraught with
> > difficulties and to be on its way out.  In particular support for
> > handling firmware uevents have been dropped from systemd and most
> > distributions don't enable the FW fallback by default any more.
> > 
> > To allow users selecting firmware for a particular device look up
> > firmware names by serial and pci_name().  Use the direct lookup to
> > disable generating uevents when enabled in Kconfig and not print
> > any warnings to logs if adapter-specific files are missing.  Users
> > can place in /lib/firmware/netronome files named:
> > 
> > pci-${pci_name}.nffw
> > serial-${serial}.nffw
> > 
> > to target a specific card.  E.g.:
> > 
> > pci-0000:04:00.0.nffw
> > pci-0000:82:00.0.nffw
> > serial-00-aa-bb-11-22-33-10-ff.nffw
> > 
> > We use the full serial number including the interface id, as it
> > appears in lspci output (bytes separated by '-').  
> 
> Where does lspci get that from?

Extended capability type 3, Device Serial Number.

> > Signed-off-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
> > ---
> >  drivers/net/ethernet/netronome/nfp/nfp_main.c | 20 +++++++++++++++++++-
> >  1 file changed, 19 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/ethernet/netronome/nfp/nfp_main.c
> > b/drivers/net/ethernet/netronome/nfp/nfp_main.c
> > index d67969d3e484..13d056da0765 100644
> > --- a/drivers/net/ethernet/netronome/nfp/nfp_main.c
> > +++ b/drivers/net/ethernet/netronome/nfp/nfp_main.c
> > @@ -188,9 +188,27 @@ nfp_net_fw_find(struct pci_dev *pdev, struct nfp_pf *pf)
> >  	struct nfp_eth_table_port *port;
> >  	const char *fw_model;
> >  	char fw_name[256];
> > +	const u8 *serial;
> >  	int spc, err = 0;
> > +	u16 interface;
> >  	int i, j;
> > 
> > +	/* First try to find a firmware image specific for this device */
> > +	interface = nfp_cpp_interface(pf->cpp);
> > +	nfp_cpp_serial(pf->cpp, &serial);
> > +	sprintf(fw_name, "netronome/serial-%pMF-%02hhx-%02hhx.nffw",
> > +		serial, interface >> 8, interface & 0xff);  
> 
> WTF???

Please be civil.

> - use snprintf().

To effectively print an integer into an amply sized array?  I need to
guarantee that the string will fit otherwise I would request a FW image
with a wrong name.  snprintf() would only mask such a bug.

> - kill those hh, the arguments are of type 'int'.

It doesn't matter.  I will be more careful in the future, though.

> In fact make 'interface' 'unsigned int' as well.

It's a value read from the hardware, and it's 16 bits wide, therefore
my preference it to explicitly size the variable.