linux-kernel - Re: [PATCH] NFS: simplify check for freeing cn

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1bfd05a9-5245-9094-b382-f01c8b35f761@redhat.com>
Date:   Sun, 16 Jan 2022 08:26:53 -0800
From:   Tom Rix <trix@...hat.com>
To:     Trond Myklebust <trondmy@...merspace.com>,
        "anna.schumaker@...app.com" <anna.schumaker@...app.com>
Cc:     "linux-nfs@...r.kernel.org" <linux-nfs@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] NFS: simplify check for freeing cn_resp


On 1/16/22 7:47 AM, Trond Myklebust wrote:
> Hi Tom,
>
> On Sun, 2022-01-16 at 06:43 -0800, trix@...hat.com wrote:
>> From: Tom Rix <trix@...hat.com>
>>
>> nfs42_files_from_same_server() is called to check if freeing
>> cn_resp is required.  Instead of calling a function, check
>> the pointer.
>>
>> Signed-off-by: Tom Rix <trix@...hat.com>
>> ---
>>   fs/nfs/nfs4file.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
>> index e79ae4cbc395e..677631ea4cfb3 100644
>> --- a/fs/nfs/nfs4file.c
>> +++ b/fs/nfs/nfs4file.c
>> @@ -180,7 +180,7 @@ 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 (cn_resp)
>>                  kfree(cn_resp);
> The kernel convention in these circumstances is to just skip the NULL
> pointer check, since kfree() does that anyway.

Yes. I'll respin.

Tom

>
>>          if (ret == -EAGAIN)
>>                  goto retry;