2020-01-21 22:15:03

by Olga Kornievskaia

Subject: [PATCH 1/1] NFSv4.2 re-initialize cn_resp in case of a retry

From: Olga Kornievskaia <[email protected]>

If nfs42_proc_copy returned a EAGAIN, we need to re-initialize the
memory in case memory allocation fails.

Fixes: 66588abe2 ("NFSv4.2 fix kfree in __nfs42_copy_file_range")
Reported-by: kbuild test robot <[email protected]>
Reported-by: Julia Lawall <[email protected]>
Signed-off-by: Olga Kornievskaia <[email protected]>
---
fs/nfs/nfs4file.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
index 620de90..9f72efe 100644
--- a/fs/nfs/nfs4file.c
+++ b/fs/nfs/nfs4file.c
@@ -177,8 +177,10 @@ static ssize_t __nfs4_copy_file_range(struct file *file_in, loff_t pos_in,
ret = nfs42_proc_copy(file_in, pos_in, file_out, pos_out, count,
nss, cnrs, sync);
out:
- if (!nfs42_files_from_same_server(file_in, file_out))
+ if (!nfs42_files_from_same_server(file_in, file_out)) {
kfree(cn_resp);
+ cn_resp = NULL;
+ }
if (ret == -EAGAIN)
goto retry;
return ret;
--
1.8.3.1

2020-01-22 18:41:17

by Anna Schumaker

Subject: Re: [PATCH 1/1] NFSv4.2 re-initialize cn_resp in case of a retry

Hi Olga,

On Tue, 2020-01-21 at 17:14 -0500, Olga Kornievskaia wrote:
> From: Olga Kornievskaia <[email protected]>
>
> If nfs42_proc_copy returned a EAGAIN, we need to re-initialize the
> memory in case memory allocation fails.

I guess I'm not sure how we would hit this. Doesn't kzalloc() return NULL if the
memory allocation fails?

>
> Fixes: 66588abe2 ("NFSv4.2 fix kfree in __nfs42_copy_file_range")
> Reported-by: kbuild test robot <[email protected]>
> Reported-by: Julia Lawall <[email protected]>
> Signed-off-by: Olga Kornievskaia <[email protected]>
> ---
> fs/nfs/nfs4file.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
> index 620de90..9f72efe 100644
> --- a/fs/nfs/nfs4file.c
> +++ b/fs/nfs/nfs4file.c
> @@ -177,8 +177,10 @@ static ssize_t __nfs4_copy_file_range(struct file
> *file_in, loff_t pos_in,
> ret = nfs42_proc_copy(file_in, pos_in, file_out, pos_out, count,
> nss, cnrs, sync);
> out:
> - if (!nfs42_files_from_same_server(file_in, file_out))
> + if (!nfs42_files_from_same_server(file_in, file_out)) {
> kfree(cn_resp);
> + cn_resp = NULL;
> + }
> if (ret == -EAGAIN)
> goto retry;
> return ret;

2020-01-22 20:29:32

by Olga Kornievskaia

Subject: Re: [PATCH 1/1] NFSv4.2 re-initialize cn_resp in case of a retry

On Wed, Jan 22, 2020 at 1:40 PM Schumaker, Anna
<[email protected]> wrote:
>
> Hi Olga,
>
> On Tue, 2020-01-21 at 17:14 -0500, Olga Kornievskaia wrote:
> > From: Olga Kornievskaia <[email protected]>
> >
> > If nfs42_proc_copy returned a EAGAIN, we need to re-initialize the
> > memory in case memory allocation fails.
>
> I guess I'm not sure how we would hit this. Doesn't kzalloc() return NULL if the
> memory allocation fails?

You are right kzalloc would always return NULL so forget about this patch.

>
> >
> > Fixes: 66588abe2 ("NFSv4.2 fix kfree in __nfs42_copy_file_range")
> > Reported-by: kbuild test robot <[email protected]>
> > Reported-by: Julia Lawall <[email protected]>
> > Signed-off-by: Olga Kornievskaia <[email protected]>
> > ---
> > fs/nfs/nfs4file.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
> > index 620de90..9f72efe 100644
> > --- a/fs/nfs/nfs4file.c
> > +++ b/fs/nfs/nfs4file.c
> > @@ -177,8 +177,10 @@ static ssize_t __nfs4_copy_file_range(struct file
> > *file_in, loff_t pos_in,
> > ret = nfs42_proc_copy(file_in, pos_in, file_out, pos_out, count,
> > nss, cnrs, sync);
> > out:
> > - if (!nfs42_files_from_same_server(file_in, file_out))
> > + if (!nfs42_files_from_same_server(file_in, file_out)) {
> > kfree(cn_resp);
> > + cn_resp = NULL;
> > + }
> > if (ret == -EAGAIN)
> > goto retry;
> > return ret;