auth_gss: fix panic in gss_pipe_downcall() in fips mode

Commit Message

Scott Mayhew Feb. 16, 2016, 9:20 p.m. UTC

On Mon, 15 Feb 2016, Trond Myklebust wrote:

> Hi Scott,
> 
> On Mon, Feb 15, 2016 at 2:28 PM, Scott Mayhew <[e-mail]> wrote:
> > md5 is disabled in fips mode, and attempting to import a gss context
> > using md5 while in fips mode will result in crypto_alg_mod_lookup()
> > returning -ENOENT, which will make its way back up to
> > gss_pipe_downcall(), where the BUG() is triggered.  Handling the -ENOENT
> > allows for a more graceful failure.
> >
> > Signed-off-by: Scott Mayhew <[e-mail]>
> > ---
> >  net/sunrpc/auth_gss/auth_gss.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
> > index 799e65b..c30fc3b 100644
> > --- a/net/sunrpc/auth_gss/auth_gss.c
> > +++ b/net/sunrpc/auth_gss/auth_gss.c
> > @@ -737,6 +737,9 @@ gss_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
> >                 case -ENOSYS:
> >                         gss_msg->msg.errno = -EAGAIN;
> >                         break;
> > +               case -ENOENT:
> > +                       gss_msg->msg.errno = -EPROTONOSUPPORT;
> > +                       break;
> >                 default:
> >                         printk(KERN_CRIT "%s: bad return from "
> >                                 "gss_fill_context: %zd\n", __func__, err);
> > --
> > 2.4.3
> >
> 
> Well debugged, but I unfortunately do have to ask if this patch is
> sufficient? In addition to -ENOENT, and -ENOMEM, it looks to me as if
> crypto_alg_mod_lookup() can also fail with -EINTR, -ETIMEDOUT, and
> -EAGAIN. Don't we also want to handle those?

You're right, I was focusing on the panic that I could easily reproduce.
I'm still not sure how I could trigger those other conditions.

> 
> In fact, peering into the rats nest that is
> gss_import_sec_context_kerberos(), it looks as if that is just a tiny
> subset of all the errors that we might run into. Perhaps the right
> thing to do here is to get rid of the BUG() (but keep the above
> printk) and just return a generic error?

That sounds fine to me -- updated patch attached.

-Scott
From d54c6b64a107a90a38cab97577de05f9a4625052 Mon Sep 17 00:00:00 2001
From: Scott Mayhew <[e-mail]>
Date: Mon, 15 Feb 2016 15:12:19 -0500
Subject: [PATCH] auth_gss: remove the BUG() from gss_pipe_downcall()

Instead return a generic error via gss_msg->msg.errno.  None of the
errors returned by gss_fill_context() should necessarily trigger a
kernel panic.

Signed-off-by: Scott Mayhew <[e-mail]>
---
 net/sunrpc/auth_gss/auth_gss.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Patch

diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
index 799e65b..cabf586 100644
--- a/net/sunrpc/auth_gss/auth_gss.c
+++ b/net/sunrpc/auth_gss/auth_gss.c
@@ -740,7 +740,7 @@  gss_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
 		default:
 			printk(KERN_CRIT "%s: bad return from "
 				"gss_fill_context: %zd\n", __func__, err);
-			BUG();
+			gss_msg->msg.errno = -EIO;
 		}
 		goto err_release_msg;
 	}