Skip to contentSkip to conversation

nfs/flexfiles: fix leak of nfs4_ff_ds_version arrays

PATCH In review Weston Andros Adamson 2 participants 4 messages Aug 1, 2017 – Aug 11, 2017

Subsystems (2)
NFS client NFS server
lore
Share & tools
lore

Weston Andros Adamson

author
#0 lore raw

PATCH 1/1 nfs/flexfiles: fix leak of nfs4_ff_ds_version arrays

The client was freeing the nfs4_ff_layout_ds, but not the contained nfs4_ff_ds_version array.

Signed-off-by Weston Andros Adamson <[email protected]>
---
 fs/nfs/flexfilelayout/flexfilelayoutdev.c | 1 +
 1 file changed, 1 insertion(+)
fs/nfs/flexfilelayout/flexfilelayoutdev.c +1 -0
index 6df7a0cf5660..f32c58bbe556 100644
@@ -32,6 +32,7 @@ void nfs4_ff_layout_free_deviceid(struct nfs4_ff_layout_ds *mirror_ds)
 {
 	nfs4_print_deviceid(&mirror_ds->id_node.deviceid);
 	nfs4_pnfs_ds_put(mirror_ds->ds);
+	kfree(mirror_ds->ds_versions);
 	kfree_rcu(mirror_ds, id_node.rcu);
 }
Signature · 1 line
2.11.0 (Apple Git-81)
author
#1 lore raw

This should probably be CCed to some stable releases too. I'll figure it = out soon.

-dros

Quoted 1 line

On Aug 1, 2017, at 4:25 PM, Weston Andros Adamson <[email protected]> =

=

Quoted 10 lines · wrote:

=20
The client was freeing the nfs4_ff_layout_ds, but not the contained nfs4_ff_ds_version array.
=20

Signed-off-by Weston Andros Adamson <[email protected]>

---
fs/nfs/flexfilelayout/flexfilelayoutdev.c | 1 +

1 file changed, 1 insertion(+)

=20

= renamed from fs/nfs/flexfilelayout/flexfilelayoutdev.c +0 -0

b/fs/nfs/flexfilelayout/flexfilelayoutdev.c

fs/nfs/flexfilelayout/flexfilelayoutdev.c void nfs4_ff_layout_free_deviceid(struct =

Quoted 4 lines

index 6df7a0cf5660..f32c58bbe556 100644

fs/nfs/flexfilelayout/flexfilelayoutdev.c +0 -0
@@ -32,6 +32,7 @@ void nfs4_ff_layout_free_deviceid(struct =

nfs4_ff_layout_ds *mirror_ds)

Quoted 10 lines
fs/nfs/flexfilelayout/flexfilelayoutdev.c +1 -0
 {
 	nfs4_print_deviceid(&mirror_ds->id_node.deviceid);
 	nfs4_pnfs_ds_put(mirror_ds->ds);
+	kfree(mirror_ds->ds_versions);
 	kfree_rcu(mirror_ds, id_node.rcu);
 }

=20
--=20
2.11.0 (Apple Git-81)
=20

#2 lore raw

Hi Dros,

can this leak be the source of issue described here: http://www.spinics.net/lists/linux-nfs/msg64890.html

Tigran.

----- Original Message -----

Quoted 43 lines

From: "Weston Andros Adamson" <[email protected]>
To: "Trond Myklebust" <[email protected]>, "Anna Schumaker" <[email protected]>
Cc: "linux-nfs" <[email protected]>
Sent: Thursday, August 3, 2017 4:07:28 AM
Subject: Re: [PATCH] nfs/flexfiles: fix leak of nfs4_ff_ds_version arrays

This should probably be CCed to some stable releases too. I'll figure it out soon.

-dros

Quoted 25 lines · Weston Andros Adamson

The client was freeing the nfs4_ff_layout_ds, but not the contained nfs4_ff_ds_version array.

Signed-off-by Weston Andros Adamson <[email protected]>

---
fs/nfs/flexfilelayout/flexfilelayoutdev.c | 1 +

1 file changed, 1 insertion(+)
fs/nfs/flexfilelayout/flexfilelayoutdev.c +1 -1
index 6df7a0cf5660..f32c58bbe556 100644
@@ -32,6 +32,7 @@ void nfs4_ff_layout_free_deviceid(struct nfs4_ff_layout_ds
 *mirror_ds)
 {
 	nfs4_print_deviceid(&mirror_ds->id_node.deviceid);
 	nfs4_pnfs_ds_put(mirror_ds->ds);
+	kfree(mirror_ds->ds_versions);
 	kfree_rcu(mirror_ds, id_node.rcu);
 }
 
--
Quoted 2 lines

2.11.0 (Apple Git-81)

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#3 lore raw

Hey Tigran,

I don't think so, this fixes a simple memory leak where memory is not = freed and nothing holds a reference to it. It should not lead to any = invalid pointers, but will eventually lead to the OOM killer killing a = bunch of processes.

There have been other recent changes that may have addressed this. Can = you reproduce with the latest changes?

-dros

Quoted 1 line

On Aug 3, 2017, at 8:57 AM, Mkrtchyan, Tigran =

Quoted 4 lines · <[email protected]> wrote:

=20
Hi Dros,
=20
can this leak be the source of issue described here: =

http://www.spinics.net/lists/linux-nfs/msg64890.html

Quoted 6 lines

=20
Tigran.
=20
----- Original Message -----

Quoted 2 lines

From: "Weston Andros Adamson" <[email protected]>
To: "Trond Myklebust" <[email protected]>, "Anna =

Schumaker" <[email protected]>

Quoted 3 lines
Quoted 3 lines

Cc: "linux-nfs" <[email protected]>
Sent: Thursday, August 3, 2017 4:07:28 AM
Subject: Re: [PATCH] nfs/flexfiles: fix leak of nfs4_ff_ds_version =

arrays

Quoted 2 lines

=20

Quoted 1 line

This should probably be CCed to some stable releases too. I'll figure =

it out

Quoted 5 lines
Quoted 5 lines

soon.
=20
-dros
=20

Quoted 1 line

On Aug 1, 2017, at 4:25 PM, Weston Andros Adamson <[email protected]> =

Quoted 7 lines · wrote:
Quoted 7 lines
Quoted 7 lines

=20
The client was freeing the nfs4_ff_layout_ds, but not the contained nfs4_ff_ds_version array.
=20

Signed-off-by Weston Andros Adamson <[email protected]>

---
fs/nfs/flexfilelayout/flexfilelayoutdev.c | 1 +

Quoted 8 lines
Quoted 8 lines

fs/nfs/flexfilelayout/flexfilelayoutdev.c void nfs4_ff_layout_free_deviceid(struct =

Quoted 8 lines
1 file changed, 1 insertion(+)

=20

fs/nfs/flexfilelayout/flexfilelayoutdev.c +0 -0
index 6df7a0cf5660..f32c58bbe556 100644
@@ -32,6 +32,7 @@ void nfs4_ff_layout_free_deviceid(struct =

nfs4_ff_layout_ds

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
fs/nfs/flexfilelayout/flexfilelayoutdev.c +1 -0
 *mirror_ds)
 {
 	nfs4_print_deviceid(&mirror_ds->id_node.deviceid);
 	nfs4_pnfs_ds_put(mirror_ds->ds);
+	kfree(mirror_ds->ds_versions);
 	kfree_rcu(mirror_ds, id_node.rcu);
Quoted 8 lines
Quoted 8 lines
Quoted 5 lines
fs/nfs/flexfilelayout/flexfilelayoutdev.c +0 -0
 }

=20
--
2.11.0 (Apple Git-81)
=20

=20
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" =

in

Quoted 2 lines
Quoted 2 lines

the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html