diff options
author | truckman <truckman@FreeBSD.org> | 2016-07-06 17:45:38 +0000 |
---|---|---|
committer | truckman <truckman@FreeBSD.org> | 2016-07-06 17:45:38 +0000 |
commit | 4199bec9b4f66720daa3225f22555919a48e7182 (patch) | |
tree | b8508db7b6ec491d1307c6f0630f0e57962c6183 /usr.sbin/mptutil | |
parent | 15fb55aa670314e87deb319e02abdf4bcbac078b (diff) | |
download | FreeBSD-src-4199bec9b4f66720daa3225f22555919a48e7182.zip FreeBSD-src-4199bec9b4f66720daa3225f22555919a48e7182.tar.gz |
MFC r300547
Fix multiple Coverity Out-of-bounds access false postive issues in CAM
The currently used idiom for clearing the part of a ccb after its
header generates one or two Coverity errors for each time it is
used. All instances generate an Out-of-bounds access (ARRAY_VS_SINGLETON)
error because of the treatment of the header as a two element array,
with a pointer to the non-existent second element being passed as
the starting address to bzero(). Some instances also alsp generate
Out-of-bounds access (OVERRUN) errors, probably because the space
being cleared is larger than the sizeofstruct ccb_hdr).
In addition, this idiom is difficult for humans to understand and
it is error prone. The user has to chose the proper struct ccb_*
type (which does not appear in the surrounding code) for the sizeof()
in the length calculation. I found several instances where the
length was incorrect, which could cause either an actual out of
bounds write, or incompletely clear the ccb.
A better way is to write the code to clear the ccb itself starting
at sizeof(ccb_hdr) bytes from the start of the ccb, and calculate
the length based on the specific type of struct ccb_* being cleared
as specified by the union ccb member being used. The latter can
normally be seen in the nearby code. This is friendlier for Coverity
and other static analysis tools because they will see that the
intent is to clear the trailing part of the ccb.
Wrap all of the boilerplate code in a convenient macro that only
requires a pointer to the desired union ccb member (or a pointer
to the union ccb itself) as an argument.
Reported by: Coverity
CID: 1007578, 1008684, 1009724, 1009773, 1011304, 1011306
CID: 1011307, 1011308, 1011309, 1011310, 1011311, 1011312
CID: 1011313, 1011314, 1011315, 1011316, 1011317, 1011318
CID: 1011319, 1011320, 1011321, 1011322, 1011324, 1011325
CID: 1011326, 1011327, 1011328, 1011329, 1011330, 1011374
CID: 1011390, 1011391, 1011392, 1011393, 1011394, 1011395
CID: 1011396, 1011397, 1011398, 1011399, 1011400, 1011401
CID: 1011402, 1011403, 1011404, 1011405, 1011406, 1011408
CID: 1011409, 1011410, 1011411, 1011412, 1011413, 1011414
CID: 1017461, 1018387, 1086860, 1086874, 1194257, 1229897
CID: 1229968, 1306229, 1306234, 1331282, 1331283, 1331294
CID: 1331295, 1331535, 1331536, 1331539, 1331540, 1341623
CID: 1341624, 1341637, 1341638, 1355264, 1355324
Reviewed by: scottl, ken, delphij, imp
MFH: 1 month
Differential Revision: https://reviews.freebsd.org/D6496
Diffstat (limited to 'usr.sbin/mptutil')
-rw-r--r-- | usr.sbin/mptutil/mpt_cam.c | 9 |
1 files changed, 3 insertions, 6 deletions
diff --git a/usr.sbin/mptutil/mpt_cam.c b/usr.sbin/mptutil/mpt_cam.c index a00e228..264cb47 100644 --- a/usr.sbin/mptutil/mpt_cam.c +++ b/usr.sbin/mptutil/mpt_cam.c @@ -241,8 +241,7 @@ fetch_scsi_capacity(struct cam_device *dev, struct mpt_standalone_disk *disk) return (ENOMEM); /* Zero the rest of the ccb. */ - bzero(&(&ccb->ccb_h)[1], sizeof(struct ccb_scsiio) - - sizeof(struct ccb_hdr)); + CCB_CLEAR_ALL_EXCEPT_HDR(&ccb->csio); scsi_read_capacity(&ccb->csio, 1, NULL, MSG_SIMPLE_Q_TAG, &rcap, SSD_FULL_SIZE, 5000); @@ -273,8 +272,7 @@ fetch_scsi_capacity(struct cam_device *dev, struct mpt_standalone_disk *disk) } /* Zero the rest of the ccb. */ - bzero(&(&ccb->ccb_h)[1], sizeof(struct ccb_scsiio) - - sizeof(struct ccb_hdr)); + CCB_CLEAR_ALL_EXCEPT_HDR(&ccb->csio); scsi_read_capacity_16(&ccb->csio, 1, NULL, MSG_SIMPLE_Q_TAG, 0, 0, 0, (uint8_t *)&rcaplong, sizeof(rcaplong), SSD_FULL_SIZE, 5000); @@ -355,8 +353,7 @@ fetch_scsi_inquiry(struct cam_device *dev, struct mpt_standalone_disk *disk) return (ENOMEM); /* Zero the rest of the ccb. */ - bzero(&(&ccb->ccb_h)[1], sizeof(struct ccb_scsiio) - - sizeof(struct ccb_hdr)); + CCB_CLEAR_ALL_EXCEPT_HDR(&ccb->csio); inq_buf = calloc(1, sizeof(*inq_buf)); if (inq_buf == NULL) { |