Re: [PATCH] sunrpc: on successful gss error pipe write, don't return error

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 18 Dec 2009 09:47:52 -0500
Trond Myklebust <Trond.Myklebust@xxxxxxxxxx> wrote:

> On Fri, 2009-12-18 at 09:39 -0500, Jeff Layton wrote: 
> > Yeah, we could do that with the existing code. I sort of don't like
> > that because it's hard to know if other functions could eventually
> > return an EACCES for another reason and then that error would bubble up
> > to this function. If you think it's the right thing to do though, I'm
> > OK with it.
> 
> The error EACCES means 'you are not authorised'. I don't see how it can
> be ambiguous here.
> 

I guess I was thinking that the pipe writer might not be authorized to
import the sec context for some reason, which would be a different
situation. If that'll never be the case, then a single error code is
probably fine. I'll respin it to just special case EACCES for now and
will plan to have it special case the other error when that work is
more fleshed out.

> > FWIW: The reason I'm poking around in here is because I'm taking a stab
> > at fixing the problem where syscalls start returning errors when a krb5
> > ticket expires.
> > 
> > As part of that, I want to have gssd send a more granular error code
> > and have the kernel adjust what it does accordingly. I'd like to have
> > it retry the upcall indefinitely when there's an expired credcache, and
> > return an error when there's no credcache at all).
> 
> That makes sense.
> 
> > Without a separate downcall error field, we'll need to special case at
> > least 2 different errors -- one for a "real" EACCES and one that
> > indicates that the ticket expired and the upcall should be retried
> > instead.
> 
> We can find another error for the 'ticket expired' case. EKEYEXPIRED
> springs to mind...
> 

That's exactly the error I was going to use.

> > > BTW: while looking at this, I spotted a nasty bug in
> > > gss_import_sec_context_kerberos(). If the kzalloc() call fails, we will
> > > return a random error code since 'p' still points to a valid memory
> > > location...
> > 
> > Good catch. Do you want fix that one, or should I?
> > 
> 
> I can do it, if you like.
> 

Sure. I see another problem in this area too. gss_import_sec_context
can return GSS_S_FAILURE which is unsigned and positive when cast to a
signed value. gss_import_sec_context checks for a negative return from
that function though. Should it be checking for non-zero instead?

-- 
Jeff Layton <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux