[NTLMSSP] s3: Fix some valgrind errors

Andrew Bartlett abartlet at samba.org
Mon Jun 21 07:06:30 MDT 2010


On Mon, 2010-06-21 at 15:24 -0500, Volker Lendecke wrote:
> The branch, master has been updated
>        via  15297ee... s3: Fix some valgrind errors
>       from  6227eac... smbtorture: Fixx off-by-one command line parsing.
> 
> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> 
> 
> - Log -----------------------------------------------------------------
> commit 15297eea0e6b1e95ddb9e2ccd25ff454a405c351
> Author: Volker Lendecke <vl at samba.org>
> Date:   Mon Jun 21 22:20:10 2010 +0200
> 
>     s3: Fix some valgrind errors
>     
>     With -d 10, there were a ton of uninitialized variables: The "NegotiateFlags"
>     in the automatically parsed ntlmssp structures were not initialized.
>     
>     This also cleans up the talloc use a bit: do early TALLOC_FREE()
>     
>     Günther, please check!
>     
>     Thanks,
>     
>     Volker

Volker,

Indeed, the problem here is actually quite subtle, and as you have
noticed these structures cannot be parsed using the information on the
wire alone - the previous state of the negotiation is required to
understand the format of the strings at the stage that the strings
appear in the structure.

The problem is that some of the pointers to strings appear before the
NegotiateFlags that we want to use to know if they are ASCII or
Unicode. 

We actually parse the strings later - but by then we have acted on the
yet to be parsed values.

Your fix is correct (but a comment explaining this would be good) - to
know in the handling of the Authenticate packet the agreed state of the
negotiation, from the Negotiate and Challenge packets, and therefore not
need to read the wire flags too early.  An additional 'belts and braces'
would be to assert that the final flags are equal (at least for the
Unicode) bit, but no real client uses ASCII here anyway. 

I'm sorry I didn't write about this to the list before, as I came across
this when ensuring s3compat was valgrind clean.  (I think I just
mentioned it to gd).  

Andrew Bartlett



More information about the samba-technical mailing list