[PATCH 6.6~6.9] cifs: fix pagecache leak when do writepages

[PATCH 6.6~6.9] cifs: fix pagecache leak when do writepages

[Yang Erkun]

After commit f3dc1bdb6b0b("cifs: Fix writeback data corruption"), the
writepages for cifs will find all folio needed writepage with two phase.
The first folio will be found in cifs_writepages_begin, and the latter
various folios will be found in cifs_extend_writeback.

All those will first get folio, and for normal case, once we set page
writeback and after do really write, we should put the reference, folio
found in cifs_extend_writeback do this with folio_batch_release. But the
folio found in cifs_writepages_begin never get the chance do it. And
every writepages call, we will leak a folio(found this problem while do
xfstests over cifs).

Besides, the exist path seem never handle this folio correctly, fix it too
with this patch.

The problem does not exist in mainline since writepages path for cifs
has changed to netfs. It's had to backport all related change, so try fix
this problem with this single patch.

Fixes: f3dc1bdb6b0b ("cifs: Fix writeback data corruption")
Signed-off-by: Yang Erkun <yangerkun@huawei.com>
---
 fs/smb/client/file.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
index 9be37d0fe724..0a48d80b3871 100644
--- a/fs/smb/client/file.c
+++ b/fs/smb/client/file.c
@@ -2860,17 +2860,21 @@ static ssize_t cifs_write_back_from_locked_folio(struct address_space *mapping,
 	rc = cifs_get_writable_file(CIFS_I(inode), FIND_WR_ANY, &cfile);
 	if (rc) {
 		cifs_dbg(VFS, "No writable handle in writepages rc=%d\n", rc);
+		folio_unlock(folio);
 		goto err_xid;
 	}
 
 	rc = server->ops->wait_mtu_credits(server, cifs_sb->ctx->wsize,
 					   &wsize, credits);
-	if (rc != 0)
+	if (rc != 0) {
+		folio_unlock(folio);
 		goto err_close;
+	}
 
 	wdata = cifs_writedata_alloc(cifs_writev_complete);
 	if (!wdata) {
 		rc = -ENOMEM;
+		folio_unlock(folio);
 		goto err_uncredit;
 	}
 
@@ -3017,17 +3021,22 @@ static ssize_t cifs_writepages_begin(struct address_space *mapping,
 lock_again:
 	if (wbc->sync_mode != WB_SYNC_NONE) {
 		ret = folio_lock_killable(folio);
-		if (ret < 0)
+		if (ret < 0) {
+			folio_put(folio);
 			return ret;
+		}
 	} else {
-		if (!folio_trylock(folio))
+		if (!folio_trylock(folio)) {
+			folio_put(folio);
 			goto search_again;
+		}
 	}
 
 	if (folio->mapping != mapping ||
 	    !folio_test_dirty(folio)) {
 		start += folio_size(folio);
 		folio_unlock(folio);
+		folio_put(folio);
 		goto search_again;
 	}
 
@@ -3057,6 +3066,7 @@ static ssize_t cifs_writepages_begin(struct address_space *mapping,
 out:
 	if (ret > 0)
 		*_start = start + ret;
+	folio_put(folio);
 	return ret;
 }
 
-- 
2.39.2

Re: [PATCH 6.6~6.9] cifs: fix pagecache leak when do writepages

[Greg KH]

On Mon, Jun 24, 2024 at 12:28:15PM +0800, Yang Erkun wrote:
> After commit f3dc1bdb6b0b("cifs: Fix writeback data corruption"), the
> writepages for cifs will find all folio needed writepage with two phase.
> The first folio will be found in cifs_writepages_begin, and the latter
> various folios will be found in cifs_extend_writeback.
> 
> All those will first get folio, and for normal case, once we set page
> writeback and after do really write, we should put the reference, folio
> found in cifs_extend_writeback do this with folio_batch_release. But the
> folio found in cifs_writepages_begin never get the chance do it. And
> every writepages call, we will leak a folio(found this problem while do
> xfstests over cifs).
> 
> Besides, the exist path seem never handle this folio correctly, fix it too
> with this patch.
> 
> The problem does not exist in mainline since writepages path for cifs
> has changed to netfs. It's had to backport all related change, so try fix
> this problem with this single patch.
> 
> Fixes: f3dc1bdb6b0b ("cifs: Fix writeback data corruption")
> Signed-off-by: Yang Erkun <yangerkun@huawei.com>
> ---
>  fs/smb/client/file.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

Re: [PATCH 6.6~6.9] cifs: fix pagecache leak when do writepages

[yangerkun]

在 2024/6/24 22:57, Greg KH 写道:
> On Mon, Jun 24, 2024 at 12:28:15PM +0800, Yang Erkun wrote:
>> After commit f3dc1bdb6b0b("cifs: Fix writeback data corruption"), the
>> writepages for cifs will find all folio needed writepage with two phase.
>> The first folio will be found in cifs_writepages_begin, and the latter
>> various folios will be found in cifs_extend_writeback.
>>
>> All those will first get folio, and for normal case, once we set page
>> writeback and after do really write, we should put the reference, folio
>> found in cifs_extend_writeback do this with folio_batch_release. But the
>> folio found in cifs_writepages_begin never get the chance do it. And
>> every writepages call, we will leak a folio(found this problem while do
>> xfstests over cifs).
>>
>> Besides, the exist path seem never handle this folio correctly, fix it too
>> with this patch.
>>
>> The problem does not exist in mainline since writepages path for cifs
>> has changed to netfs. It's had to backport all related change, so try fix
>> this problem with this single patch.
>>
>> Fixes: f3dc1bdb6b0b ("cifs: Fix writeback data corruption")
>> Signed-off-by: Yang Erkun <yangerkun@huawei.com>
>> ---
>>   fs/smb/client/file.c | 16 +++++++++++++---
>>   1 file changed, 13 insertions(+), 3 deletions(-)
>>
> 
> <formletter>
> 
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read:
>      https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> for how to do this properly.
> 
> </formletter>

Thanks for your reminder, will do it in v2!