Re: [PATCH] nfs4: Fix memory corruption due to not expected FS_LOCATIONS v3

[Vitaliy Gusev <gusev.vitaliy@nexenta.com>]

On Tue, 2011-03-22 at 19:10 -0400, Trond Myklebust wrote:
> On Wed, 2011-03-23 at 01:39 +0300, Vitaliy Gusev wrote:
> > On Tue, 2011-03-22 at 17:46 -0400, Trond Myklebust wrote:
> > 
> > > Why are you limiting this to fs_locations?
> > 
> > Sorry, I haven't seen any other attribute that can cause memory
> > corruption.
> 
> decode_attr_filehandle() will certainly cause an Oops if someone inserts one.

Hmm, Thanks! But it seems that here plain check on NULL is enough...

> 
> There may be more occurrences in the future if/when we need to add
> support for more attributes.
> 
> > >  As I believe I said earlier,
> > > any attribute that we didn't explicitly request is an error and can
> > > cause corruption in the client.
> > 
> > There are checks on each decode attr function. For instance,
> > decode_attr_filehandle:
> > 
> >  	if (unlikely(bitmap[0] & (FATTR4_WORD0_FILEHANDLE - 1U)))
> > 		return -EIO;
> > 
> > So any non handled attribute raise EIO error.
> 
> If we're going to do this, then I suggest we add an 'expected bitmask'
> argument to 'decode_attr_bitmap()'. If the server sets any bit that is
> not part of this expected bitmask, then we can immediately return an EIO
> without having to decode any further.
> 
> That will allow us to get rid of the 'if (unlikely(bitmap[] & ....)'
> tests altogether.

You are right. I'll do as you said.

-- 
Thanks,
Vitaliy Gusev