[s3] CID 1427623 and possible memory leak

[Swen Schillig]

On Fri, 2018-05-25 at 12:19 +0200, Swen Schillig via samba-technical
wrote:
> 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, 

I guess I have to correct myself here.
At least the pointer info6 can be NULL.

The call stack is as follows

winbindd_dual_pam_auth()->winbindd_dual_pam_auth_kerberos()->winbindd_raw_kerberos_login()
->kerberos_return_pac(.... talloc_move is failing => pac_data_ctr==NULL but result==NT_STATUS_OK...)
or 
 pac_data is NULL but still result==NT_STATUS_OK...)

in both cases info6 will be NULL and the succeeding call to map_info6_to_validation from winbindd_dual_pam_auth
will fail with an Explicit null dereference.

This is exactly as I described in my earlier reply to you, it can be very hard to prove / guarantee that
a pointer is set with something useful before de-referencing if that is supposed to happen 3-4 levels up or down
the call-stack.

Any comments ?

Cheers Swen