Re: [PATCH 5/6] SUNRPC: Add RPC based upcall mechanism for RPCGSS auth

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

 



On Fri, Apr 12, 2013 at 06:21:11PM +0000, Myklebust, Trond wrote:
> > -----Original Message-----
> > From: J. Bruce Fields [mailto:bfields@xxxxxxxxxxxx]
> > Sent: Friday, April 12, 2013 2:12 PM
> > To: Myklebust, Trond
> > Cc: J. Bruce Fields; linux-nfs@xxxxxxxxxxxxxxx; chuck.lever@xxxxxxxxxx;
> > simo@xxxxxxxxxx
> > Subject: Re: [PATCH 5/6] SUNRPC: Add RPC based upcall mechanism for
> > RPCGSS auth
> > 
> > Trond, apologies, I'm just getting back to this and had a couple
> > questions:
> > 
> > On Thu, Feb 21, 2013 at 06:35:46PM +0000, Myklebust, Trond wrote:
> > > On Thu, 2013-02-21 at 11:38 -0500, J. Bruce Fields wrote:
> > > > +static int gssx_dec_buffer(struct xdr_stream *xdr,
> > > > +			   gssx_buffer *buf)
> > > > +{
> > > > +	u32 length;
> > > > +	__be32 *p;
> > > > +
> > > > +	p = xdr_inline_decode(xdr, 4);
> > > > +	if (unlikely(p == NULL))
> > > > +		return -ENOSPC;
> > > > +
> > > > +	length = be32_to_cpup(p);
> > > > +	p = xdr_inline_decode(xdr, length);
> > >
> > > combine for efficiency with the 4 byte allocation above.
> > 
> > This was just a think-o on your part, right?  There's no way to decode the
> > whole thing in one gulp without knowing the length ahead of time.
> > Or am I missing something?
> 
> No. You are right, that was a brain-fart...
> 
> > > > +static int gssx_dec_option_array(struct xdr_stream *xdr,
> > > > +				 struct gssx_option_array *oa)
> > > > +{
> > > > +	struct svc_cred *creds;
> > > > +	u32 count, i;
> > > > +	__be32 *p;
> > > > +	int err;
> > > > +
> > > > +	p = xdr_inline_decode(xdr, 4);
> > > > +	if (unlikely(p == NULL))
> > > > +		return -ENOSPC;
> > > > +	count = be32_to_cpup(p++);
> > > > +	if (count != 0) {
> > > > +		/* we recognize only 1 currently: CREDS_VALUE */
> > > > +		oa->count = 1;
> > > > +
> > > > +		oa->data = kmalloc(sizeof(struct gssx_option), GFP_KERNEL);
> > >
> > > Sleeping kmallocs inside XDR encode/decode routines is strongly
> > > discouraged. Particularly so for something that can be preallocated.
> > 
> > And here: I don't mind doing that, but: there's no real problem here as long
> > as we're only calling this stuff synchronously, right?
> 
> If you can guarantee that it won't ever be called from rpciod, then fine.
> However anything that might end up being called from a rpciod context must at the very least guarantee that there are no __GFP_FS allocations, since those can deadlock the NFS client.

Got it, thanks

--b.
--
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