From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753186AbeDQVs1 (ORCPT ); Tue, 17 Apr 2018 17:48:27 -0400 Received: from mail-pg0-f43.google.com ([74.125.83.43]:43386 "EHLO mail-pg0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753133AbeDQVsX (ORCPT ); Tue, 17 Apr 2018 17:48:23 -0400 X-Google-Smtp-Source: AIpwx481LgHZ+IGeoDWnrVazGYgdcCvFJg9XevFJntBtZH4ycyyLvRP3FHxizZ2m0PjXQiw1zFzb7A== Subject: Re: usercopy whitelist woe in scsi_sense_cache To: Kees Cook Cc: Paolo Valente , Oleksandr Natalenko , Bart Van Assche , David Windsor , "James E.J. Bottomley" , "Martin K. Petersen" , linux-scsi@vger.kernel.org, LKML , Christoph Hellwig , Hannes Reinecke , Johannes Thumshirn , linux-block@vger.kernel.org References: <10360653.ov98egbaqx@natalenko.name> <2864697.7uzmEJovl2@natalenko.name> <8473f909-2123-0cfc-43b1-beba0b1aef9b@kernel.dk> <07f263ff-cea6-ac3c-944b-0f36fee8ba25@kernel.dk> From: Jens Axboe Message-ID: <8b32e079-d4e6-3fea-a89d-ff856e4e13b1@kernel.dk> Date: Tue, 17 Apr 2018 15:48:19 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 4/17/18 3:47 PM, Kees Cook wrote: > On Tue, Apr 17, 2018 at 2:39 PM, Jens Axboe wrote: >> On 4/17/18 3:25 PM, Kees Cook wrote: >>> On Tue, Apr 17, 2018 at 1:46 PM, Kees Cook wrote: >>>> I see elv.priv[1] assignments made in a few places -- is it possible >>>> there is some kind of uninitialized-but-not-NULL state that can leak >>>> in there? >>> >>> Got it. This fixes it for me: >>> >>> diff --git a/block/blk-mq.c b/block/blk-mq.c >>> index 0dc9e341c2a7..859df3160303 100644 >>> --- a/block/blk-mq.c >>> +++ b/block/blk-mq.c >>> @@ -363,7 +363,7 @@ static struct request *blk_mq_get_request(struct >>> request_queue *q, >>> >>> rq = blk_mq_rq_ctx_init(data, tag, op); >>> if (!op_is_flush(op)) { >>> - rq->elv.icq = NULL; >>> + memset(&rq->elv, 0, sizeof(rq->elv)); >>> if (e && e->type->ops.mq.prepare_request) { >>> if (e->type->icq_cache && rq_ioc(bio)) >>> blk_mq_sched_assign_ioc(rq, bio); >>> @@ -461,7 +461,7 @@ void blk_mq_free_request(struct request *rq) >>> e->type->ops.mq.finish_request(rq); >>> if (rq->elv.icq) { >>> put_io_context(rq->elv.icq->ioc); >>> - rq->elv.icq = NULL; >>> + memset(&rq->elv, 0, sizeof(rq->elv)); >>> } >>> } >> >> This looks like a BFQ problem, this should not be necessary. Paolo, >> you're calling your own prepare request handler from the insert >> as well, and your prepare request does nothing if rq->elv.icq == NULL. > > I sent the patch anyway, since it's kind of a robustness improvement, > I'd hope. If you fix BFQ also, please add: It's also a memset() in the hot path, would prefer to avoid that... The issue here is really the convoluted bfq usage of insert/prepare, I'm sure Paolo can take it from here. -- Jens Axboe