Re: [PATCH next] drivers/scsi/aic7xxx/aic79xx_osm: Use kstrdup() instead of kmalloc() and strcpy()

[David Laight <david.laight.linux@gmail.com>]

On Sat, 6 Jun 2026 23:42:36 +0200
Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote:

> Le 06/06/2026 à 22:26, david.laight.linux@gmail.com a écrit :
> > From: David Laight <david.laight.linux@gmail.com>
> > 
> > Signed-off-by: David Laight <david.laight.linux@gmail.com>
> > ---
> > This is one of a group of patches that remove potentially unbounded
> > strcpy() calls.
> > 
> > They are mostly replaced by strscpy() or, when strlen() has just been
> > called, with memcpy() (usually including the '\0').
> > 
> > Calls with copy string literals into arrays are left unchanged.
> > They are safe and easily detected as such.
> > 
> > The changes were made by getting the compiler to detect the calls and
> > then fixing the code by hand.
> > 
> > Note that all the changes are only compile tested.
> > 
> > Some Makefiles were changed to allow files to contain strcpy().
> > As well as 'difficult to fix' files, this included 'show' functions
> > as they really need to use sysfs_emit() or seq_printf().
> > 
> > All the patches are being sent individually to avoid very long cc lists.
> > Apologies for the terse commit messages and likely unexpected tags.
> > (There are about 100 patches in total.)
> > 
> >   drivers/scsi/aic7xxx/aic79xx_osm.c | 6 ++----
> >   1 file changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/scsi/aic7xxx/aic79xx_osm.c b/drivers/scsi/aic7xxx/aic79xx_osm.c
> > index feb1707feb7e..97ebee94230e 100644
> > --- a/drivers/scsi/aic7xxx/aic79xx_osm.c
> > +++ b/drivers/scsi/aic7xxx/aic79xx_osm.c
> > @@ -1233,11 +1233,9 @@ ahd_linux_register_host(struct ahd_softc *ahd, struct scsi_host_template *templa
> >   	ahd_set_unit(ahd, ahd_linux_unit++);
> >   	ahd_unlock(ahd, &s);
> >   	sprintf(buf, "scsi%d", host->host_no);
> > -	new_name = kmalloc(strlen(buf) + 1, GFP_ATOMIC);
> > -	if (new_name != NULL) {
> > -		strcpy(new_name, buf);
> > +	new_name = kstrdup(buf, GFP_ATOMIC);  
> 
> I think that kasprintf() would simplify code and do the same.
> 
> Otherwise, s/sprintf/snprintf/ could be done, as in the patch for 
> aic7xxx_osm.c

Looks like I missed the sprintf() here.
I was doing a lot of simple changes and trying not to rewrite too much.

Looking a bit deeper into the ahd code there is an 8 byte pointer
that usually references an 8 byte buffer.
A fixed char name[16] will use less memory overall.
The only other thing it ever references is the result of:
	sprintf(buf, "ahd_pci:%d:%d:%d",
		ahd_get_pci_bus(pci),
		ahd_get_pci_slot(pci),
		ahd_get_pci_function(pci));
which it only does temporarily during ahd_linux_pci_dev_probe().
Even that is usually less than 16 bytes.
Given the probe function pretty much never fails, the scsi%d
string could be generated earlier with just a trace to tie
the two together.

But that is far beyond this set of changes.

Doing a 'v2' that isn't just an edit of the email will be a pita.

-- David

> 
> CJ
> 
> > +	if (new_name != NULL)
> >   		ahd_set_name(ahd, new_name);
> > -	}
> >   	host->unique_id = ahd->unit;
> >   	ahd_linux_initialize_scsi_bus(ahd);
> >   	ahd_intr_enable(ahd, TRUE);  
>