Received: by 2002:a25:7ec1:0:0:0:0:0 with SMTP id z184csp5996361ybc; Wed, 27 Nov 2019 13:06:52 -0800 (PST) X-Google-Smtp-Source: APXvYqwCAZCpL3Gmgd5iavCD2wMEIC0GfIqYfzPJCFVvNYRhEpNe8wkuhzPyvnDIgDxfJwOknQWg X-Received: by 2002:a50:ef08:: with SMTP id m8mr34972144eds.14.1574888811991; Wed, 27 Nov 2019 13:06:51 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1574888811; cv=none; d=google.com; s=arc-20160816; b=EIj/VeTzSDPHLnLgHrnq+MuE/WAOIvinMXtcdqb3salvkewAaoTWPe0a1FbfCTQwTr IMIS2E1j4oDktyPXvR1UgbkJHKTYRnmhVy4tNX8s0pBMwlZtA80566K7+xMnanHuqte+ Vsx25D1RxtSSh9EGhoLLb3rMEwaaUMswcg2o0KPgvoT87EEVqzuL2w5GcLxCz/CClpQC sQinrDh3rle6RSrFltRNe/j5CEldErXEcgrqCF1qxgCS4qgIjTphHt6JnQr6wKS5OWjI 4Z6Tnw+8L8XvYkd7shEN66/cOTJYuQQZkrZv94Z3VYojzlqW3jcYWdYhrlmqDTPVL4MM 2Wmw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=GJtg2ugaTUaZ9VA40pbAAHV27DHEO5DKjHLcyUMcJ34=; b=WRxEtqLH/whSI05YE5MAB40n3Qr0yQrokLq3cxos0PKhAHrk3/Q1cq3E4N5JpnV/AY p4YDWmmnlKo25w0/2cavN6O1FONQcD7iLdnyDrHJZYrAUIXojks22lrjNu5edf6zwa5W DM1hiiMLrelob6mvHS4p6s2J5sYqtc70rVbrbUeTkGQbc5lHHbSkwLptpYJfHbVhpzxP nxt424n1HtzUlxlr5X15YXvMzSEMVIdDj7ZVnv2F5x744ezRzBU7MK/Y/EWRiNfXGeZE 57s6/PTl4QQChjh0/3OV9ohrpC4RzYWUzKkD4XRrKEAQMyYHKz2Ow8j/abliUqIjAacU ALLA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=ualKKvp7; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id v23si10717615edy.287.2019.11.27.13.06.28; Wed, 27 Nov 2019 13:06:51 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=ualKKvp7; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732035AbfK0VD1 (ORCPT + 99 others); Wed, 27 Nov 2019 16:03:27 -0500 Received: from mail.kernel.org ([198.145.29.99]:56546 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732026AbfK0VDV (ORCPT ); Wed, 27 Nov 2019 16:03:21 -0500 Received: from localhost (83-86-89-107.cable.dynamic.v4.ziggo.nl [83.86.89.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 3665B21775; Wed, 27 Nov 2019 21:03:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1574888600; bh=Vt4u48HOCbcz3ybOMB9dVcvqoXTpvruV+PEMafpIE0E=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ualKKvp77mL36PxAT5m45uy9R4tWnnntPbhQ36OoZeBiuTSsf87oy6chkmZ4mGbkO 9DtNsn9ke8EpQByG4FUCqQLxlZiFbFUxgypQC524NBwkMSRy8R8SGAAiIpBhIS8qp4 d3k5zdAiDLue6kQULb6V4rsP3dhKhXNfaXdG/s9Q= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Changwei Ge , Guozhonghua , Mark Fasheh , Joel Becker , Junxiao Bi , Joseph Qi , Andrew Morton , Linus Torvalds , Sasha Levin Subject: [PATCH 4.19 198/306] ocfs2: dont put and assigning null to bh allocated outside Date: Wed, 27 Nov 2019 21:30:48 +0100 Message-Id: <20191127203129.655847876@linuxfoundation.org> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191127203114.766709977@linuxfoundation.org> References: <20191127203114.766709977@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Changwei Ge [ Upstream commit cf76c78595ca87548ca5e45c862ac9e0949c4687 ] ocfs2_read_blocks() and ocfs2_read_blocks_sync() are both used to read several blocks from disk. Currently, the input argument *bhs* can be NULL or NOT. It depends on the caller's behavior. If the function fails in reading blocks from disk, the corresponding bh will be assigned to NULL and put. Obviously, above process for non-NULL input bh is not appropriate. Because the caller doesn't even know its bhs are put and re-assigned. If buffer head is managed by caller, ocfs2_read_blocks and ocfs2_read_blocks_sync() should not evaluate it to NULL. It will cause caller accessing illegal memory, thus crash. Link: http://lkml.kernel.org/r/HK2PR06MB045285E0F4FBB561F9F2F9B3D5680@HK2PR06MB0452.apcprd06.prod.outlook.com Signed-off-by: Changwei Ge Reviewed-by: Guozhonghua Cc: Mark Fasheh Cc: Joel Becker Cc: Junxiao Bi Cc: Joseph Qi Cc: Changwei Ge Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds Signed-off-by: Sasha Levin --- fs/ocfs2/buffer_head_io.c | 77 ++++++++++++++++++++++++++++++--------- 1 file changed, 59 insertions(+), 18 deletions(-) diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c index 9f8250df99f1f..f9b84f7a3e4bb 100644 --- a/fs/ocfs2/buffer_head_io.c +++ b/fs/ocfs2/buffer_head_io.c @@ -99,25 +99,34 @@ int ocfs2_write_block(struct ocfs2_super *osb, struct buffer_head *bh, return ret; } +/* Caller must provide a bhs[] with all NULL or non-NULL entries, so it + * will be easier to handle read failure. + */ int ocfs2_read_blocks_sync(struct ocfs2_super *osb, u64 block, unsigned int nr, struct buffer_head *bhs[]) { int status = 0; unsigned int i; struct buffer_head *bh; + int new_bh = 0; trace_ocfs2_read_blocks_sync((unsigned long long)block, nr); if (!nr) goto bail; + /* Don't put buffer head and re-assign it to NULL if it is allocated + * outside since the caller can't be aware of this alternation! + */ + new_bh = (bhs[0] == NULL); + for (i = 0 ; i < nr ; i++) { if (bhs[i] == NULL) { bhs[i] = sb_getblk(osb->sb, block++); if (bhs[i] == NULL) { status = -ENOMEM; mlog_errno(status); - goto bail; + break; } } bh = bhs[i]; @@ -157,9 +166,26 @@ int ocfs2_read_blocks_sync(struct ocfs2_super *osb, u64 block, submit_bh(REQ_OP_READ, 0, bh); } +read_failure: for (i = nr; i > 0; i--) { bh = bhs[i - 1]; + if (unlikely(status)) { + if (new_bh && bh) { + /* If middle bh fails, let previous bh + * finish its read and then put it to + * aovoid bh leak + */ + if (!buffer_jbd(bh)) + wait_on_buffer(bh); + put_bh(bh); + bhs[i - 1] = NULL; + } else if (bh && buffer_uptodate(bh)) { + clear_buffer_uptodate(bh); + } + continue; + } + /* No need to wait on the buffer if it's managed by JBD. */ if (!buffer_jbd(bh)) wait_on_buffer(bh); @@ -169,8 +195,7 @@ int ocfs2_read_blocks_sync(struct ocfs2_super *osb, u64 block, * so we can safely record this and loop back * to cleanup the other buffers. */ status = -EIO; - put_bh(bh); - bhs[i - 1] = NULL; + goto read_failure; } } @@ -178,6 +203,9 @@ int ocfs2_read_blocks_sync(struct ocfs2_super *osb, u64 block, return status; } +/* Caller must provide a bhs[] with all NULL or non-NULL entries, so it + * will be easier to handle read failure. + */ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr, struct buffer_head *bhs[], int flags, int (*validate)(struct super_block *sb, @@ -187,6 +215,7 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr, int i, ignore_cache = 0; struct buffer_head *bh; struct super_block *sb = ocfs2_metadata_cache_get_super(ci); + int new_bh = 0; trace_ocfs2_read_blocks_begin(ci, (unsigned long long)block, nr, flags); @@ -212,6 +241,11 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr, goto bail; } + /* Don't put buffer head and re-assign it to NULL if it is allocated + * outside since the caller can't be aware of this alternation! + */ + new_bh = (bhs[0] == NULL); + ocfs2_metadata_cache_io_lock(ci); for (i = 0 ; i < nr ; i++) { if (bhs[i] == NULL) { @@ -220,7 +254,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr, ocfs2_metadata_cache_io_unlock(ci); status = -ENOMEM; mlog_errno(status); - goto bail; + /* Don't forget to put previous bh! */ + break; } } bh = bhs[i]; @@ -314,16 +349,27 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr, } } - status = 0; - +read_failure: for (i = (nr - 1); i >= 0; i--) { bh = bhs[i]; if (!(flags & OCFS2_BH_READAHEAD)) { - if (status) { - /* Clear the rest of the buffers on error */ - put_bh(bh); - bhs[i] = NULL; + if (unlikely(status)) { + /* Clear the buffers on error including those + * ever succeeded in reading + */ + if (new_bh && bh) { + /* If middle bh fails, let previous bh + * finish its read and then put it to + * aovoid bh leak + */ + if (!buffer_jbd(bh)) + wait_on_buffer(bh); + put_bh(bh); + bhs[i] = NULL; + } else if (bh && buffer_uptodate(bh)) { + clear_buffer_uptodate(bh); + } continue; } /* We know this can't have changed as we hold the @@ -341,9 +387,7 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr, * uptodate. */ status = -EIO; clear_buffer_needs_validate(bh); - put_bh(bh); - bhs[i] = NULL; - continue; + goto read_failure; } if (buffer_needs_validate(bh)) { @@ -353,11 +397,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr, BUG_ON(buffer_jbd(bh)); clear_buffer_needs_validate(bh); status = validate(sb, bh); - if (status) { - put_bh(bh); - bhs[i] = NULL; - continue; - } + if (status) + goto read_failure; } } -- 2.20.1