Received-SPF: pass (google.com: domain of linux-nfs+bounces-2873-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
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 08:00:04 -0400
Message-ID: <13EE0F08-5567-48B8-A7C2-88A086FBDA89@redhat.com>
In-Reply-To: <ae03a217-e643-4127-bb4a-4993ad6a9d00@moroto.mountain>
References: <ae03a217-e643-4127-bb4a-4993ad6a9d00@moroto.mountain>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

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.

Ben