From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out30-119.freemail.mail.aliyun.com (out30-119.freemail.mail.aliyun.com [115.124.30.119]) (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 841833438B6 for ; Wed, 20 May 2026 04:41:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=115.124.30.119 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779252119; cv=none; b=Y9TllUxHRrJxi+bzvw+LE04DxxFXqGOkivr3mGKvpU2r26etcr2ZSy8kWolBMNQ9izO7HGOmDgAaQEoGBflpB+6SFIzDdvT5MJRxzxP6haGm7ibZ+9pTsh2xOG3VcxFclxDke8YlyIp3DpJzD0EU3MAinqBGPMxmZwB+pL09CnE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779252119; c=relaxed/simple; bh=DgiwFa40M1HYFL8SzxE9GwmyOcOZ67UqxAhNwUjSxkA=; h=Message-ID:Date:MIME-Version:Subject:From:To:Cc:References: In-Reply-To:Content-Type; b=bN7LV1Kb+RijqnW9cUzx73xnxXikRKV3FO+22aKeTe4oMwF2el6YuRvYcfeM8kPHqOYVZFsUBsz2lAUfCWinoOWM4oUox+H/JQzJ9zqm3z4xcYyghCOxNCAajCRWgUllRdB5NjqMIPZ815mm1T3394xFpXU/oQ0wKDr/62SIFjo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com; spf=pass smtp.mailfrom=linux.alibaba.com; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b=eN0DzNoD; arc=none smtp.client-ip=115.124.30.119 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b="eN0DzNoD" DKIM-Signature:v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1779252111; h=Message-ID:Date:MIME-Version:Subject:From:To:Content-Type; bh=+OaZwJUGLjgCIBpYsr1gr8lN155FvC1QL1sH+9BnMXE=; b=eN0DzNoDcHu5J8a48Ycu58dn4ldF1PzmorRPS6gdvZpt1fRkEu7bVnSj7iZyD3l9evIG/sjRDgHtL2bXhbG/lC+4HoEPDxYcgAq5bJNBkPhslsGrI2bcMDHkcPhA4HLP2FPsZ45g2tiHQVxLj1k41Qbu1W3Xsme32zizRgbQ/jc= X-Alimail-AntiSpam:AC=PASS;BC=-1|-1;BR=01201311R181e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=maildocker-contentspam033037033178;MF=hsiangkao@linux.alibaba.com;NM=1;PH=DS;RN=11;SR=0;TI=SMTPD_---0X3HJC5n_1779252109; Received: from 30.221.132.121(mailfrom:hsiangkao@linux.alibaba.com fp:SMTPD_---0X3HJC5n_1779252109 cluster:ay36) by smtp.aliyun-inc.com; Wed, 20 May 2026 12:41:50 +0800 Message-ID: Date: Wed, 20 May 2026 12:41:49 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] erofs: fix metabuf leak in shared xattr initialization From: Gao Xiang To: Jia Zhu , Gao Xiang , Chao Yu Cc: linux-erofs@lists.ozlabs.org, linux-kernel@vger.kernel.org, Yue Hu , Jeffle Xu , Sandeep Dhavale , Hongbo Li , Chunhai Guo , Amir Goldstein References: <20260520034252.40163-1-zhujia.zj@bytedance.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 2026/5/20 12:25, Gao Xiang wrote: > Hi Jia, > > On 2026/5/20 11:42, Jia Zhu wrote: >> erofs_init_inode_xattrs() uses a local metabuf while reading the inline >> xattr header and the shared xattr id array. >> >> It currently drops that metabuf from some error paths and from the success >> path, but the erofs_bread() failure while reading the shared xattr id array >> goes straight to out_unlock. >> >> This became observable when file-backed metadata reads started calling >> rw_verify_area() before reusing or dropping the current metabuf.  Before >> that, the read_mapping_folio() failure path already dropped the old metabuf >> before returning an error. even it's exposable directly due to commit 307210c262a2 "erofs: verify metadata accesses for file-backed mounts") I still hope we could mark it as an xattr implementaion issue instead of a random consequence, we should erofs_put_metabuf() as long as `erofs_buf` was successfully used once. >> >> Consolidate the local metabuf cleanup at out_unlock. erofs_put_metabuf() >> is a no-op if no page has been acquired, and this keeps all paths after >> taking EROFS_I_BL_XATTR_BIT covered by one cleanup site. >> >> Fixes: 307210c262a2 ("erofs: verify metadata accesses for file-backed mounts") > > > Hmm, I don't think it's a correct "Fixes:", and > the commit message should be fixed too. > > I think it's an issue due to missing erofs_put_metabuf() > in the erofs_init_inode_xattrs() error path instead. > > I think > Fixes: bb88e8da0025 ("erofs: use meta buffers for xattr operations") > > is a more proper "Fixes:". > > Thanks, > Gao Xiang > >> Signed-off-by: Jia Zhu >> --- >>   fs/erofs/xattr.c | 4 +--- >>   1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c >> index 41e311019a251..df7ea019526d7 100644 >> --- a/fs/erofs/xattr.c >> +++ b/fs/erofs/xattr.c >> @@ -89,13 +89,11 @@ static int erofs_init_inode_xattrs(struct inode *inode) >>           vi->xattr_isize - sizeof(struct erofs_xattr_ibody_header)) { >>           erofs_err(sb, "invalid h_shared_count %u @ nid %llu", >>                 vi->xattr_shared_count, vi->nid); >> -        erofs_put_metabuf(&buf); >>           ret = -EFSCORRUPTED; >>           goto out_unlock; >>       } >>       vi->xattr_shared_xattrs = kmalloc_objs(uint, vi->xattr_shared_count); >>       if (!vi->xattr_shared_xattrs) { >> -        erofs_put_metabuf(&buf); >>           ret = -ENOMEM; >>           goto out_unlock; >>       } >> @@ -112,12 +110,12 @@ static int erofs_init_inode_xattrs(struct inode *inode) >>           } >>           vi->xattr_shared_xattrs[i] = le32_to_cpu(*xattr_id); >>       } >> -    erofs_put_metabuf(&buf); >>       /* paired with smp_mb() at the beginning of the function. */ >>       smp_mb(); >>       set_bit(EROFS_I_EA_INITED_BIT, &vi->flags); >>   out_unlock: >> +    erofs_put_metabuf(&buf); >>       clear_and_wake_up_bit(EROFS_I_BL_XATTR_BIT, &vi->flags); >>       return ret; >>   } >