re: NFSv4.x: Enforce the ca_maxresponsesize_cached on the back channel

re: NFSv4.x: Enforce the ca_maxresponsesize_cached on the back channel

[Dan Carpenter]

Hello Trond Myklebust,

The patch 80f9642724af: "NFSv4.x: Enforce the
ca_maxresponsesize_cached on the back channel" from Jan 23, 2016,
leads to the following static checker warning:

  fs/nfs/callback_proc.c:540 nfs4_callback_sequence()
  warn: inconsistent returns 'spin_lock:&tbl->slot_tbl_lock'.
    Locked on:   line 504
    Unlocked on: line 540

fs/nfs/callback_proc.c
   490          res->csr_highestslotid = tbl->server_highest_slotid;
   491          res->csr_target_highestslotid = tbl->target_highest_slotid;
   492  
   493          status = validate_seqid(tbl, slot, args);
   494          if (status)
   495                  goto out_unlock;
   496          if (!nfs4_try_to_lock_slot(tbl, slot)) {
   497                  status = htonl(NFS4ERR_DELAY);
   498                  goto out_unlock;
                        ^^^^^^^^^^^^^^^
   499          }
   500          cps->slot = slot;
   501  
   502          /* The ca_maxresponsesize_cached is 0 with no DRC */
   503          if (args->csa_cachethis != 0)
   504                  return htonl(NFS4ERR_REP_TOO_BIG_TO_CACHE);

This should also unlock and possibly do other unwinding?

   505  
   506          /*
   507           * Check for pending referring calls.  If a match is found, a
   508           * related callback was received before the response to the original
   509           * call.
   510           */
   511          if (referring_call_exists(clp, args->csa_nrclists, args->csa_rclists)) {
   512                  status = htonl(NFS4ERR_DELAY);
   513                  goto out_unlock;
   514          }

regards,
dan carpenter

Re: NFSv4.x: Enforce the ca_maxresponsesize_cached on the back channel

[Olga Kornievskaia]

Yep that leads to the kernel oops. I have posted a patch. Hopefully
Trond will approve it.

On Thu, May 12, 2016 at 2:11 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Hello Trond Myklebust,
>
> The patch 80f9642724af: "NFSv4.x: Enforce the
> ca_maxresponsesize_cached on the back channel" from Jan 23, 2016,
> leads to the following static checker warning:
>
>   fs/nfs/callback_proc.c:540 nfs4_callback_sequence()
>   warn: inconsistent returns 'spin_lock:&tbl->slot_tbl_lock'.
>     Locked on:   line 504
>     Unlocked on: line 540
>
> fs/nfs/callback_proc.c
>    490          res->csr_highestslotid = tbl->server_highest_slotid;
>    491          res->csr_target_highestslotid = tbl->target_highest_slotid;
>    492
>    493          status = validate_seqid(tbl, slot, args);
>    494          if (status)
>    495                  goto out_unlock;
>    496          if (!nfs4_try_to_lock_slot(tbl, slot)) {
>    497                  status = htonl(NFS4ERR_DELAY);
>    498                  goto out_unlock;
>                         ^^^^^^^^^^^^^^^
>    499          }
>    500          cps->slot = slot;
>    501
>    502          /* The ca_maxresponsesize_cached is 0 with no DRC */
>    503          if (args->csa_cachethis != 0)
>    504                  return htonl(NFS4ERR_REP_TOO_BIG_TO_CACHE);
>
> This should also unlock and possibly do other unwinding?
>
>    505
>    506          /*
>    507           * Check for pending referring calls.  If a match is found, a
>    508           * related callback was received before the response to the original
>    509           * call.
>    510           */
>    511          if (referring_call_exists(clp, args->csa_nrclists, args->csa_rclists)) {
>    512                  status = htonl(NFS4ERR_DELAY);
>    513                  goto out_unlock;
>    514          }
>
> regards,
> dan carpenter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html