[PATCH] Address several issues in Samba

[Jeremy Allison]

On Thu, Feb 23, 2017 at 02:26:06PM +0100, Andreas Schneider wrote:
> On Friday, 17 February 2017 23:45:02 CET Jeremy Allison wrote:
> > On Fri, Feb 17, 2017 at 12:09:36PM +0100, Andreas Schneider wrote:
> > > Hello,
> > > 
> > > I'm currently packaging Samba for the next RHEL release. Our covscan tool
> > > found a lot of issues. I would like to address some. The atttached
> > > patchset
> > > fixes 12 issues and adds a modeling file for coverity we can upload to
> > > tell it how tests are working. It can be extended and uploaded. Currently
> > > it is only for cmocka tests.
> > > 
> > > Review and push appreciated.
> > 
> > +1 on all of these except:
> > 
> > ----------------------------------------------------------------------------
> > ------- From 9fe8650643bd9bc5657d52d2507b3b76ef15def7 Mon Sep 17 00:00:00
> > 2001 From: Andreas Schneider <asn at samba.org>
> >  Date: Fri, 17 Feb 2017 10:04:14 +0100
> >  Subject: [PATCH 08/13] s3:winbind: Add a paranoia check that result is not
> >   NULL
> > 
> >  Found by covscan.
> > 
> >  BUG: https://bugzilla.samba.org/show_bug.cgi?id=12592
> > 
> >  Signed-off-by: Andreas Schneider <asn at samba.org>
> >  ---
> >   source3/winbindd/winbindd_list_users.c | 3 +++
> >   1 file changed, 3 insertions(+)
> > 
> >  diff --git a/source3/winbindd/winbindd_list_users.c
> > b/source3/winbindd/winbindd_list_users.c index 9a751a75c5b..aa76b3c4f79
> > 100644
> >  --- a/source3/winbindd/winbindd_list_users.c
> >  +++ b/source3/winbindd/winbindd_list_users.c
> >  @@ -171,6 +171,9 @@ NTSTATUS winbindd_list_users_recv(struct tevent_req
> > *req, return map_nt_error_from_unix(ret);
> >                }
> >        }
> >  +     if (result == NULL) {
> >  +             return NT_STATUS_INTERNAL_ERROR;
> >  +     }
> > 
> >        len = talloc_get_size(result);
> > ----------------------------------------------------------------------------
> > -------
> > 
> > Can you share the coverity error message on this one ?
> > 
> > Returning response->data.num_entries == 0 with
> > NT_STATUS_OK (which is what you'd get if result == NULL)
> > looks OK to me.
> 
> 2. Defect type: FORWARD_NULL
> 3. samba-4.6.0rc3/source3/winbindd/winbindd_list_users.c:159: assign_zero: 
> Assigning: "result" = "NULL".
> 16. samba-4.6.0rc3/source3/winbindd/winbindd_list_users.c:186: var_deref_op: 
> Dereferencing null pointer "result".

Ah, it's a false positive. If result == NULL
then:

175         len = talloc_get_size(result);

will return len = 0, so we can't get to the
problematic code.

If you want to make the false positive message
go away, try the following patch. This doesn't
change the semantics of winbindd_list_users_recv().

Cheers,

	Jeremy.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-s3-winbind-work-around-coverity-false-positive.patch
Type: text/x-diff
Size: 856 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170223/357e6646/0001-s3-winbind-work-around-coverity-false-positive.diff>