From owner-svn-src-head@FreeBSD.ORG Mon Jul 15 17:42:55 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 74AEBC25; Mon, 15 Jul 2013 17:42:55 +0000 (UTC) (envelope-from mavbsd@gmail.com) Received: from mail-lb0-x22b.google.com (mail-lb0-x22b.google.com [IPv6:2a00:1450:4010:c04::22b]) by mx1.freebsd.org (Postfix) with ESMTP id 6306CD03; Mon, 15 Jul 2013 17:42:54 +0000 (UTC) Received: by mail-lb0-f171.google.com with SMTP id 13so9755662lba.30 for ; Mon, 15 Jul 2013 10:42:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:message-id:date:from:user-agent:mime-version:to:subject :references:in-reply-to:content-type:content-transfer-encoding; bh=WtlfN2x4WMOOQXqaQi8BLKXRGsisP8d0ERrxg2//Fhs=; b=Su7sdukga2UWhragq30JyP9RWhqqXtW2EOFJ6pOVH3MJfZBpl+jMDpP49ihfIuqi4J Mp2UkRAg6uTo95LbSuGE8kO561VrLBrUiR1jhd5RIVWDAhWIJB8XNV06IvbQOhTy04xZ pAQRED06gy3OpaCvciM9VJX34P2UBxI8Rni9y8xhYheRUfVCFcmnSJLOtu6CkqXq3n2Y 0t1kl3Sq7t1Q9nZlJFEzb/B8v4zvdHmvb5X5+Nd6d47MscFPh/ydN5SBYpA965SJ5WnL S0JC+2oxu9ZnORr13SfVR8sYEGC2A7z+2TR5puk6Rr6IOSLSha/swFN9cVo56Y1n4Fp9 3RWw== X-Received: by 10.112.144.35 with SMTP id sj3mr24963523lbb.4.1373910173058; Mon, 15 Jul 2013 10:42:53 -0700 (PDT) Received: from mavbook.mavhome.dp.ua (mavhome.mavhome.dp.ua. [213.227.240.37]) by mx.google.com with ESMTPSA id et10sm18783822lbc.6.2013.07.15.10.42.50 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 15 Jul 2013 10:42:51 -0700 (PDT) Sender: Alexander Motin Message-ID: <51E43498.70804@FreeBSD.org> Date: Mon, 15 Jul 2013 20:42:48 +0300 From: Alexander Motin User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:17.0) Gecko/20130616 Thunderbird/17.0.6 MIME-Version: 1.0 To: =?ISO-8859-1?Q?Ulrich_Sp=F6rlein?= , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r253322 - in head/sys/cam: . scsi References: <201307131335.r6DDZAQp039101@svn.freebsd.org> <20130715144844.GA9092@acme.spoerlein.net> In-Reply-To: <20130715144844.GA9092@acme.spoerlein.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 Jul 2013 17:42:55 -0000 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