All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benny Halevy <bhalevy@panasas.com>
To: "J. Bruce Fields" <bfields@fieldses.org>
Cc: linux-nfs@vger.kernel.org
Subject: Re: [PATCH 2/2] nfsd41: mask out unsupported pnfs attributes
Date: Fri, 01 Oct 2010 17:35:05 +0200	[thread overview]
Message-ID: <4CA5FFA9.7090301@panasas.com> (raw)
In-Reply-To: <20101001152032.GF17310@fieldses.org>

On 2010-10-01 17:20, J. Bruce Fields wrote:
> On Fri, Oct 01, 2010 at 05:17:51PM +0200, Benny Halevy wrote:
>> On 2010-10-01 16:48,  J. Bruce Fields wrote:
>>> On Thu, Sep 30, 2010 at 08:47:58PM +0200, Benny Halevy wrote:
>>>> These attributes are valid in NFSv4.1, the just doesn't support them yet.
>>>
>>> The existing code handles unsupported attributes in the operations
>>> themselves.  Perhaps it makes sense to move those checks here, but if
>>> so, explain why, and let's do this for all unsupported attributes, not
>>> just these two.
>>
>> The client can run a DOS attack on the server by requesting invalid attributes
>> and tripping the BUG_ONs in nfsd4_encode_fattr.
> 
> How can they do that?  getattr and readdir, for example, both handle
> this.  But I may well be missing something!
> 

You're right.  I got confused by the pnfsd specific bug I fixed here:
http://marc.info/?l=linux-nfs&m=128587444310102&w=2

In this case LAYOUT_BLKSIZE was marked as supported but nothing
really supported it so we ended up with an invalid result sent
to the client.

So let's drop this patch. sorry.

Benny

>> We can/should also change the BUG_ONs to either report invalid
>> attribute or just silently ignore them, but the client is
>> perfectly entitled to get attrs we don't support :)
> 
> Sure.
> 
>>> Looking back at the spec....  I guess it's only on operations that set
>>> attributes that we return NFS4ERR_ATTRNOTSUPP, and otherwise we silently
>>> ignore them?
>>
>> For the GETATTR case, we just return the attrmask for the attrs we support.
>> IOW:
>>    The server returns an attribute bitmap that
>>    indicates the attribute values that it was able to return, which will
>>    include all attributes requested by the client that are attributes
>>    supported by the server for the target file system.
> 
> OK, makes sense.
> 
> --b.

      reply	other threads:[~2010-10-01 15:35 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-30 18:47 [PATCH 2/2] nfsd41: mask out unsupported pnfs attributes Benny Halevy
2010-09-30 19:20 ` [PATCH 1/2] SQUASHME: pnfsd: FATTR4_WORD2_LAYOUT_BLKSIZE is supported only under CONFIG_PNFSD Benny Halevy
2010-09-30 19:20 ` [PATCH 2/2] pnfsd: mask out unsupported pnfs attributes only when !CONFIG_PNFSD Benny Halevy
2010-10-01 14:48 ` [PATCH 2/2] nfsd41: mask out unsupported pnfs attributes  J. Bruce Fields
2010-10-01 15:17   ` Benny Halevy
2010-10-01 15:20     ` J. Bruce Fields
2010-10-01 15:35       ` Benny Halevy [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4CA5FFA9.7090301@panasas.com \
    --to=bhalevy@panasas.com \
    --cc=bfields@fieldses.org \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.