From: Benjamin Coddington <bcodding@redhat.com>
To: Dan Carpenter <dan.carpenter@linaro.org>
Cc: linux-nfs@vger.kernel.org, Chuck Lever III <chuck.lever@oracle.com>
Subject: Re: [bug report] NFSv4: Fix free of uninitialized nfs4_label on referral lookup.
Date: Wed, 17 Apr 2024 09:51:48 -0400 [thread overview]
Message-ID: <F0002E44-B2E9-4DE3-BF3B-771F814A8EE1@redhat.com> (raw)
In-Reply-To: <7c4df27b-9698-4d49-a35f-9395b75348d3@moroto.mountain>
On 17 Apr 2024, at 8:40, Dan Carpenter wrote:
> On Wed, Apr 17, 2024 at 08:00:04AM -0400, Benjamin Coddington wrote:
>> On 15 Apr 2024, at 4:08, Dan Carpenter wrote:
>>
>>> [ Why is Smatch only complaining now, 2 years later??? It is a mystery.
>>> -dan ]
>>>
>>> Hello Benjamin Coddington,
>>
>> Hi Dan!
>>
>>> Commit c3ed222745d9 ("NFSv4: Fix free of uninitialized nfs4_label on
>>> referral lookup.") from May 14, 2022 (linux-next), leads to the
>>> following Smatch static checker warning:
>>>
>>> fs/nfs/nfs4state.c:2138 nfs4_try_migration()
>>> warn: missing error code here? 'nfs_alloc_fattr()' failed. 'result' = '0'
>>>
>>> fs/nfs/nfs4state.c
>>> 2115 static int nfs4_try_migration(struct nfs_server *server, const struct cred *cred)
>>> 2116 {
>>> 2117 struct nfs_client *clp = server->nfs_client;
>>> 2118 struct nfs4_fs_locations *locations = NULL;
>>> 2119 struct inode *inode;
>>> 2120 struct page *page;
>>> 2121 int status, result;
>>> 2122
>>> 2123 dprintk("--> %s: FSID %llx:%llx on \"%s\"\n", __func__,
>>> 2124 (unsigned long long)server->fsid.major,
>>> 2125 (unsigned long long)server->fsid.minor,
>>> 2126 clp->cl_hostname);
>>> 2127
>>> 2128 result = 0;
>>> ^^^^^^^^^^^
>>>
>>> 2129 page = alloc_page(GFP_KERNEL);
>>> 2130 locations = kmalloc(sizeof(struct nfs4_fs_locations), GFP_KERNEL);
>>> 2131 if (page == NULL || locations == NULL) {
>>> 2132 dprintk("<-- %s: no memory\n", __func__);
>>> 2133 goto out;
>>> ^^^^^^^^
>>> Success.
>>>
>>> 2134 }
>>> 2135 locations->fattr = nfs_alloc_fattr();
>>> 2136 if (locations->fattr == NULL) {
>>> 2137 dprintk("<-- %s: no memory\n", __func__);
>>> --> 2138 goto out;
>>> ^^^^^^^^^
>>> Here too.
>>
>> My patch was following the precedent set by c9fdeb280b8cc. I believe the
>> idea is that the function can fail without an error and the client will
>> retry the next time the server says -NFS4ERR_MOVED.
>>
>> Is there a way to appease smatch here? I don't have a lot of smatch
>> smarts.
>
> Generally, I tell people to just ignore it. Anyone with questions can
> look up this email thread.
>
> But if you really wanted to silence it, Smatch counts it as intentional
> if the "result = 0;" is within five lines of the goto out.
Good to know! In this case, I think the maintainers would show annoyance
with that sort of patch. A comment here about the successful return code on
an allocation failure would have avoided this, and I probably should have
recognized this patch might create an issue and inserted one. Thanks for
the report.
Ben
next prev parent reply other threads:[~2024-04-17 13:51 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-15 8:08 [bug report] NFSv4: Fix free of uninitialized nfs4_label on referral lookup Dan Carpenter
2024-04-17 12:00 ` Benjamin Coddington
2024-04-17 12:40 ` Dan Carpenter
2024-04-17 13:51 ` Benjamin Coddington [this message]
2024-04-17 15:08 ` Dan Carpenter
2024-04-17 18:30 ` Benjamin Coddington
2024-04-17 18:52 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=F0002E44-B2E9-4DE3-BF3B-771F814A8EE1@redhat.com \
--to=bcodding@redhat.com \
--cc=chuck.lever@oracle.com \
--cc=dan.carpenter@linaro.org \
--cc=linux-nfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.