Re: [PATCH 1/2] rpc: change BUG to WARN in gss_encode_v1_msg

["J. Bruce Fields" <bfields@fieldses.org>]

On Wed, Oct 16, 2013 at 06:17:54PM +0000, Myklebust, Trond wrote:
> On Wed, 2013-10-02 at 09:48 -0400, J. Bruce Fields wrote:
> > From: "J. Bruce Fields" <bfields@redhat.com>
> > 
> > Failure of this assertion would indeed be serious, but as we've seen a
> > BUG() in a worker thread (this can run from rpciod context) can screw up
> > the system so badly that the oops doesn't even get to the logs; better
> > to WARN() and soldier on.
> > 
> > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > ---
> >  net/sunrpc/auth_gss/auth_gss.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
> > index 0846566..254a9f0 100644
> > --- a/net/sunrpc/auth_gss/auth_gss.c
> > +++ b/net/sunrpc/auth_gss/auth_gss.c
> > @@ -454,7 +454,7 @@ static void gss_encode_v1_msg(struct gss_upcall_msg *gss_msg,
> >  	gss_msg->msg.len += len;
> >  
> >  	gss_msg->msg.data = gss_msg->databuf;
> > -	BUG_ON(gss_msg->msg.len > UPCALL_BUF_LEN);
> > +	WARN_ON_ONCE(gss_msg->msg.len > UPCALL_BUF_LEN);
> >  }
> >  
> >  static struct gss_upcall_msg *
> 
> Umm.... Why aren't we using scnprintf() in order to prevent string
> buffer overflows before they happen?

Yeah, this should really be caught earlier.

And we want to actually fail, not give gssd a truncated request.

--b.