Linux NFS development
 help / color / mirror / Atom feed
From: Simo Sorce <simo@redhat.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: linux-nfs@vger.kernel.org, Bruce Fields <bfields@redhat.com>
Subject: re: SUNRPC: Add RPC based upcall mechanism for RPCGSS auth
Date: Mon, 06 May 2013 11:54:53 -0400	[thread overview]
Message-ID: <1367855693.2976.205.camel@willson.li.ssimo.org> (raw)
In-Reply-To: <20130506154435.GA14732@elgon.mountain>

On Mon, 2013-05-06 at 18:44 +0300, Dan Carpenter wrote:
> Hello Simo Sorce,
> 
> The patch 1d658336b05f: "SUNRPC: Add RPC based upcall mechanism for 
> RPCGSS auth" from May 25, 2012, leads to the following warning:
> "net/sunrpc/auth_gss/gss_rpc_xdr.c:30 gssx_check_pointer()
> 	 warn: signedness bug returning '(-28)'"
> 
> net/sunrpc/auth_gss/gss_rpc_xdr.c
>     24  static bool gssx_check_pointer(struct xdr_stream *xdr)
>     25  {
>     26          __be32 *p;
>     27  
>     28          p = xdr_reserve_space(xdr, 4);
>     29          if (unlikely(p == NULL))
>     30                  return -ENOSPC;
>                                ^^^^^^^
> This is casted implicitly to "true".  Functions named "check" are a bad
> idea anyways because it's not clear what the return value will be.  It's
> better to use "gssx_pointer_ok()" or "valid" where obviously true means
> that the pointer is ok.
> 
>     31          return *p?true:false;
> 
> We just reserved "*p" so doesn't this point to uninitialized data?  It
> points to a __be32.  So we're not really checking a pointer we're
> checking that __be32 is non-zero.
> 
>     32  }
> 
> regards,
> dan carpenter

Dan,
thanks a lot for pointing this one out.

Bruce,
we should fix it in 3.10 if we can make it.

probably easiest way is to kill lines 29/30 and do:
return (p && *p) ? true : false;

It would be also ok to change the name to gssx_pointer_is_valid() I
guess.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York


  reply	other threads:[~2013-05-06 15:54 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-06 15:44 SUNRPC: Add RPC based upcall mechanism for RPCGSS auth Dan Carpenter
2013-05-06 15:54 ` Simo Sorce [this message]
2013-05-07 21:49   ` J. Bruce Fields
2013-05-07 21:57     ` Simo Sorce

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=1367855693.2976.205.camel@willson.li.ssimo.org \
    --to=simo@redhat.com \
    --cc=bfields@redhat.com \
    --cc=dan.carpenter@oracle.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox