From patchwork Fri Dec 4 19:16:54 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Bottomley X-Patchwork-Id: 7771401 Return-Path: X-Original-To: patchwork-linux-scsi@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id F15409F350 for ; Fri, 4 Dec 2015 19:17:17 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id F17BE2060C for ; Fri, 4 Dec 2015 19:17:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id ED61C20609 for ; Fri, 4 Dec 2015 19:17:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752274AbbLDTQ5 (ORCPT ); Fri, 4 Dec 2015 14:16:57 -0500 Received: from bedivere.hansenpartnership.com ([66.63.167.143]:57356 "EHLO bedivere.hansenpartnership.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752100AbbLDTQ5 (ORCPT ); Fri, 4 Dec 2015 14:16:57 -0500 Received: from localhost (localhost [127.0.0.1]) by bedivere.hansenpartnership.com (Postfix) with ESMTP id 562C38EE258; Fri, 4 Dec 2015 11:16:56 -0800 (PST) Received: from bedivere.hansenpartnership.com ([127.0.0.1]) by localhost (bedivere.hansenpartnership.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id yeDqrq037Uv6; Fri, 4 Dec 2015 11:16:56 -0800 (PST) Received: from [153.66.254.242] (unknown [184.11.141.41]) by bedivere.hansenpartnership.com (Postfix) with ESMTPSA id C6A5C8EE0A2; Fri, 4 Dec 2015 11:16:55 -0800 (PST) Message-ID: <1449256614.27077.11.camel@HansenPartnership.com> Subject: Re: BUG: KASAN: slab-out-of-bounds in ses_enclosure_data_process+0x900/0xe50 From: James Bottomley To: emilne@redhat.com Cc: Andrea Gelmini , linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org, dgilbert@interlog.com Date: Fri, 04 Dec 2015 11:16:54 -0800 In-Reply-To: <1449248315.4067.140.camel@localhost.localdomain> References: <20151201202053.GA4753@glen> <1449097101.2203.67.camel@HansenPartnership.com> <20151203203656.GA9278@glen> <1449176346.2187.19.camel@HansenPartnership.com> <20151203222004.GA22311@glen> <1449248315.4067.140.camel@localhost.localdomain> X-Mailer: Evolution 3.12.11 Mime-Version: 1.0 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, 2015-12-04 at 11:58 -0500, Ewan Milne wrote: > On Thu, 2015-12-03 at 23:20 +0100, Andrea Gelmini wrote: > > On Thu, Dec 03, 2015 at 12:59:06PM -0800, James Bottomley wrote: > > > sg_map -i > > > > > > in your system, you should see something with an inquiry string like > > > enclosure. It's the /dev/sg of that you need to run sg_ses on. > > > > root@glen:/home/gelma# sg_map -i > > /dev/sg0 /dev/sda ATA Samsung SSD 850 1B6Q > > /dev/sg1 /dev/sr0 HL-DT-ST DVDRAM GU40N QX23 > > /dev/sg2 /dev/sdb WD My Passport 0820 1007 > > /dev/sg3 WD SES Device 1007 > > > > And following Douglas' instructions: > > > > root@glen:/home/gelma# lsscsi -gs > > [0:0:0:0] disk ATA Samsung SSD 850 1B6Q /dev/sda /dev/sg0 1.02TB > > [1:0:0:0] cd/dvd HL-DT-ST DVDRAM GU40N QX23 /dev/sr0 /dev/sg1 - > > [8:0:0:0] disk WD My Passport 0820 1007 /dev/sdb /dev/sg2 2.00TB > > [8:0:0:1] enclosu WD SES Device 1007 - /dev/sg3 > > > > root@glen:/home/gelma# sg_ses /dev/sg3 > > WD SES Device 1007 > > Supported diagnostic pages: > > Supported Diagnostic Pages [sdp] [0x0] > > Short Enclosure Status (SES) [ses] [0x8] > > [0x80] > > [0x83] > > [0x84] > > [0x85] > > > > > > Well, if it's better for you, I can give you root access to a machine with this device > > connected to. > > > > Thanks a lot for your time, > > Andrea > > There seems to be a problem with the ses code if the device only reports > Short Enclosure Status. We probably need to do something like: > > diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c > index eba183c..065a528 100644 > --- a/drivers/scsi/ses.c > +++ b/drivers/scsi/ses.c > @@ -537,6 +537,15 @@ static int ses_intf_add(struct device *cdev, > if (result) > goto recv_failed; > > + /* If enclosure only supports Short Enclosure Status page (08), > + * it returns that page regardless of what we requested, and > + * only returns vendor-specific status. There is nothing wrong > + * with such enclosures, we just can't make use of them. */ > + if (hdr_buf[0] == 8) { > + err = -ENODEV; > + goto err_init_free_nomsg; > + } > + > len = (hdr_buf[2] << 8) + hdr_buf[3] + 4; > buf = kzalloc(len, GFP_KERNEL); > if (!buf) > @@ -646,9 +655,10 @@ static int ses_intf_add(struct device *cdev, > kfree(ses_dev->page2); > kfree(ses_dev->page1); > err_init_free: > + sdev_printk(KERN_ERR, sdev, "Failed to bind enclosure %d\n", > err); > + err_init_free_nomsg: > kfree(ses_dev); > kfree(hdr_buf); > - sdev_printk(KERN_ERR, sdev, "Failed to bind enclosure %d\n", > err); > return err; > } > > Otherwise we go off issuing commands for pages the device says it won't > return. I don't see offhand how this would cause KASAN errors though. I think we have two separate bugs. One is the usual USB devices getting into a new SCSI standard and getting it wrong. The other looks to be an enumeration problem ... possibly because ses2 added an indexed descriptor which current SES doesn't cope with. Anyway, in concentrating on the USB problem first, I think we need to code a little more defensively, like this. James --- -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index dcb0d76..7d9cec5 100644 --- a/drivers/scsi/ses.c +++ b/drivers/scsi/ses.c @@ -84,6 +84,7 @@ static void init_device_slot_control(unsigned char *dest_desc, static int ses_recv_diag(struct scsi_device *sdev, int page_code, void *buf, int bufflen) { + int ret; unsigned char cmd[] = { RECEIVE_DIAGNOSTIC, 1, /* Set PCV bit */ @@ -92,9 +93,26 @@ static int ses_recv_diag(struct scsi_device *sdev, int page_code, bufflen & 0xff, 0 }; + unsigned char recv_page_code; - return scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buf, bufflen, + ret = scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buf, bufflen, NULL, SES_TIMEOUT, SES_RETRIES, NULL); + if (unlikely(!ret)) + return ret; + + recv_page_code = ((unsigned char *)buf)[0]; + + if (likely(recv_page_code == page_code)) + return ret; + + /* successful diagnostic but wrong page code. This happens to some + * USB devices, just print a message and pretend there was an error */ + + sdev_printk(KERN_ERR, sdev, + "Wrong diagnostic page; asked for %d got %u\n", + page_code, recv_page_code); + + return -EINVAL; } static int ses_send_diag(struct scsi_device *sdev, int page_code,