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

["Russell King (Oracle)" <linux@armlinux.org.uk>]

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!