On Mon, Dec 12, 2022 at 5:52 PM Shin'ichiro Kawasaki
<shinichiro.kawasaki@xxxxxxx> wrote:
>
> The function mpi3mr_get_all_tgt_info calculates size of alltgt_info and
> allocate memory for it. After preparing valid data in alltgt_info, it
> calls sg_copy_from_buffer to copy alltgt_info to job->request_payload,
> specifying length of the payload as copy length. This length is larger
> than the calculated alltgt_info size. It causes memory access to invalid
> address and results in "BUG: KASAN: slab-out-of-bounds". The BUG was
> observed during boot using systems with eHBA-9600. By updating the HBA
> firmware to latest version 8.3.1.0 the BUG was not observed during boot,
> but still observed when command "storcli2 /c0 show" is executed.
>
> Fix the BUG by specifying the calculated alltgt_info size as copy
> length. Also check that the copy destination payload length is larger
> than the copy length.
>
> Fixes: f5e6d5a34376 ("scsi: mpi3mr: Add support for driver commands")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@xxxxxxx>
> Reviewed-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
Thanks for the patch, though this code needs a fix, the changes are
not correct and needs modification.
> ---
> drivers/scsi/mpi3mr/mpi3mr_app.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c
> index 9baac224b213..2e35b0fece9c 100644
> --- a/drivers/scsi/mpi3mr/mpi3mr_app.c
> +++ b/drivers/scsi/mpi3mr/mpi3mr_app.c
> @@ -322,6 +322,13 @@ static long mpi3mr_get_all_tgt_info(struct mpi3mr_ioc *mrioc,
>
> kern_entrylen = (num_devices - 1) * sizeof(*devmap_info);
> size = sizeof(*alltgt_info) + kern_entrylen;
> +
> + if (size > job->request_payload.payload_len) {
> + dprint_bsg_err(mrioc, "%s: payload length is too small\n",
> + __func__);
> + return rval;
> + }
> +
This check is not needed, this is already handled by reducing the size
to be copied to the given payload size
> alltgt_info = kzalloc(size, GFP_KERNEL);
> if (!alltgt_info)
> return -ENOMEM;
> @@ -358,7 +365,7 @@ static long mpi3mr_get_all_tgt_info(struct mpi3mr_ioc *mrioc,
>
> sg_copy_from_buffer(job->request_payload.sg_list,
> job->request_payload.sg_cnt,
> - alltgt_info, job->request_payload.payload_len);
> + alltgt_info, size);
instead of size, this should be min_entry_len+sizeof(u32).
> rval = 0;
> out:
> kfree(alltgt_info);
> --
> 2.37.1
>
--
This electronic communication and the information and any files transmitted
with it, or attached to it, are confidential and are intended solely for
the use of the individual or entity to whom it is addressed and may contain
information that is confidential, legally privileged, protected by privacy
laws, or otherwise restricted from disclosure to anyone else. If you are
not the intended recipient or the person responsible for delivering the
e-mail to the intended recipient, you are hereby notified that any use,
copying, distributing, dissemination, forwarding, printing, or copying of
this e-mail is strictly prohibited. If you received this e-mail in error,
please return the e-mail to the sender, delete it from your computer, and
destroy any printed copy of it.
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
