Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 15 Jul 2013 16:48:44 +0200
From:      Ulrich =?utf-8?B?U3DDtnJsZWlu?= <uqs@FreeBSD.org>
To:        Alexander Motin <mav@FreeBSD.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r253322 - in head/sys/cam: . scsi
Message-ID:  <20130715144844.GA9092@acme.spoerlein.net>
In-Reply-To: <201307131335.r6DDZAQp039101@svn.freebsd.org>
References:  <201307131335.r6DDZAQp039101@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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 ...)



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130715144844.GA9092>