| Message ID | 1566287651-11386-1-git-send-email-wenwen@cs.uga.edu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | NFSv4: Fix a memory leak bug | expand |
Hi Wenwen, On Tue, 2019-08-20 at 02:54 -0500, Wenwen Wang wrote: > In nfs4_try_migration(), if nfs4_begin_drain_session() fails, the > previously allocated 'page' and 'locations' are not deallocated, > leading to > memory leaks. To fix this issue, free 'page' and 'locations' before > returning the error. > > Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu> > --- > fs/nfs/nfs4state.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c > index cad4e06..37823dc 100644 > --- a/fs/nfs/nfs4state.c > +++ b/fs/nfs/nfs4state.c > @@ -2095,8 +2095,12 @@ static int nfs4_try_migration(struct > nfs_server *server, const struct cred *cred > } > > status = nfs4_begin_drain_session(clp); > - if (status != 0) > + if (status != 0) { > + if (page != NULL) > + __free_page(page); > + kfree(locations); > return status; Thanks for the suggestion! I think a better option would be to switch the "return status" into a "goto out" so we can keep all our cleanup code in a single place in case we ever need to change it in the future. What do you think? Anna > + } > > status = nfs4_replace_transport(server, locations); > if (status != 0) { > -- > 2.7.4 >
On Tue, Aug 20, 2019 at 9:41 AM Schumaker, Anna <Anna.Schumaker@netapp.com> wrote: > > Hi Wenwen, > > On Tue, 2019-08-20 at 02:54 -0500, Wenwen Wang wrote: > > In nfs4_try_migration(), if nfs4_begin_drain_session() fails, the > > previously allocated 'page' and 'locations' are not deallocated, > > leading to > > memory leaks. To fix this issue, free 'page' and 'locations' before > > returning the error. > > > > Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu> > > --- > > fs/nfs/nfs4state.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c > > index cad4e06..37823dc 100644 > > --- a/fs/nfs/nfs4state.c > > +++ b/fs/nfs/nfs4state.c > > @@ -2095,8 +2095,12 @@ static int nfs4_try_migration(struct > > nfs_server *server, const struct cred *cred > > } > > > > status = nfs4_begin_drain_session(clp); > > - if (status != 0) > > + if (status != 0) { > > + if (page != NULL) > > + __free_page(page); > > + kfree(locations); > > return status; > > Thanks for the suggestion! I think a better option would be to switch > the "return status" into a "goto out" so we can keep all our cleanup > code in a single place in case we ever need to change it in the future. > > What do you think? Thanks for your comments! I will rework the patch. Wenwen > Anna > > > + } > > > > status = nfs4_replace_transport(server, locations); > > if (status != 0) { > > -- > > 2.7.4 > >
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c index cad4e06..37823dc 100644 --- a/fs/nfs/nfs4state.c +++ b/fs/nfs/nfs4state.c @@ -2095,8 +2095,12 @@ static int nfs4_try_migration(struct nfs_server *server, const struct cred *cred } status = nfs4_begin_drain_session(clp); - if (status != 0) + if (status != 0) { + if (page != NULL) + __free_page(page); + kfree(locations); return status; + } status = nfs4_replace_transport(server, locations); if (status != 0) {
In nfs4_try_migration(), if nfs4_begin_drain_session() fails, the previously allocated 'page' and 'locations' are not deallocated, leading to memory leaks. To fix this issue, free 'page' and 'locations' before returning the error. Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu> --- fs/nfs/nfs4state.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)