Hi,
> diff --git a/fs/cachefiles/rdwr.c b/fs/cachefiles/rdwr.c
> index ad74a6a..ead4981 100644
> --- a/fs/cachefiles/rdwr.c
> +++ b/fs/cachefiles/rdwr.c
> @@ -523,7 +523,10 @@ static int cachefiles_read_backing_file(struct
> cachefiles_object *object,
> netpage->index, cachefiles_gfp);
> if (ret < 0) {
> if (ret == -EEXIST) {
> + page_cache_release(backpage);
> + backpage = NULL;
> page_cache_release(netpage);
> + netpage = NULL;
> fscache_retrieval_complete(op, 1);
> continue;
> }
> @@ -596,7 +599,10 @@ static int cachefiles_read_backing_file(struct
> cachefiles_object *object,
> netpage->index, cachefiles_gfp);
> if (ret < 0) {
> if (ret == -EEXIST) {
> + page_cache_release(backpage);
> + backpage = NULL;
> page_cache_release(netpage);
> + netpage = NULL;
> fscache_retrieval_complete(op, 1);
> continue;
> }
This doesn't directly apply to master any more but this does (but is untested):
Subject: [PATCH] cachefiles: page reference leak fix
Link: https://www.redhat.com/archives/linux-cachefs/2014-August/msg00017.html
Link: https://www.redhat.com/archives/linux-cachefs/2014-August/msg00021.html
Signed-off-by: Shantanu Goel <sgoel01@xxxxxxxxx>
[dja: forward ported to current upstream]
Signed-off-by: Daniel Axtens <dja@xxxxxxxxxx>
---
fs/cachefiles/rdwr.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/fs/cachefiles/rdwr.c b/fs/cachefiles/rdwr.c
index 40f7595aad10..db233588a69a 100644
--- a/fs/cachefiles/rdwr.c
+++ b/fs/cachefiles/rdwr.c
@@ -535,7 +535,10 @@ static int cachefiles_read_backing_file(struct cachefiles_object *object,
netpage->index, cachefiles_gfp);
if (ret < 0) {
if (ret == -EEXIST) {
+ put_page(backpage);
+ backpage = NULL;
put_page(netpage);
+ netpage = NULL;
fscache_retrieval_complete(op, 1);
continue;
}
@@ -608,7 +611,10 @@ static int cachefiles_read_backing_file(struct cachefiles_object *object,
netpage->index, cachefiles_gfp);
if (ret < 0) {
if (ret == -EEXIST) {
+ put_page(backpage);
+ backpage = NULL;
put_page(netpage);
+ netpage = NULL;
fscache_retrieval_complete(op, 1);
continue;
}
--
2.17.1
However, I am not sure about the first hunk.
For the second hunk - in backing_page_already_uptodate, we can see that
the backing page will be released if adding the netpage to the page
cache LRU succeeds, but will not be release in the -EEXIST case. This
change causes it to be release.
With the first hunk, we're in monitor_backing_page. It looks to me like
you could get to the release before the relevant get, so I'm worried it
will incorrectly change the refcount. Am I understanding this
incorrectly?
Regards,
Daniel
--
Linux-cachefs mailing list
Linux-cachefs@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/linux-cachefs
