[PATCH] Fix bug #10406 - vfs_dirsort can trample on its own private data.

[Andrew Bartlett]

On Tue, 2014-02-11 at 10:43 -0800, Jeremy Allison wrote:
> On Tue, Feb 11, 2014 at 11:14:58PM +1300, Andrew Bartlett wrote:
> > On Mon, 2014-02-10 at 21:31 -0800, Jeremy Allison wrote:
> > > Now tested and confirmed good by the OEM who reported it,
> > > this patch fixes the problem that the current dirsort
> > > module only keeps one active private data pointer on
> > > the connection struct, so opening a second directory
> > > on the same connection will overwrite it.
> > > 
> > > Please review & push if happy !
> > 
> > Can you make sure that in 'make test' we now run raw.search and the os2
> > delete test against the tmpenc share (or perhaps better a new share for
> > this purpose) that uses dirsort?  It would be great to ensure this is
> > tested.
> 
> New, washes whiter ! Cleans with more enzyme action ! :-).
> 
> make test now included :-).
> 
> Please review and push if happy !

Looking over the code, I do wonder about the risks we now have for a
client-based DoS.  

I know this is a boutique module - but if an OEM is using it, then it
suggests that it could be deployed in situations where users could abuse
it. 

My worry is:  What happens if a user opens a large directory a large
number of times.  It seems the server-side memory use would be
unbounded, as would the length of the list. 

What do you think we should do about this?  Just warn in the manpage, or
do something more drastic?

Certainly DoS potential is much better than the data corruption
(essentially) issues the module had before (the incorrect 7zip
archives), but it just worries me a little.

Is there any way to do this with just one entry in the cache, and just
re-fetch and re-sort if a different directory is opened?

What do you think?

Andrew Bartlett

-- 
Andrew Bartlett
http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba