Re: [PATCH] scsi: scsi_debug: Fix a warning in resp_write_scat()

[Harshit Mogalapalli]

Hi Doug,

On 11/11/22 10:53 pm, Douglas Gilbert wrote:
> On 2022-11-11 05:05, Harshit Mogalapalli wrote:
> > As 'lbdof_blen' is coming from user, if the size in kzalloc()
> > is >= MAX_ORDER then we hit a warning.
> >
> > Call trace:
> >
> > sg_ioctl
> >   sg_ioctl_common
> >     scsi_ioctl
> >      sg_scsi_ioctl
> >       blk_execute_rq
> >        blk_mq_sched_insert_request
> >         blk_mq_run_hw_queue
> >          __blk_mq_delay_run_hw_queue
> >           __blk_mq_run_hw_queue
> >            blk_mq_sched_dispatch_requests
> >             __blk_mq_sched_dispatch_requests
> >              blk_mq_dispatch_rq_list
> >               scsi_queue_rq
> >                scsi_dispatch_cmd
> >                 scsi_debug_queuecommand
> >                  schedule_resp
> >                   resp_write_scat
> >
> > If you try to allocate a memory larger than(>=) MAX_ORDER, then kmalloc()
> > will definitely fail.  It creates a stack trace and messes up dmesg.
> > The user controls the size here so if they specify a too large size it
> > will fail.
> >
> > Add __GFP_NOWARN in order to avoid too large allocation warning.
> > This is detected by static analysis using smatch.
> >
> > Fixes: 481b5e5c7949 ("scsi: scsi_debug: add resp_write_scat function")
> > Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@xxxxxxxxxx>
>
> Acked-by: Douglas Gilbert <dgilbert@xxxxxxxxxxxx>

Thank you very much for checking the patch.

We have two more similar cases in the scsi_debug.c file in resp_verify() 
and resp_report_zones() functions. These are also detected using static 
analysis with smatch.

I have sent out patches for those.

Thanks,
Harshit

> Thanks.
>
> > ---
> >   drivers/scsi/scsi_debug.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> > index 697fc57bc711..273224d29ce9 100644
> > --- a/drivers/scsi/scsi_debug.c
> > +++ b/drivers/scsi/scsi_debug.c
> > @@ -3778,7 +3778,7 @@ static int resp_write_scat(struct scsi_cmnd *scp,
> >           mk_sense_buffer(scp, ILLEGAL_REQUEST, INVALID_FIELD_IN_CDB, 0);
> >           return illegal_condition_result;
> >       }
> > -    lrdp = kzalloc(lbdof_blen, GFP_ATOMIC);
> > +    lrdp = kzalloc(lbdof_blen, GFP_ATOMIC | __GFP_NOWARN);
> >       if (lrdp == NULL)
> >           return SCSI_MLQUEUE_HOST_BUSY;
> >       if (sdebug_verbose)