From patchwork Sat May 26 15:31:58 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Changwei Ge X-Patchwork-Id: 10429031 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 1C4F860249 for ; Sat, 26 May 2018 15:34:08 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 09E5B294CC for ; Sat, 26 May 2018 15:34:08 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id F1C31294D1; Sat, 26 May 2018 15:34:07 +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=-5.2 required=2.0 tests=BAYES_00,FREEMAIL_FROM, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from userp2120.oracle.com (userp2120.oracle.com [156.151.31.85]) (using TLSv1.2 with cipher AES256-SHA256 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 03E4A294CD for ; Sat, 26 May 2018 15:34:05 +0000 (UTC) Received: from pps.filterd (userp2120.oracle.com [127.0.0.1]) by userp2120.oracle.com (8.16.0.22/8.16.0.22) with SMTP id w4QFQbOf067113; Sat, 26 May 2018 15:32:59 GMT Received: from userv0021.oracle.com (userv0021.oracle.com [156.151.31.71]) by userp2120.oracle.com with ESMTP id 2j7084s2c6-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Sat, 26 May 2018 15:32:58 +0000 Received: from oss.oracle.com (oss-old-reserved.oracle.com [137.254.22.2]) by userv0021.oracle.com (8.14.4/8.14.4) with ESMTP id w4QFWtuS012194 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 26 May 2018 15:32:55 GMT Received: from localhost ([127.0.0.1] helo=lb-oss.oracle.com) by oss.oracle.com with esmtp (Exim 4.63) (envelope-from ) id 1fMbBf-0006VE-8Q; Sat, 26 May 2018 08:32:55 -0700 Received: from userv0021.oracle.com ([156.151.31.71]) by oss.oracle.com with esmtp (Exim 4.63) (envelope-from ) id 1fMbAs-0006Tw-Is for ocfs2-devel@oss.oracle.com; Sat, 26 May 2018 08:32:07 -0700 Received: from userp2040.oracle.com (userp2040.oracle.com [156.151.31.90]) by userv0021.oracle.com (8.14.4/8.14.4) with ESMTP id w4QFW5Ac011172 (version=TLSv1/SSLv3 cipher=AES256-SHA256 bits=256 verify=FAIL) for ; Sat, 26 May 2018 15:32:06 GMT Received: from pps.filterd (userp2040.oracle.com [127.0.0.1]) by userp2040.oracle.com (8.16.0.22/8.16.0.22) with SMTP id w4QFTrgs024296 for ; Sat, 26 May 2018 15:32:05 GMT Received: from apc01-sg2-obe.outbound.protection.outlook.com (mail-oln040092253096.outbound.protection.outlook.com [40.92.253.96]) by userp2040.oracle.com with ESMTP id 2j6x7mgt7j-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=OK) for ; Sat, 26 May 2018 15:32:05 +0000 Received: from SG2APC01FT058.eop-APC01.prod.protection.outlook.com (10.152.250.55) by SG2APC01HT031.eop-APC01.prod.protection.outlook.com (10.152.251.9) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.20.696.11; Sat, 26 May 2018 15:31:58 +0000 Received: from HK2PR06MB0452.apcprd06.prod.outlook.com (10.152.250.51) by SG2APC01FT058.mail.protection.outlook.com (10.152.251.117) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.20.696.11 via Frontend Transport; Sat, 26 May 2018 15:31:58 +0000 Received: from HK2PR06MB0452.apcprd06.prod.outlook.com ([fe80::54f:c47d:f84b:5d1e]) by HK2PR06MB0452.apcprd06.prod.outlook.com ([fe80::54f:c47d:f84b:5d1e%5]) with mapi id 15.20.0797.011; Sat, 26 May 2018 15:31:58 +0000 From: Changwei Ge To: Andrew Morton , "ocfs2-devel@oss.oracle.com" , Guozhonghua , "mark@fasheh.com" , "jiangqi903@gmail.com" , "junxiao.bi@oracle.com" Thread-Topic: [PATCH v2] ocfs2: don't put and assigning null to bh allocated outside Thread-Index: AQHT9QavTSMeEcZlUESGMvQbymxHfQ== Date: Sat, 26 May 2018 15:31:58 +0000 Message-ID: Accept-Language: en-US, zh-CN Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-clientproxiedby: HK2PR04CA0051.apcprd04.prod.outlook.com (2603:1096:202:14::19) To HK2PR06MB0452.apcprd06.prod.outlook.com (2a01:111:e400:a41b::18) x-incomingtopheadermarker: OriginalChecksum:04DE47D8529A98C0EB094A813CC966D986E2393B72C239F408C9F311703D0B7F; UpperCasedChecksum:46F39DBD72172E92EF46DA11F42283F6B699B2FC4A90B2C0DEAFE3CFB290A3EF; SizeAsReceived:7601; Count:48 x-ms-exchange-messagesentrepresentingtype: 1 x-tmn: [0S5AEWzz8IvPoZKS0SV/L8P9sdDznXY+uyiGZ6qAwGQ=] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; SG2APC01HT031; 7:NV5p9le0ZSuU0PExjQLLcUH54bc5IVv9Dh+wUIhnv7elLJ5CsTgjgrd+ABmkKWmxgF7XLQtacvKAqmmQ4ZoQNGUM3DjvHTD3H32LkDTDfLabEUKNL7aFIbZx1zk0DlucirQEagYlzOiHTc6CTI/MVUrVVdaTpWFz0Hs6pEFyjLKwB92ncHxGjnpCd76jU7hvIYtwiCj1lRUn4puG/1GadvziUGs5b0jBVS3Md4KgQ7gdwY9H6HusvUpErX3wPUV5 x-incomingheadercount: 48 x-eopattributedmessage: 0 x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(201702061078)(5061506573)(5061507331)(1603103135)(2017031320274)(2017031324274)(2017031323274)(2017031322404)(1603101448)(1601125466)(1701031045); SRVR:SG2APC01HT031; x-ms-traffictypediagnostic: SG2APC01HT031: x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(444000031); SRVR:SG2APC01HT031; BCL:0; PCL:0; RULEID:; SRVR:SG2APC01HT031; x-forefront-prvs: 0684F80A5C x-forefront-antispam-report: SFV:NSPM; SFS:(7070007)(189003)(199004)(6506007)(106356001)(81612004)(6346003)(74482002)(386003)(82202002)(102836004)(33656002)(7696005)(105586002)(55016002)(59450400001)(2900100001)(6436002)(81156014)(14454004)(68736007)(110136005)(25786009)(5660300001)(8676002)(97736004)(9686003)(305945005)(476003)(3280700002)(486006)(4326008)(74316002)(104016004)(2201001)(99286004)(2501003)(5250100002)(86362001)(3660700001)(8936002)(26005)(39060400002); DIR:OUT; SFP:1901; SCL:1; SRVR:SG2APC01HT031; H:HK2PR06MB0452.apcprd06.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; MX:1; A:1; LANG:; received-spf: None (protection.outlook.com: live.cn does not designate permitted sender hosts) x-microsoft-antispam-message-info: EUp9rbVqt7QmdKzIPa6Oft1R9Uc8D1pDWavt9+HYDQCHx22+kP+4x/NgIr44hTIK+yYazVovUSkfKXJWQiG+8nAP7GxACreL/NTnBVbNZKppW25L93Hc8vKM5V1asveFbRzzLXKGdZ42UkSlbrsisj5fSxlNJQVC0TrMcPest//Iggrk3n8OXnjjFIQIQe++ MIME-Version: 1.0 X-MS-Office365-Filtering-Correlation-Id: 4dbc7e9b-a2a3-4b00-99c0-08d5c31dd159 X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: d4d70346-2c10-4f39-8c00-e767963926d9 X-MS-Exchange-CrossTenant-Network-Message-Id: 4dbc7e9b-a2a3-4b00-99c0-08d5c31dd159 X-MS-Exchange-CrossTenant-rms-persistedconsumerorg: d4d70346-2c10-4f39-8c00-e767963926d9 X-MS-Exchange-CrossTenant-originalarrivaltime: 26 May 2018 15:31:58.1694 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Internet X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-Transport-CrossTenantHeadersStamped: SG2APC01HT031 X-CLX-Shades: MLX X-CLX-Response: 1TFkXGxkSEQpMehcaEQpZTRdnZnIRCllJFxpxGhAadwYbGxJxGx4QGncGGBo GGhEKWV4XaG55EQpJRhdFWEtJRk91WlhFTl9JXkNFRBl1T0sRCkNOFwdCZgdPaBtgfBNaB3NabG 1/GhsSaxlbYWQeeR97YXJvEQpYXBcfBBoEGxkfB0kaHhNOHBJJBRsaBBsaGgQeEgQfEBseGh8aE QpeWRd5elNhexEKTVwXHBscEQpMWhdoaUJrexEKTU4XaBEKTEYXY2sRCkNaFx4aBBMYBBgfGQQT HBEKQl4XGxEKRF4XGxoRCkRJFxsRCkJGF2BTbh5DGXNnWEZmEQpCXBcaEQpCRRdhRlhrHWFQEm9 oTxEKQk4XYHgeW1JaW38dfkARCkJMF29cGXwFRXpQHUhIEQpCbBdkYU9LYEJIEngdZxEKQkAXbm lIbx0eW2ZPGFARCkJYF2J9b3kBTxgZcHB7EQpaWBcYEQpwZxdvYRxZZ1IfehJsGBAZGhEKcGgXb xIYSxt8TR9FaUEQGRoRCnBoF2xAS18bWm54fWMdEBkaEQpwaBduUFhBW1N4X2UZXBAZGhEKcGgX b01GZ2NHWB9OUGIQGRoRCnBoF2NZWWEcfW4fWWIBEBkaEQpwbBdtThtvUwFHUkgdcxAZGhEKbX4 XGhEKWE0XSxEg X-PDR: PASS X-Source-IP: 40.92.253.96 X-ServerName: mail-oln040092253096.outbound.protection.outlook.com X-Proofpoint-SPF-Result: pass X-Proofpoint-SPF-Record: v=spf1 include:spf-a.hotmail.com include:spf-b.hotmail.com include:spf-c.hotmail.com include:spf-d.hotmail.com ~all X-Proofpoint-Virus-Version: vendor=nai engine=5900 definitions=8905 signatures=668702 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 priorityscore=0 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=138 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1711220000 definitions=main-1805260182 X-Spam: Clean Subject: [Ocfs2-devel] [PATCH v2] ocfs2: don't put and assigning null to bh allocated outside X-BeenThere: ocfs2-devel@oss.oracle.com X-Mailman-Version: 2.1.9 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: ocfs2-devel-bounces@oss.oracle.com Errors-To: ocfs2-devel-bounces@oss.oracle.com X-Proofpoint-Virus-Version: vendor=nai engine=5900 definitions=8905 signatures=668702 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 mlxscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1711220000 definitions=main-1805260182 X-Virus-Scanned: ClamAV using ClamSMTP From: Changwei Ge 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. change from v1: Fix misuse of bh, which may cause NULL pointer accessing. Signed-off-by: Changwei Ge --- 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 d9ebe11..9f0304f 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]; @@ -158,9 +167,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); @@ -170,8 +196,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; } } @@ -179,6 +204,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, @@ -188,6 +216,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); @@ -213,6 +242,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) { @@ -221,7 +255,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]; @@ -316,16 +351,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 @@ -342,9 +388,7 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr, * for this bh as it's not marked locally * uptodate. */ status = -EIO; - put_bh(bh); - bhs[i] = NULL; - continue; + goto read_failure; } if (buffer_needs_validate(bh)) { @@ -354,11 +398,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; } }