• Scott Mayhew's avatar
    auth_gss: fix panic in gss_pipe_downcall() in fips mode · 437b300c
    Scott Mayhew authored
    On Mon, 15 Feb 2016, Trond Myklebust wrote:
    
    > Hi Scott,
    >
    > On Mon, Feb 15, 2016 at 2:28 PM, Scott Mayhew <smayhew@redhat.com> 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 <smayhew@redhat.com>
    > > ---
    > >  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 <smayhew@redhat.com>
    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: default avatarScott Mayhew <smayhew@redhat.com>
    Signed-off-by: default avatarTrond Myklebust <trond.myklebust@primarydata.com>
    437b300c
auth_gss.c 52.9 KB