From patchwork Wed May 23 21:17:14 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 10422423 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 0E7616032C for ; Wed, 23 May 2018 21:17:25 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id F1FB928BCC for ; Wed, 23 May 2018 21:17:24 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E686A2925D; Wed, 23 May 2018 21:17:24 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4951328BCC for ; Wed, 23 May 2018 21:17:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934308AbeEWVRV (ORCPT ); Wed, 23 May 2018 17:17:21 -0400 Received: from mail-ua0-f195.google.com ([209.85.217.195]:46047 "EHLO mail-ua0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934077AbeEWVRQ (ORCPT ); Wed, 23 May 2018 17:17:16 -0400 Received: by mail-ua0-f195.google.com with SMTP id j5-v6so15728463uak.12 for ; Wed, 23 May 2018 14:17:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=48uapMwOuQrDu7yg515Wrcrf2lIASzvw0N4U/ja+I4M=; b=u3MJ5mdawgNfqYQx9uvJLKpMy6f2Y7L/rCjp1upsMwCkdxy8S/g0yG4zdGIWl6rnhO 8Q3vyT9ujtuDkBLAWAeiz8F4QLSUNYTYKOqfi9BGOJlmn6j7POciVg3xhfAV9InuPb1u bUD4Md6tk5887VsUEs0kj9qOXjxHdwMo+XNjrQA5t9r82ZhkM7du7tOGTyQrmcsDPmcB vadnUjCbzqnnwcnLYDmF51wzU1qxVnsgpbW12Tw1i4QiUGtf2deQcL8yFAxcPxah6pVu epTZibOjjYPCB4DfYRPHqirXghB6EnI82UVafD78/6nshkD+nuxKseHuoEm5zDU0IyeP wH+Q== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=48uapMwOuQrDu7yg515Wrcrf2lIASzvw0N4U/ja+I4M=; b=BKbPdS40nSPpN0Qggg2+DsY58qfRoeRY4qRLHs5jIGdLzLejLq5VwuyHFlkBs2vjoF binmk13PMRha7WFJbbRLFSzNM3id/lVHmXcrcFtrG822l6MxDPZ3vmwZXyL4DHYX7I5Q BG6krm4lNdbjYcWpFr/WA5TjK++GuqjZ6kcDA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=48uapMwOuQrDu7yg515Wrcrf2lIASzvw0N4U/ja+I4M=; b=mh1Lq8osH5Z/tKnt5XX6sFn1UimuEaQHKDuvM/OFvTMKKPt8VcY1nG/ytJAO2IK4HM oTakfdcElDNqT06whG9+LSD+TxgGsm4Y59YdVueZb5nvoKgUS/Rj+NMFqjhNpPBuqgev KTa8LGGoJiRRIg/xw70CMTmtGVp5NvIz8HMlbV9klOA5c+GIRuMoHiAzDDo7Nz3Inth7 jYK3RuC1op/lhh5cxJleJ3LErHtXZKAeg1JxrAfCoapVuKXuNRX8xzoY+PbLUQx4mAv+ sUSuyN9UBIoW9+GkMmK2t/DLEbP/LFuvVa0ukYT2gxsIptYmIUXnOUCwibfDc/9GyIV/ 6leA== X-Gm-Message-State: ALKqPweRmMr3tOBa9ozScbCQj4CxECajcd0wiv/Z2QIjsrePyx2lKwWJ CzNLfTyutIcrScQcBy3sKboIL/ZHHqQA5ekZd68KPA== X-Google-Smtp-Source: AB8JxZplOdmi82een3ndt+Z9gQvQmF5hwZsSLjq5xQKJhKL/YAcsaqyd3ZtGfvsLeDFxqFngrbPBvyfA8p3jPVt6hmw= X-Received: by 2002:a9f:2823:: with SMTP id c32-v6mr3254583uac.193.1527110235007; Wed, 23 May 2018 14:17:15 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a1f:bd1:0:0:0:0:0 with HTTP; Wed, 23 May 2018 14:17:14 -0700 (PDT) In-Reply-To: References: <20180522183613.GA3784@infradead.org> <732f4249-5681-4a54-ec21-4ecc3d3a74e5@kernel.dk> <20180522191309.GA23615@infradead.org> <8d4af5c4-96fa-54ee-d5c1-b887b1de5a3c@kernel.dk> <9A0BC289-4203-4C77-A012-AAB07F42061F@kernel.dk> <20180523142545.GA16248@infradead.org> <24d36869-e037-042d-cb16-20a81b34eb76@kernel.dk> From: Kees Cook Date: Wed, 23 May 2018 14:17:14 -0700 X-Google-Sender-Auth: yXx59FbAikDTjQXloBUdgYcI1G8 Message-ID: Subject: Re: [PATCH 3/6] block: Create scsi_sense.h for SCSI and ATAPI To: "Martin K. Petersen" Cc: Jens Axboe , Christoph Hellwig , James Bottomley , Tejun Heo , Borislav Petkov , "David S. Miller" , "Manoj N. Kumar" , "Matthew R. Ochs" , Uma Krishnan , linux-block , linux-ide@vger.kernel.org, linux-scsi@vger.kernel.org, LKML Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, May 23, 2018 at 2:06 PM, Martin K. Petersen wrote: > > Kees, > >> obj-$(CONFIG_SCSI) += scsi/ >> >> So: this needs to live in block/ just like CONFIG_BLK_SCSI_REQUEST's >> scsi_ioctl.c. I will split it into CONFIG_BLK_SCSI_SENSE, but I'll >> still need to move the code from drivers/scsi/ to block/. Is this >> okay? > > The reason this sucks is that scsi_normalize_sense() is an inherent core > feature in the SCSI layer. Dealing with sense data for ioctls is just a > fringe use case. True, though I'm finding other robustness issues in the CDROM code. They're probably all insane corner cases, but it seems like it'd be nice to just fix them: if (blk_rq_unmap_user(bio)) This code wasn't checking req->sense_len, for example. It'll just get stale data at worst case, but it's still ugly, especially since we have a solution to do it right. > I don't want to get too hung up on what goes where. But architecturally > it really irks me to move a core piece of SCSI state machine > functionality out of the subsystem to accommodate ioctl handling. It looks like there is more in block/scsi_ioctl.c than just ioctl handling, which is why I put the function in there originally. Honestly, it's almost so small I could make it a static inline. :P > I'm traveling today so I probably won't get a chance to look closely > until tomorrow morning. No worries; thanks for looking at it! -Kees diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c index 3522d2cae1b6..7726c8618c30 100644 --- a/drivers/cdrom/cdrom.c +++ b/drivers/cdrom/cdrom.c @@ -2222,9 +2223,12 @@ static int cdrom_read_cdda_bpc(struct cdrom_device_info *cdi, __u8 __user *ubuf, blk_execute_rq(q, cdi->disk, rq, 0); if (scsi_req(rq)->result) { - struct request_sense *s = req->sense; + struct scsi_sense_hdr sshdr; + ret = -EIO; - cdi->last_sense = s->sense_key; + scsi_normalize_sense(req->sense, req->sense_len, + &sshdr); + cdi->last_sense = sshdr.sense_key; }