Received: by 2002:a25:8b12:0:0:0:0:0 with SMTP id i18csp1442147ybl; Sun, 18 Aug 2019 03:41:07 -0700 (PDT) X-Google-Smtp-Source: APXvYqzSK72I2GZkKSFzSAg/reg91Q8ACECmhmnf9hM+EFIcJW/OiQZcosHt6kh5PDMsTyFtUX57 X-Received: by 2002:a17:902:9a07:: with SMTP id v7mr17823585plp.245.1566124867045; Sun, 18 Aug 2019 03:41:07 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1566124867; cv=none; d=google.com; s=arc-20160816; b=TB1t0f+HodAsMGphtQZioCCcl2qv6aCfqBQJzdYS3N2o7Fd7NU6H7N0pB8zQDy17lr D7z92ldpiDuxoEMm/x2CjP64/OBETQbkmHUioX3ZQ92Q54qeRomdFC6XoVyx52HIHo7w ffWHuOaLtxHInDsOx68aq/9r6OIEhTi2gdjRbfLQP5Vxz8QGLpX42LWYhxCy0Yr90igc mOKvLwNqiFiTbAG8XSnFC8M9lncPXf7Vr7o1sB62aAvSaEI0MkoIszH3ucW1tvYgFZCS fPJnATbB3JivmgEHK9pTDFZbxr6E+qXEtM7/xW5kuYgNDtCtfgCUGJNokFcDesVOb5gp d3sQ== 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 :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject:dkim-signature; bh=Uw3GHIRsELVaG/LPw4AMHfTYPoP8WVXsEJ8n7NO/iIs=; b=yx0jewCnPlGJjBaqG9fdzkAVqgEtaT68qdAxuHWrQwNzrTim9PmbCZvHX3Q8S+W41l QFAz1hQSQI9JnuKwWyQ0ldLpRxjrI2pvICJ1WmtxLzihgJtLVSE83ySIUxWbrU1ninEg FWlnrXNOudQ5KJrvcISZBRqlArMP0DUPvcnjn9T7WYlNBrv/FyEpKxoOn+3Pvl9ZUOCI QL5i2bk3R/MUrtrmsrflFacwXi4eHsMzSmHDn2k7PN7yU1vyjEFKzq6mt3X18qz5a9T5 QTuRUFGMAOncO+UA4digo8H7B/KxcBufgwnqCTxMdDYuUhSoN2Zzlp67kcFEvlBYoeYS +s7Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=CsD7cjaN; 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; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id a65si7545055pgc.213.2019.08.18.03.40.51; Sun, 18 Aug 2019 03:41:07 -0700 (PDT) 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=CsD7cjaN; 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; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726595AbfHRKkA (ORCPT + 99 others); Sun, 18 Aug 2019 06:40:00 -0400 Received: from mail.kernel.org ([198.145.29.99]:37374 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726089AbfHRKkA (ORCPT ); Sun, 18 Aug 2019 06:40:00 -0400 Received: from [192.168.0.101] (unknown [180.111.132.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id CB5402173B; Sun, 18 Aug 2019 10:39:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1566124798; bh=NZToQaeJzz8Ty+Y0B57dsWw46aBahEhnOAkutIVO54Y=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=CsD7cjaN8Kycm4SVNR7vGn1g2VrzQcmT40WDMLnYfOS6oeI6RVJVrJd+vn4F/oW5e Lonw4lm85ZI/xRvSs4p4tNebnaWPfW2rGYI6V5fcdfC8IXMnZiyPxB4kiCybncg2rf NoXjpYAKkJLM7wzG/WfrfT3xQ8H9xja220BFAKmE= Subject: Re: [PATCH v2] staging: erofs: fix an error handling in erofs_readdir() To: Matthew Wilcox , Gao Xiang Cc: Chao Yu , Richard Weinberger , Greg Kroah-Hartman , devel@driverdev.osuosl.org, linux-fsdevel@vger.kernel.org, LKML , linux-erofs@lists.ozlabs.org, Miao Xie , Fang Wei , Gao Xiang , stable@vger.kernel.org References: <20190818014835.5874-1-hsiangkao@aol.com> <20190818015631.6982-1-hsiangkao@aol.com> <20190818022055.GA14592@bombadil.infradead.org> <20190818023240.GA7739@hsiangkao-HP-ZHAN-66-Pro-G1> <20190818025339.GB14592@bombadil.infradead.org> From: Chao Yu Message-ID: Date: Sun, 18 Aug 2019 18:39:52 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20190818025339.GB14592@bombadil.infradead.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2019-8-18 10:53, Matthew Wilcox wrote: > On Sun, Aug 18, 2019 at 10:32:45AM +0800, Gao Xiang wrote: >> On Sat, Aug 17, 2019 at 07:20:55PM -0700, Matthew Wilcox wrote: >>> On Sun, Aug 18, 2019 at 09:56:31AM +0800, Gao Xiang wrote: >>>> @@ -82,8 +82,12 @@ static int erofs_readdir(struct file *f, struct dir_context *ctx) >>>> unsigned int nameoff, maxsize; >>>> >>>> dentry_page = read_mapping_page(mapping, i, NULL); >>>> - if (IS_ERR(dentry_page)) >>>> - continue; >>>> + if (IS_ERR(dentry_page)) { >>>> + errln("fail to readdir of logical block %u of nid %llu", >>>> + i, EROFS_V(dir)->nid); >>>> + err = PTR_ERR(dentry_page); >>>> + break; >>> >>> I don't think you want to use the errno that came back from >>> read_mapping_page() (which is, I think, always going to be -EIO). >>> Rather you want -EFSCORRUPTED, at least if I understand the recent >>> patches to ext2/ext4/f2fs/xfs/... >> >> Thanks for your reply and noticing this. :) >> >> Yes, as I talked with you about read_mapping_page() in a xfs related >> topic earlier, I think I fully understand what returns here. >> >> I actually had some concern about that before sending out this patch. >> You know the status is >> PG_uptodate is not set and PG_error is set here. >> >> But we cannot know it is actually a disk read error or due to >> corrupted images (due to lack of page flags or some status, and >> I think it could be a waste of page structure space for such >> corrupted image or disk error)... >> >> And some people also like propagate errors from insiders... >> (and they could argue about err = -EFSCORRUPTED as well..) >> >> I'd like hear your suggestion about this after my words above? >> still return -EFSCORRUPTED? > > I don't think it matters whether it's due to a disk error or a corrupted > image. We can't read the directory entry, so we should probably return > -EFSCORRUPTED. Thinking about it some more, read_mapping_page() can > also return -ENOMEM, so it should probably look something like this: > > err = 0; > if (dentry_page == ERR_PTR(-ENOMEM)) > err = -ENOMEM; > else if (IS_ERR(dentry_page)) { > errln("fail to readdir of logical block %u of nid %llu", > i, EROFS_V(dir)->nid); > err = -EFSCORRUPTED; Well, if there is real IO error happen under filesystem, we should return -EIO instead of EFSCORRUPTED? The right fix may be that doing sanity check on on-disk blkaddr, and return -EFSCORRUPTED if the blkaddr is invalid and propagate the error to its caller erofs_readdir(), IIUC below error info. > [36297.354090] attempt to access beyond end of device > [36297.354098] loop17: rw=0, want=29887428984, limit=1953128 > [36297.354107] attempt to access beyond end of device > [36297.354109] loop17: rw=0, want=29887428480, limit=1953128 > [36301.827234] attempt to access beyond end of device > [36301.827243] loop17: rw=0, want=29887428480, limit=1953128 Thanks, > } > > if (err) > break; >