linux-kernel - reply: [PATCH AUTOSEL 6.11 47/76] fs: nfs: fix missing refcnt by replacing folio_set_private by folio_attach

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <0120b0d030c14601ba3a19f016615eff@BJMBX01.spreadtrum.com>
Date: Sat, 5 Oct 2024 03:59:34 +0000
From: 黄朝阳 (Zhaoyang Huang)
	<zhaoyang.huang@...soc.com>
To: Sasha Levin <sashal@...nel.org>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        "stable@...r.kernel.org"
	<stable@...r.kernel.org>,
        康纪滨 (Steve Kang)
	<Steve.Kang@...soc.com>
CC: Anna Schumaker <anna.schumaker@...cle.com>,
        "trondmy@...nel.org"
	<trondmy@...nel.org>,
        "anna@...nel.org" <anna@...nel.org>,
        "linux-nfs@...r.kernel.org" <linux-nfs@...r.kernel.org>
Subject: reply: [PATCH AUTOSEL 6.11 47/76] fs: nfs: fix missing refcnt by
 replacing folio_set_private by folio_attach_private

>From: Zhaoyang Huang <zhaoyang.huang@...soc.com>
>
>[ Upstream commit 03e02b94171b1985dd0aa184296fe94425b855a3 ]
>
>This patch is inspired by a code review of fs codes which aims at folio's extra
>refcnt that could introduce unwanted behavious when judging refcnt, such
>as[1].That is, the folio passed to mapping_evict_folio carries the refcnts from
>find_lock_entries, page_cache, corresponding to PTEs and folio's private if has.
>However, current code doesn't take the refcnt for folio's private which could
>have mapping_evict_folio miss the one to only PTE and lead to call
>filemap_release_folio wrongly.
>
>[1]
>long mapping_evict_folio(struct address_space *mapping, struct folio *folio)
>{ ...
>//current code will misjudge here if there is one pte on the folio which is be
>deemed as the one as folio's private
>        if (folio_ref_count(folio) >
>                        folio_nr_pages(folio) + folio_has_private(folio) +
>1)
>                return 0;
>        if (!filemap_release_folio(folio, 0))
>                return 0;
>
>        return remove_mapping(mapping, folio); }
>
>Signed-off-by: Zhaoyang Huang <zhaoyang.huang@...soc.com>
>Signed-off-by: Anna Schumaker <anna.schumaker@...cle.com>
>Signed-off-by: Sasha Levin <sashal@...nel.org>
Sorry, this patch should be dropped since Trond has explained that there is no need to grab refcnt here as nfs_page has hold the refcount on this folio.
>---
> fs/nfs/write.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
>diff --git a/fs/nfs/write.c b/fs/nfs/write.c index
>d074d0ceb4f01..80c6ded5f74c6 100644
>--- a/fs/nfs/write.c
>+++ b/fs/nfs/write.c
>@@ -772,8 +772,7 @@ static void nfs_inode_add_request(struct nfs_page
>*req)
>        nfs_lock_request(req);
>        spin_lock(&mapping->i_private_lock);
>        set_bit(PG_MAPPED, &req->wb_flags);
>-       folio_set_private(folio);
>-       folio->private = req;
>+       folio_attach_private(folio, req);
>        spin_unlock(&mapping->i_private_lock);
>        atomic_long_inc(&nfsi->nrequests);
>        /* this a head request for a page group - mark it as having an @@
>-797,8 +796,7 @@ static void nfs_inode_remove_request(struct nfs_page
>*req)
>
>                spin_lock(&mapping->i_private_lock);
>                if (likely(folio)) {
>-                       folio->private = NULL;
>-                       folio_clear_private(folio);
>+                       folio_detach_private(folio);
>                        clear_bit(PG_MAPPED,
>&req->wb_head->wb_flags);
>                }
>                spin_unlock(&mapping->i_private_lock);
>--
>2.43.0