From: "J. Bruce Fields" <[email protected]>
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 <[email protected]>
---
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
On Wed, 2013-10-02 at 09:48 -0400, J. Bruce Fields wrote:
+AD4- From: +ACI-J. Bruce Fields+ACI- +ADw-bfields+AEA-redhat.com+AD4-
+AD4-
+AD4- Failure of this assertion would indeed be serious, but as we've seen a
+AD4- BUG() in a worker thread (this can run from rpciod context) can screw up
+AD4- the system so badly that the oops doesn't even get to the logs+ADs- better
+AD4- to WARN() and soldier on.
+AD4-
+AD4- Signed-off-by: J. Bruce Fields +ADw-bfields+AEA-redhat.com+AD4-
+AD4- ---
+AD4- net/sunrpc/auth+AF8-gss/auth+AF8-gss.c +AHw- 2 +--
+AD4- 1 file changed, 1 insertion(+-), 1 deletion(-)
+AD4-
+AD4- diff --git a/net/sunrpc/auth+AF8-gss/auth+AF8-gss.c b/net/sunrpc/auth+AF8-gss/auth+AF8-gss.c
+AD4- index 0846566..254a9f0 100644
+AD4- --- a/net/sunrpc/auth+AF8-gss/auth+AF8-gss.c
+AD4- +-+-+- b/net/sunrpc/auth+AF8-gss/auth+AF8-gss.c
+AD4- +AEAAQA- -454,7 +-454,7 +AEAAQA- static void gss+AF8-encode+AF8-v1+AF8-msg(struct gss+AF8-upcall+AF8-msg +ACo-gss+AF8-msg,
+AD4- gss+AF8-msg-+AD4-msg.len +-+AD0- len+ADs-
+AD4-
+AD4- gss+AF8-msg-+AD4-msg.data +AD0- gss+AF8-msg-+AD4-databuf+ADs-
+AD4- - BUG+AF8-ON(gss+AF8-msg-+AD4-msg.len +AD4- UPCALL+AF8-BUF+AF8-LEN)+ADs-
+AD4- +- WARN+AF8-ON+AF8-ONCE(gss+AF8-msg-+AD4-msg.len +AD4- UPCALL+AF8-BUF+AF8-LEN)+ADs-
+AD4- +AH0-
+AD4-
+AD4- static struct gss+AF8-upcall+AF8-msg +ACo-
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+AEA-netapp.com
http://www.netapp.com
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" <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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.
From: "J. Bruce Fields" <[email protected]>
Signed-off-by: J. Bruce Fields <[email protected]>
---
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