Re: [PATCH] ipr: Fix regression when loading firmware

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Fri, 2016-02-26 at 14:52 -0600, Brian King wrote:
> On 02/25/2016 10:54 AM, Gabriel Krisman Bertazi wrote:
> > Commit d63c7dd5bcb9 ("ipr: Fix out-of-bounds null overwrite")
> > removed the
> > end of line handling when storing the update_fw sysfs attribute. 
> >  This
> > changed the userpace API because it started refusing writes
> > terminated
> > by a line feed, which broke the update tools we already have.
> > 
> > This patch re-adds that handling, so both a write terminated by a
> > line
> > feed or not can make it through with the update.
> > 
> > Fixes: d63c7dd5bcb9 ("ipr: Fix out-of-bounds null overwrite")
> > Signed-off-by: Gabriel Krisman Bertazi <krisman@linux.vnet.ibm.com>
> > Cc: Insu Yun <wuninsu@gmail.com>
> > ---
> >  drivers/scsi/ipr.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
> > index 3b3e099..d6a691e 100644
> > --- a/drivers/scsi/ipr.c
> > +++ b/drivers/scsi/ipr.c
> > @@ -4002,6 +4002,7 @@ static ssize_t ipr_store_update_fw(struct
> > device *dev,
> >  	struct ipr_sglist *sglist;
> >  	char fname[100];
> >  	char *src;
> > +	char *endline;
> >  	int result, dnld_size;
> > 
> >  	if (!capable(CAP_SYS_ADMIN))
> > @@ -4009,6 +4010,10 @@ static ssize_t ipr_store_update_fw(struct
> > device *dev,
> > 
> >  	snprintf(fname, sizeof(fname), "%s", buf);
> > 
> > +	endline = strchr(fname, '\n');
> > +	if (endline)
> > +		*endline = '\0';
> > +
> >  	if (request_firmware(&fw_entry, fname, &ioa_cfg->pdev
> > ->dev)) {
> >  		dev_err(&ioa_cfg->pdev->dev, "Firmware file %s not
> > found\n", fname);
> >  		return -EIO;
> > 
> 
> Acked-by: Brian King <brking@linux.vnet.ibm.com>
> 
> James - since this is a regression, can we get this fix in for 4.5 
> yet?

Yes, but in future, could you actually check patches in your driver
before they get sent to Linus?  This one went via the usual merge
window process, so there was plenty of time to test it. We get a lot of
these apparently innocuous minor bug fixes that actually contain a much
more subtle bug.  They're very difficult for reviewers to spot, but
they do show up on hardware testing.

James