[PATCH] s3-lib/util: fix read across end of namelist string

[Jeremy Allison]

On Wed, Apr 09, 2014 at 01:16:38AM +0200, Michael Adam wrote:
> Hi Jeremy,
> 
> On 2014-04-08 at 10:42 -0700, Jeremy Allison wrote:
> > On Tue, Apr 08, 2014 at 10:26:25AM -0700, Jeremy Allison wrote:
> > > On Tue, Apr 08, 2014 at 10:25:40AM +0200, Bjoern Baumbach wrote:
> > > > Hi!
> > > > 
> > > > If the namelist, which set_namearray receives, is not terminated with a
> > > >  '/', we try to read the next character behind the string termination '\0'.
> > > > In the case that the namelist is followed by a (more or less) valid
> > > > string, we could produce several effects like unintentional vetoed,
> > > > hidden or non-oplocked files or failures like:
> > > > "Conversion error: Incomplete multibyte sequence(..."
> > > > 
> > > > Please find attached the proposed patch :-)
> > > 
> > > Reviewed-and-pushed. Congratulations - that
> > > was a *GREAT* catch ! Thanks a lot Bjoern.
> 
> Indeed! I was part of the debugging and I wonder
> how we have survived this so long... :-)
> 
> > What do you think of the following additional
> > patch ? Your fix addresses the problem and
> > fixes the bug, but leaves the faulty logic
> > inside of the while loops. This patch fixes
> > the faulty logic as well, leaving the code
> > (hopefully) cleaner and easier to follow,
> > but it isn't strictly needed.
> > 
> > Comments + reviews ? Is it worth adding ?
> 
> This is definitely an improvement.

Thanks !

> ==> "Reviewed-by: Michael Adam <obnox at samba.org>"
> 
> If I get it right, this patch would also
> fix the bug w/o Björn's namelist_end patch, correct?

Yes.

> But I think we can keep them both.

Yeah, belt-and-braces IMHO.

> I can push it to master, but I have a small addition,
> making the if (name_end) check explit to read
> if (name_and != NULL)  - attached.
> 
> Jeremy, feel free to add it on top if you like it
> or squash it with your patch which touches this
> context anyways.

OK, I'll squash and push - thanks !

Jeremy.