Re: [PATCH v2] scsi: sg: finish request on error in sg_read()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] sg: compilation error due to undefined label free_old_hdr
--

commit 016a191c362e931770f582b73bf2ede9e57bbb56
Author: Hui Peng <benquike@gmail.com>

scsi: sg: finish request on error in sg_read()

This commit modifies sg_read() to ensure that orphaned requests are properly
finished and removed when an error occurs. It does this by introducing a new
label to call the necessary cleanup functions before returning.

> diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
> index 3f9e08725602c..581419e1695fc 100644
> --- a/drivers/scsi/sg.c
> +++ b/drivers/scsi/sg.c
[ ... ]
> @@ -543,10 +545,10 @@ sg_read(struct file *filp, char __user *buf, size_t count, loff_t * ppos)
>  		}
>  	} else
>  		count = (old_hdr->result == 0) ? 0 : -EIO;
> +	retval = count;
> +finish_req:
>  	sg_finish_rem_req(srp);
>  	sg_remove_request(sfp, srp);
> -	retval = count;
> -free_old_hdr:

[Severity: Low]
Because the free_old_hdr label is removed here, does this cause a
"label used but not defined" compilation error in sg_read()?

The error paths earlier in the function still try to jump to this deleted
label upon copy_to_user() failure and sg_read_oxfer() failure:

    if (copy_to_user(buf, old_hdr, SZ_SG_HEADER)) {
        retval = -EFAULT;
        goto free_old_hdr;
    }
    ...
    if (sg_read_oxfer(srp, buf, count - SZ_SG_HEADER)) {
        retval = -EFAULT;
        goto free_old_hdr;
    }

Should those goto statements be updated to jump to finish_req instead?

>  	kfree(old_hdr);
>  	return retval;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924054625.2282544-1-benquike@gmail.com?part=1