svn commit: r253322 - in head/sys/cam: . scsi

[Alexander Motin]

On 15.07.2013 17:48, Ulrich Spörlein wrote:
> On Sat, 2013-07-13 at 13:35:10 +0000, Alexander Motin wrote:
>> Author: mav
>> Date: Sat Jul 13 13:35:09 2013
>> New Revision: 253322
>> URL: http://svnweb.freebsd.org/changeset/base/253322
>>
>> Log:
>>    Improve handling of 0x3F/0x0E "Reported LUNs data has changed" and 0x25/0x00
>>    "Logical unit not supported" errors.  First initiates specific target rescan,
>>    second -- destroys specific LUN.  That allows to automatically detect changes
>>    in list of device LUNs.  This mechanism doesn't work when target is completely
>>    idle, but probably that is all what can be done without active polling.
>>
>>    Reviewed by:	ken
>>    Sponsored by:	iXsystems, Inc.
>>
>> Modified:
>>    head/sys/cam/cam_periph.c
>>    head/sys/cam/scsi/scsi_all.c
>>    head/sys/cam/scsi/scsi_all.h
>>
>> Modified: head/sys/cam/cam_periph.c
>> ==============================================================================
>> @@ -1761,12 +1759,25 @@ cam_periph_error(union ccb *ccb, cam_fla
>>   			xpt_async(AC_LOST_DEVICE, newpath, NULL);
>>   			xpt_free_path(newpath);
>>   		}
>> +	}
>>
>>   	/* Broadcast UNIT ATTENTIONs to all periphs. */
>> -	} else if (scsi_extract_sense_ccb(ccb,
>> -	    &error_code, &sense_key, &asc, &ascq) &&
>> -	    sense_key == SSD_KEY_UNIT_ATTENTION) {
>> +	if ((action & SSQ_UA) != 0)
>>   		xpt_async(AC_UNIT_ATTENTION, orig_ccb->ccb_h.path, orig_ccb);
>> +
>> +	/* Rescan target on "Reported LUNs data has changed" */
>> +	if ((action & SSQ_RESCAN) != 0) {
>> +		if (xpt_create_path(&newpath, NULL,
>> +				    xpt_path_path_id(ccb->ccb_h.path),
>> +				    xpt_path_target_id(ccb->ccb_h.path),
>> +				    -1) == CAM_REQ_CMP) {
>> +
>> +			scan_ccb = xpt_alloc_ccb_nowait();
>> +			scan_ccb->ccb_h.path = newpath;
>> +			scan_ccb->ccb_h.func_CODe = XPT_SCAN_BUS;
>> +			scan_ccb->crcn.flags = 0;
>> +			xpt_rescan(scan_ccb);
>> +		}
>>   	}
>>
>>   	/* Attempt a retry */
>>
>
> This introduces a possible NULL dereference. xpt_alloc_ccb_nowait() may
> return NULL. Coverity reports that this is checked for NULL returns 31
> out of 36 times. Please grep over the tree and fix this plus the other 4
> locations where this is not being null-checked. Thanks!
>
> This has no CID yet (they run a background check that merges and
> assigns CIDs, and this is a fresh run ...)

Thank you. That was a copy-paste of the existing code. I am testing fix 
for three alike cases I've found in CAM.

-- 
Alexander Motin