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

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

[J. Bruce Fields]

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 *
-- 
1.7.9.5

[PATCH 2/2] sunrpc: comment typo fix

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 net/sunrpc/xprtsock.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index ee03d35..940d723 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2859,8 +2859,8 @@ static struct rpc_xprt *xs_setup_bc_tcp(struct xprt_create *args)
 	if (args->bc_xprt->xpt_bc_xprt) {
 		/*
 		 * This server connection already has a backchannel
-		 * export; we can't create a new one, as we wouldn't be
-		 * able to match replies based on xid any more.  So,
+		 * transport; we can't create a new one, as we wouldn't
+		 * be able to match replies based on xid any more.  So,
 		 * reuse the already-existing one:
 		 */
 		 return args->bc_xprt->xpt_bc_xprt;
-- 
1.7.9.5

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

[Myklebust, Trond]

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?

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

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

[J. Bruce Fields]

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.