svn commit: r309956 - stable/10/sbin/camcontrol
Kenneth D. Merry
ken at FreeBSD.org
Mon Dec 12 21:54:50 UTC 2016
Author: ken
Date: Mon Dec 12 21:54:49 2016
New Revision: 309956
URL: https://svnweb.freebsd.org/changeset/base/309956
Log:
MFC r307684, r307747
------------------------------------------------------------------------
r307684 | ken | 2016-10-20 13:42:26 -0600 (Thu, 20 Oct 2016) | 13 lines
For CCBs allocated on the stack, we need to clear the entire CCB, not just
the header. Otherwise stack garbage can lead to random flags getting set.
This showed up as 'camcontrol rescan all' failing with EINVAL because the
address type wasn't CAM_DATA_VADDR.
sbin/camcontrol/camcontrol.c:
In rescan_or_reset_bus(), bzero the stack-allocated CCBs before
use instead of clearing the body.
Sponsored by: Spectra Logic
------------------------------------------------------------------------
r307747 | ken | 2016-10-21 12:54:56 -0600 (Fri, 21 Oct 2016) | 27 lines
Fix a problem in camcontrol(8) that cropped up with r307684.
In r307684, I changed rescan_or_reset_bus() to bzero stack-allocated CCBs
before sending them to the kernel because there was stack garbage in there
that wound up meaning that bogus CCB flags were set.
While this fixed the 'camcontrol rescan all' case (XPT_DEV_MATCH CCBs were
failing previously), it broke the 'camcontrol rescan 0' (or any other
number) case when INVARIANTS are turned on. Rescanning a single bus
reliably produced an assert in cam_periph_runccb():
panic: cam_periph_runccb: ccb=0xfffff80044ffe000, func_code=0x708, flags=0xffffdde0
The flags values don't make sense from the code. Changing the CCBs in
rescan_or_reset_bus() from stack to heap allocated avoids the problem.
It would be better to understand why userland stack allocated CCBs don't
work properly, since there may be other code that breaks if stack allocated
CCBs don't work.
sbin/camcontrol/camcontrol.c:
In rescan_or_reset_bus(), allocate the CCBs using malloc(3) instead
of on the stack to avoid an assertion in cam_periph_runccb().
Sponsored by: Spectra Logic
------------------------------------------------------------------------
Sponsored by: Spectra Logic
Modified:
stable/10/sbin/camcontrol/camcontrol.c
Directory Properties:
stable/10/ (props changed)
Modified: stable/10/sbin/camcontrol/camcontrol.c
==============================================================================
--- stable/10/sbin/camcontrol/camcontrol.c Mon Dec 12 21:49:29 2016 (r309955)
+++ stable/10/sbin/camcontrol/camcontrol.c Mon Dec 12 21:54:49 2016 (r309956)
@@ -3124,8 +3124,8 @@ dorescan_or_reset(int argc, char **argv,
static int
rescan_or_reset_bus(path_id_t bus, int rescan)
{
- union ccb ccb, matchccb;
- int fd, retval;
+ union ccb *ccb = NULL, *matchccb = NULL;
+ int fd = -1, retval;
int bufsize;
retval = 0;
@@ -3136,35 +3136,41 @@ rescan_or_reset_bus(path_id_t bus, int r
return(1);
}
+ ccb = malloc(sizeof(*ccb));
+ if (ccb == NULL) {
+ warn("failed to allocate CCB");
+ retval = 1;
+ goto bailout;
+ }
+ bzero(ccb, sizeof(*ccb));
+
if (bus != CAM_BUS_WILDCARD) {
- ccb.ccb_h.func_code = rescan ? XPT_SCAN_BUS : XPT_RESET_BUS;
- ccb.ccb_h.path_id = bus;
- ccb.ccb_h.target_id = CAM_TARGET_WILDCARD;
- ccb.ccb_h.target_lun = CAM_LUN_WILDCARD;
- ccb.crcn.flags = CAM_FLAG_NONE;
+ ccb->ccb_h.func_code = rescan ? XPT_SCAN_BUS : XPT_RESET_BUS;
+ ccb->ccb_h.path_id = bus;
+ ccb->ccb_h.target_id = CAM_TARGET_WILDCARD;
+ ccb->ccb_h.target_lun = CAM_LUN_WILDCARD;
+ ccb->crcn.flags = CAM_FLAG_NONE;
/* run this at a low priority */
- ccb.ccb_h.pinfo.priority = 5;
+ ccb->ccb_h.pinfo.priority = 5;
- if (ioctl(fd, CAMIOCOMMAND, &ccb) == -1) {
+ if (ioctl(fd, CAMIOCOMMAND, ccb) == -1) {
warn("CAMIOCOMMAND ioctl failed");
- close(fd);
- return(1);
+ retval = 1;
+ goto bailout;
}
- if ((ccb.ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP) {
+ if ((ccb->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP) {
fprintf(stdout, "%s of bus %d was successful\n",
rescan ? "Re-scan" : "Reset", bus);
} else {
fprintf(stdout, "%s of bus %d returned error %#x\n",
rescan ? "Re-scan" : "Reset", bus,
- ccb.ccb_h.status & CAM_STATUS_MASK);
+ ccb->ccb_h.status & CAM_STATUS_MASK);
retval = 1;
}
- close(fd);
- return(retval);
-
+ goto bailout;
}
@@ -3178,58 +3184,64 @@ rescan_or_reset_bus(path_id_t bus, int r
* no-op, sending a rescan to the xpt bus would result in a status of
* CAM_REQ_INVALID.
*/
- CCB_CLEAR_ALL_EXCEPT_HDR(&matchccb.cdm);
- matchccb.ccb_h.func_code = XPT_DEV_MATCH;
- matchccb.ccb_h.path_id = CAM_BUS_WILDCARD;
+ matchccb = malloc(sizeof(*matchccb));
+ if (matchccb == NULL) {
+ warn("failed to allocate CCB");
+ retval = 1;
+ goto bailout;
+ }
+ bzero(matchccb, sizeof(*matchccb));
+ matchccb->ccb_h.func_code = XPT_DEV_MATCH;
+ matchccb->ccb_h.path_id = CAM_BUS_WILDCARD;
bufsize = sizeof(struct dev_match_result) * 20;
- matchccb.cdm.match_buf_len = bufsize;
- matchccb.cdm.matches=(struct dev_match_result *)malloc(bufsize);
- if (matchccb.cdm.matches == NULL) {
+ matchccb->cdm.match_buf_len = bufsize;
+ matchccb->cdm.matches=(struct dev_match_result *)malloc(bufsize);
+ if (matchccb->cdm.matches == NULL) {
warnx("can't malloc memory for matches");
retval = 1;
goto bailout;
}
- matchccb.cdm.num_matches = 0;
+ matchccb->cdm.num_matches = 0;
- matchccb.cdm.num_patterns = 1;
- matchccb.cdm.pattern_buf_len = sizeof(struct dev_match_pattern);
+ matchccb->cdm.num_patterns = 1;
+ matchccb->cdm.pattern_buf_len = sizeof(struct dev_match_pattern);
- matchccb.cdm.patterns = (struct dev_match_pattern *)malloc(
- matchccb.cdm.pattern_buf_len);
- if (matchccb.cdm.patterns == NULL) {
+ matchccb->cdm.patterns = (struct dev_match_pattern *)malloc(
+ matchccb->cdm.pattern_buf_len);
+ if (matchccb->cdm.patterns == NULL) {
warnx("can't malloc memory for patterns");
retval = 1;
goto bailout;
}
- matchccb.cdm.patterns[0].type = DEV_MATCH_BUS;
- matchccb.cdm.patterns[0].pattern.bus_pattern.flags = BUS_MATCH_ANY;
+ matchccb->cdm.patterns[0].type = DEV_MATCH_BUS;
+ matchccb->cdm.patterns[0].pattern.bus_pattern.flags = BUS_MATCH_ANY;
do {
unsigned int i;
- if (ioctl(fd, CAMIOCOMMAND, &matchccb) == -1) {
+ if (ioctl(fd, CAMIOCOMMAND, matchccb) == -1) {
warn("CAMIOCOMMAND ioctl failed");
retval = 1;
goto bailout;
}
- if ((matchccb.ccb_h.status != CAM_REQ_CMP)
- || ((matchccb.cdm.status != CAM_DEV_MATCH_LAST)
- && (matchccb.cdm.status != CAM_DEV_MATCH_MORE))) {
+ if ((matchccb->ccb_h.status != CAM_REQ_CMP)
+ || ((matchccb->cdm.status != CAM_DEV_MATCH_LAST)
+ && (matchccb->cdm.status != CAM_DEV_MATCH_MORE))) {
warnx("got CAM error %#x, CDM error %d\n",
- matchccb.ccb_h.status, matchccb.cdm.status);
+ matchccb->ccb_h.status, matchccb->cdm.status);
retval = 1;
goto bailout;
}
- for (i = 0; i < matchccb.cdm.num_matches; i++) {
+ for (i = 0; i < matchccb->cdm.num_matches; i++) {
struct bus_match_result *bus_result;
/* This shouldn't happen. */
- if (matchccb.cdm.matches[i].type != DEV_MATCH_BUS)
+ if (matchccb->cdm.matches[i].type != DEV_MATCH_BUS)
continue;
- bus_result = &matchccb.cdm.matches[i].result.bus_result;
+ bus_result =&matchccb->cdm.matches[i].result.bus_result;
/*
* We don't want to rescan or reset the xpt bus.
@@ -3238,23 +3250,23 @@ rescan_or_reset_bus(path_id_t bus, int r
if (bus_result->path_id == CAM_XPT_PATH_ID)
continue;
- ccb.ccb_h.func_code = rescan ? XPT_SCAN_BUS :
+ ccb->ccb_h.func_code = rescan ? XPT_SCAN_BUS :
XPT_RESET_BUS;
- ccb.ccb_h.path_id = bus_result->path_id;
- ccb.ccb_h.target_id = CAM_TARGET_WILDCARD;
- ccb.ccb_h.target_lun = CAM_LUN_WILDCARD;
- ccb.crcn.flags = CAM_FLAG_NONE;
+ ccb->ccb_h.path_id = bus_result->path_id;
+ ccb->ccb_h.target_id = CAM_TARGET_WILDCARD;
+ ccb->ccb_h.target_lun = CAM_LUN_WILDCARD;
+ ccb->crcn.flags = CAM_FLAG_NONE;
/* run this at a low priority */
- ccb.ccb_h.pinfo.priority = 5;
+ ccb->ccb_h.pinfo.priority = 5;
- if (ioctl(fd, CAMIOCOMMAND, &ccb) == -1) {
+ if (ioctl(fd, CAMIOCOMMAND, ccb) == -1) {
warn("CAMIOCOMMAND ioctl failed");
retval = 1;
goto bailout;
}
- if ((ccb.ccb_h.status & CAM_STATUS_MASK) ==CAM_REQ_CMP){
+ if ((ccb->ccb_h.status & CAM_STATUS_MASK)==CAM_REQ_CMP){
fprintf(stdout, "%s of bus %d was successful\n",
rescan? "Re-scan" : "Reset",
bus_result->path_id);
@@ -3267,22 +3279,24 @@ rescan_or_reset_bus(path_id_t bus, int r
fprintf(stderr, "%s of bus %d returned error "
"%#x\n", rescan? "Re-scan" : "Reset",
bus_result->path_id,
- ccb.ccb_h.status & CAM_STATUS_MASK);
+ ccb->ccb_h.status & CAM_STATUS_MASK);
retval = 1;
}
}
- } while ((matchccb.ccb_h.status == CAM_REQ_CMP)
- && (matchccb.cdm.status == CAM_DEV_MATCH_MORE));
+ } while ((matchccb->ccb_h.status == CAM_REQ_CMP)
+ && (matchccb->cdm.status == CAM_DEV_MATCH_MORE));
bailout:
if (fd != -1)
close(fd);
- if (matchccb.cdm.patterns != NULL)
- free(matchccb.cdm.patterns);
- if (matchccb.cdm.matches != NULL)
- free(matchccb.cdm.matches);
+ if (matchccb != NULL) {
+ free(matchccb->cdm.patterns);
+ free(matchccb->cdm.matches);
+ free(matchccb);
+ }
+ free(ccb);
return(retval);
}
More information about the svn-src-all
mailing list