| Message ID | 20221212015706.2609544-2-shinichiro.kawasaki@wdc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scsi: mpi3mr: fix issues found by KASAN | expand |
On 12/12/22 10:57, Shin'ichiro Kawasaki 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@vger.kernel.org > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> > --- > 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..f14556d50832 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: too small payload length\n", "%s: payload length is too small\n" maybe ? > + __func__); > + return rval; > + } > + > 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); > rval = 0; > out: > kfree(alltgt_info); Otherwise, looks OK to me. Reviewed-by: Damien Le Moal
On Dec 12, 2022 / 14:27, Damien Le Moal wrote: > On 12/12/22 10:57, Shin'ichiro Kawasaki 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@vger.kernel.org > > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> > > --- > > 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..f14556d50832 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: too small payload length\n", > > "%s: payload length is too small\n" maybe ? Thanks. Will refelect to v2. > > > + __func__); > > + return rval; > > + } > > + > > 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); > > rval = 0; > > out: > > kfree(alltgt_info); > > Otherwise, looks OK to me. > > Reviewed-by: Damien Le Moal > > -- > Damien Le Moal > Western Digital Research >
diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c index 9baac224b213..f14556d50832 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: too small payload length\n", + __func__); + return rval; + } + 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); rval = 0; out: kfree(alltgt_info);
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@vger.kernel.org Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> --- drivers/scsi/mpi3mr/mpi3mr_app.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)