Re: [PATCH] nfs: system crashes after NFS4ERR_MOVED recovery

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Bill,

On Wed, 2018-02-07 at 14:53 -0600, Bill Baker wrote:
> nfs4_update_server unconditionally releases the nfs_client for the
> source server. If migration fails, this can cause the source server's
> nfs_client struct to be left with a low reference count, resulting in
> use-after-free.
> 
> NFS: state manager: migration failed on NFSv4 server nfsvmu10 with
> error 6
> WARNING: CPU: 16 PID: 17960 at fs/nfs/client.c:281 
> nfs_put_client+0xfa/0x110 [nfs]()
>          nfs_put_client+0xfa/0x110 [nfs]
>          nfs4_run_state_manager+0x30/0x40 [nfsv4]
>          kthread+0xd8/0xf0
> 
> BUG: unable to handle kernel NULL pointer dereference at
> 00000000000002a8
>          nfs4_xdr_enc_write+0x6b/0x160 [nfsv4]
>          rpcauth_wrap_req+0xac/0xf0 [sunrpc]
>          call_transmit+0x18c/0x2c0 [sunrpc]
>          __rpc_execute+0xa6/0x490 [sunrpc]
>          rpc_async_schedule+0x15/0x20 [sunrpc]
>          process_one_work+0x160/0x470
>          worker_thread+0x112/0x540
>          kthread+0xd8/0xf0
> 
> Reported-by: Helen Chao <helen.chao@xxxxxxxxxx>
> Fixes: 32e62b7c3ef0 ("NFS: Add nfs4_update_server")
> Signed-off-by: Bill Baker <bill.baker@xxxxxxxxxx>
> Reviewed-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> ---
>   fs/nfs/nfs4client.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
> index 65a7e5d..c818cdd 100644
> --- a/fs/nfs/nfs4client.c
> +++ b/fs/nfs/nfs4client.c
> @@ -1226,11 +1226,11 @@ int nfs4_update_server(struct nfs_server 
> *server, const char *hostname,
>                                  clp->cl_proto, clnt->cl_timeout,
>                                  clp->cl_minorversion, net);
>          clear_bit(NFS_MIG_TSM_POSSIBLE, &server->mig_status);
> -       nfs_put_client(clp);
>          if (error != 0) {
>                  nfs_server_insert_lists(server);
>                  return error;
>          }
> +       nfs_put_client(clp);
> 
>          if (server->nfs_client->cl_hostname == NULL)
>                  server->nfs_client->cl_hostname = kstrdup(hostname, 
> GFP_KERNEL);
> 

That looks almost right. Isn't there now a reference leak if
nfs4_set_client() returns -ELOOP? I think that is really down to an
existing bug in nfs4_set_client() rather than in your fix, however the
fix does re-expose that bug.

Cheers
  Trond

-- 
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@xxxxxxxxxxxxxxx
��.n��������+%������w��{.n�����{��w���jg��������ݢj����G�������j:+v���w�m������w�������h�����٥




[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux