netdev - Re: [PATCH net-next V2 2/5] net: lan743x: Add support to software-nodes for sfp

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 for Android: free password hash cracker in your pocket

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

Message-ID: <ZuiHhoxi05IOWphr@shell.armlinux.org.uk>
Date: Mon, 16 Sep 2024 20:31:18 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Raju Lakkaraju <Raju.Lakkaraju@...rochip.com>
Cc: Christophe JAILLET <christophe.jaillet@...adoo.fr>,
	netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, bryan.whitehead@...rochip.com,
	UNGLinuxDriver@...rochip.com, maxime.chevallier@...tlin.com,
	rdunlap@...radead.org, andrew@...n.ch, Steen.Hegelund@...rochip.com,
	daniel.machon@...rochip.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next V2 2/5] net: lan743x: Add support to
 software-nodes for sfp

On Thu, Sep 12, 2024 at 12:02:57PM +0530, Raju Lakkaraju wrote:
> Hi Christophe,
> 
> The 09/11/2024 18:54, Christophe JAILLET wrote:
> > > +static int pci1xxxx_i2c_adapter_get(struct lan743x_adapter *adapter)
> > > +{
> > > +     struct pci1xxxx_i2c *i2c_drvdata;
> > > +
> > > +     i2c_drvdata = pci1xxxx_perif_drvdata_get(adapter, PCI1XXXX_PERIF_I2C_ID);
> > > +     if (!i2c_drvdata)
> > > +             return -EPROBE_DEFER;
> > > +
> > > +     adapter->i2c_adap = &i2c_drvdata->adap;
> > > +     snprintf(adapter->nodes->i2c_name, sizeof(adapter->nodes->i2c_name),
> > > +              adapter->i2c_adap->name);
> > 
> > strscpy() ?
> > 
> 
> Accepted. I will fix.
> Here snprintf( ) does not take any format string, we can use strscpy( ).

As a general tip for safe programming... never use snprintf() as a
"short cut" for copying strings. It may do stuff that you don't
expect!

For example, taking the above case, if "adapter->i2c_adap->name"
contains any % characters, then, as you are passing it as the
_format_ _string_, sprintf() will try to interpret those as printf
escape sequences, and thus _can_ attempt to dereference arguments
that were never passed to snprintf().

If you really want to do this kind of thing, at least write it in
a safe way...

	snprintf(..., "%s", string);

rather than:

	snprintf(..., string);

so that "string" doesn't attempt to be escape-expanded.

Of course, using proper string copying functions that do what you
want in a cheap way is always more preferable to the printf related
functions!

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!