From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out199-14.us.a.mail.aliyun.com (out199-14.us.a.mail.aliyun.com [47.90.199.14]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2EA09168B5 for ; Fri, 16 Jun 2023 13:14:37 +0000 (UTC) X-Alimail-AntiSpam:AC=PASS;BC=-1|-1;BR=01201311R621e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=ay29a033018045170;MF=hsiangkao@linux.alibaba.com;NM=1;PH=DS;RN=7;SR=0;TI=SMTPD_---0VlGJdN7_1686921257; Received: from 192.168.33.9(mailfrom:hsiangkao@linux.alibaba.com fp:SMTPD_---0VlGJdN7_1686921257) by smtp.aliyun-inc.com; Fri, 16 Jun 2023 21:14:19 +0800 Message-ID: Date: Fri, 16 Jun 2023 21:14:16 +0800 Precedence: bulk X-Mailing-List: fsverity@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 Subject: Re: [PATCH v2 5/6] ovl: Validate verity xattr when resolving lowerdata To: Alexander Larsson , Amir Goldstein Cc: Eric Biggers , miklos@szeredi.hu, linux-unionfs@vger.kernel.org, tytso@mit.edu, fsverity@lists.linux.dev References: <20230616052444.GA181948@sol.localdomain> From: Gao Xiang In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 2023/6/16 19:33, Alexander Larsson wrote: > On Fri, Jun 16, 2023 at 11:28 AM Amir Goldstein wrote: >> >> On Fri, Jun 16, 2023 at 11:39 AM Alexander Larsson wrote: >>> >>> On Fri, Jun 16, 2023 at 10:12 AM Amir Goldstein wrote: >>>> >>>> >>>>> I don't believe this format will actually need to change much. >>>>> However, I do agree >>>>> with the general requirement for *some* ability to move forward with >>>>> this format, >>>>> so I'm gonna go with a single version byte. >>>>> >>>> >>>> I disagree. >>>> If you present a real life use case why that really matters >>>> then I can reconsider. >>> >>> I disagree, but I'll add them. >>> >> >> Let's ask for a 3rd opinion. >> don't add them for now, unless Miklos says that you should. > > I added them to the branch anyway for now. However, if we're going > full header + flags anyway, I wonder if we really need the > "overlay.digest" xattr at all? We could just put the header + optional > digest in the "overlay.metacopy" xattr, and then just read/store one > xattr. Right now metacopy is zero size, but adding some data to it > would not break ovl_check_metacopy_xattr() in older kernels. > > Basically, during the lookup we get the metacopy xattr anyway, and > when we do we could record in a flag that there is a digest in it, > then during open we don't have to look for a separate digest xattr, > just re-load the metacopy xattr if the flag is set. With this in place > we can also easily add other flags to overlay.metacopy, which imho > makes a ton more sense than adding flags to overlay.digest. My own slight concern about this is that: Previously, all metacopy inodes shares a common EROFS shared xattr which can be cached in memory once when the first read and other inodes won't trigger any I/Os for this. If "overlay.metacopy" xattr is not empty thus it cannot be shared, I guess at least you could place it into an EROFS inline xattr, that may be good as well if you also keep "header + flags". But I'm not sure if it's a good idea to keep the full fsverity digest in "overlay.metacopy" honestly, especially overlayfs fsverity feature is off. That will amplify I/Os (which could be landed in EROFS shared xattrs as "overlay.digest" in the past and now we need to read it immediately.) Especially for the "ls -lR" workload, I guess you might need to evaluate this way (only add flags vs flags + digest in "overlay.metacopy") first. Thanks, Gao Xiang > > I'll have a look at this. >