[s3] CID 1427623 and possible memory leak

[Swen Schillig]

On Fri, 2018-05-25 at 11:39 +0200, Volker Lendecke wrote:
> On Fri, May 25, 2018 at 11:04:52AM +0200, Swen Schillig via samba-
> technical wrote:
> > From 500b7733730d8186e03e236a467d12ddbb32e399 Mon Sep 17 00:00:00
> > 2001
> > From: Swen Schillig <swen at vnet.ibm.com>
> > Date: Fri, 25 May 2018 10:46:52 +0200
> > Subject: [PATCH 2/2] [s3] CID 1427623: Explicit NULL dereference.
> > 
> > Signed-off-by: Swen Schillig <swen at vnet.ibm.com
> 
> Can you explain how this fixes 1427623? 1427623 is in winbindd_pam.c.
> I would rather fix this in winbindd_pam.c. Coverity is very bad at
> understanding NTSTATUS, there is TONS of those defects.
> 
> Can you figure out how to tell Coverity that in winbindd_pam.c
> "info3"
> is !=NULL if and only if NT_STATUS_OK(result) is true? This would fix
> dozens of defects in one run.

You're right, neither the info3 nor info6 coverity findings are real
vital issues, however, I thought it is a good idea to asure a NON-NULL
de-reference in the copy_netr_SamInfo3 / copy_netr_SamInfo6 functions
for coverity's sake and all developers having to hop a good few numbers
up the call-stack to make sure it is correct...for now !

That's why I was looking around to other consumers being in a similar
situation regarding the info[36] pointers and a I saw a good few having
those checks, therefore, I added the ones we're currently looking at.

However I know that we cannot / should not add those checks wherever
we're (currently) trusting the caller...that's why I didn't even look
at the numerous coverity tainted-var findings.

So where does this leave us.... I guess you could NAK those patches
as they do not really fix a vital issue...but you could as well +1 for
the above described reasons.

Either way, thanks for your review, really appreciated.

Cheers Swen