On Thu, 29 Nov 2018 08:24:38 -0800, Bart Van Assche wrote:

> On Thu, 2018-11-29 at 02:01 +0100, David Disseldorp wrote:
> > diff --git a/drivers/target/target_core_configfs.c 
> > b/drivers/target/target_core_configfs.c
> > index f6b1549f4142..9f49b1afd685 100644
> > --- a/drivers/target/target_core_configfs.c
> > +++ b/drivers/target/target_core_configfs.c
> > @@ -613,12 +613,12 @@ static void dev_set_t10_wwn_model_alias(struct 
> > se_device *dev)
> >     const char *configname;
> >  
> >     configname = config_item_name(&dev->dev_group.cg_item);
> > -   if (strlen(configname) >= 16) {
> > +   if (strlen(configname) >= INQUIRY_MODEL_LEN) {
> >             pr_warn("dev[%p]: Backstore name '%s' is too long for "
> >                     "INQUIRY_MODEL, truncating to 16 bytes\n", dev,
> >                     configname);
> >     }
> > -   snprintf(&dev->t10_wwn.model[0], 16, "%s", configname);
> > +   snprintf(&dev->t10_wwn.model[0], INQUIRY_MODEL_LEN, "%s", configname);  
> 
> Both the old and the new statement truncate inquiry strings that are 16 bytes
> long, which is a bug.

As mentioned in the changelog, I don't think we can fix this without
potentially breaking existing deployments - e.g. a "fourfourfourfour"
backstore name with emulate_model_alias=1 would change inquiry product
ID from "fourfourfourfou" to "fourfourfourfour" following kernel
upgrade.

> Additionally, have you considered to use strlcpy()
> instead of snprintf()?

Happy to change the logic below over if you find it easier to follow.

Cheers, David

Reply via email to