From patchwork Fri Oct 30 23:57:00 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Calvin Owens X-Patchwork-Id: 7530111 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 BB7639F399 for ; Fri, 30 Oct 2015 23:57:51 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id E0BEE206FC for ; Fri, 30 Oct 2015 23:57:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id F1618206EF for ; Fri, 30 Oct 2015 23:57:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751848AbbJ3X5e (ORCPT ); Fri, 30 Oct 2015 19:57:34 -0400 Received: from mx0a-00082601.pphosted.com ([67.231.145.42]:37370 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750834AbbJ3X5d (ORCPT ); Fri, 30 Oct 2015 19:57:33 -0400 Received: from pps.filterd (m0044008.ppops.net [127.0.0.1]) by mx0a-00082601.pphosted.com (8.15.0.59/8.15.0.59) with SMTP id t9UNrtNB003974; Fri, 30 Oct 2015 16:57:28 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=fb.com; h=from : to : cc : subject : date : message-id : mime-version : content-type; s=facebook; bh=hFidItMPQaAIsiMqgkUALpSMJVbB2ftOZl9sJUgV4nM=; b=qRgxlBYW+rz7D564aaGMKaOJs61aJBZZY+UZSKnu14bqZMDBdpKmL7Wup+SfRTFLZ0JH aUln/bjKNTyEwUXTNc7vBb/EKFVeIIzj/Tz2WATL4lQHj0KV2oyjxMzJUcEG9DWRdI6M qXKMgJPhiV8/nODk1K4HVMRAVfJwaqC/VQk= Received: from mail.thefacebook.com ([199.201.64.23]) by mx0a-00082601.pphosted.com with ESMTP id 1xvhnyr6bq-2 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NOT); Fri, 30 Oct 2015 16:57:27 -0700 Received: from shellserver002.frc3.facebook.com (192.168.52.123) by PRN-CHUB05.TheFacebook.com (192.168.16.15) with Microsoft SMTP Server id 14.3.248.2; Fri, 30 Oct 2015 16:57:14 -0700 From: Calvin Owens To: Doug Gilbert , "James E.J. Bottomley" CC: , , , Subject: [PATCH] sg: Fix double-free when drives detach during SG_IO Date: Fri, 30 Oct 2015 16:57:00 -0700 Message-ID: <944215ec334c3c5b3af98210acf8f6479f5539c8.1445634103.git.calvinowens@fb.com> X-Mailer: git-send-email 2.4.6 MIME-Version: 1.0 X-Originating-IP: [192.168.52.123] X-Proofpoint-Spam-Reason: safe X-FB-Internal: Safe X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2015-10-31_01:, , signatures=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=-7.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,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 In sg_common_write(), we free the block request and return -ENODEV if the device is detached in the middle of the SG_IO ioctl(). Unfortunately, sg_finish_rem_req() also tries to free srp->rq, so we end up freeing rq->cmd in the already free rq object, and then free the object itself out from under the current user. This ends up corrupting random memory via the list_head on the rq object. The most common crash trace I saw is this: ------------[ cut here ]------------ kernel BUG at block/blk-core.c:1420! Call Trace: [] blk_put_request+0x5b/0x80 [] sg_finish_rem_req+0x6b/0x120 [sg] [] sg_common_write.isra.14+0x459/0x5a0 [sg] [] ? selinux_file_alloc_security+0x48/0x70 [] sg_new_write.isra.17+0x195/0x2d0 [sg] [] sg_ioctl+0x644/0xdb0 [sg] [] do_vfs_ioctl+0x90/0x520 [] ? file_has_perm+0x97/0xb0 [] SyS_ioctl+0x91/0xb0 [] tracesys+0xdd/0xe2 RIP [] __blk_put_request+0x154/0x1a0 The solution is straightforward: just set srp->rq to NULL in the failure branch so that sg_finish_rem_req() doesn't attempt to re-free it. Additionally, since sg_rq_end_io() will never be called on the object when this happens, we need to free memory backing ->cmd if it isn't embedded in the object itself. KASAN was extremely helpful in finding the root cause of this bug. Signed-off-by: Calvin Owens Acked-by: Douglas Gilbert --- drivers/scsi/sg.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c index 9d7b7db..503ab8b 100644 --- a/drivers/scsi/sg.c +++ b/drivers/scsi/sg.c @@ -787,8 +787,14 @@ sg_common_write(Sg_fd * sfp, Sg_request * srp, return k; /* probably out of space --> ENOMEM */ } if (atomic_read(&sdp->detaching)) { - if (srp->bio) + if (srp->bio) { + if (srp->rq->cmd != srp->rq->__cmd) + kfree(srp->rq->cmd); + blk_end_request_all(srp->rq, -EIO); + srp->rq = NULL; + } + sg_finish_rem_req(srp); return -ENODEV; }