Bug 8564 - pam_winbind segfault in pam_sm_authenticate()
Summary: pam_winbind segfault in pam_sm_authenticate()
Status: RESOLVED FIXED
Alias: None
Product: Samba 3.6
Classification: Unclassified
Component: Winbind (show other bugs)
Version: unspecified
Hardware: All All
: P5 normal
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-02 16:28 UTC by David Disseldorp
Modified: 2012-10-29 10:54 UTC (History)
2 users (show)

See Also:

Attachments
proposed fix (878 bytes, patch)
2011-11-02 16:40 UTC, David Disseldorp 		no flags Details
proposed fix rb1 (879 bytes, patch)
2011-11-02 16:55 UTC, David Disseldorp 		ddiss: review? (gd)
asn: review+
Details
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description David Disseldorp 2011-11-02 16:28:12 UTC 
pam_sm_authenticate() calls _pam_winbind_init_context() to initialise a pam winbind context structure.

If context initialisation fails, the pam_sm_authenticate() cleanup path attempts to deference the uninitialised (null) context pointer.

The relevant nsswitch/pam_winbind.c code excerpts are:

2584 PAM_EXTERN
2585 int pam_sm_authenticate(pam_handle_t *pamh, int flags,
2586                         int argc, const char **argv)
2587 {
...
2597         struct pwb_context *ctx = NULL;
2598 
2599         retval = _pam_winbind_init_context(pamh, flags, argc, argv, &ctx);
2600         if (retval) {
2601                 goto out;
2602         }
...
2716 out:
...
2736         _PAM_LOG_FUNCTION_LEAVE("pam_sm_authenticate", ctx, retval);
2737 
2738         TALLOC_FREE(ctx);
2739 
2740         return retval;
2741 }

_PAM_LOG_FUNCTION_LEAVE macro dereferences ctx, causing the segfault.
Comment 1 David Disseldorp 2011-11-02 16:40:52 UTC 
Created attachment 7052 [details]
proposed fix

The following changes since commit 2107ba5be87d6a9f7691cfcbe5411fa7174120b7:

  ldb: fix compiler warning (2011-11-02 16:51:24 +0100)

are available in the git repository at:
  git://git.samba.org/ddiss/samba.git bso_8564_pam_winbind_segv

David Disseldorp (1):
      pam_winbind: fix segfault in pam_sm_authenticate()

 nsswitch/pam_winbind.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-
Comment 2 David Disseldorp 2011-11-02 16:55:03 UTC 
Created attachment 7054 [details]
proposed fix rb1

The previously proposed fix did not free info3 data attached to the pam handle.

The following changes since commit 2107ba5be87d6a9f7691cfcbe5411fa7174120b7:

  ldb: fix compiler warning (2011-11-02 16:51:24 +0100)

are available in the git repository at:
  git://git.samba.org/ddiss/samba.git bso_8564_pam_winbind_segv_rb1

David Disseldorp (1):
      pam_winbind: fix segfault in pam_sm_authenticate()

 nsswitch/pam_winbind.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)
Comment 3 David Disseldorp 2012-01-03 12:37:41 UTC 
Hi Günther, were you able to check this patch? Andreas has given it the green light, just waiting on your (n)ack.
Comment 4 Andreas Schneider 2012-10-26 18:58:36 UTC 
Karolin, could you please add this patch to the next release. Thanks.
Comment 5 Andreas Schneider 2012-10-26 19:00:38 UTC 
I've signed the patch and pushed it to autobuild. Please pick for 3.6 and 4.0.
Comment 6 Andreas Schneider 2012-10-26 19:39:55 UTC 
Dave: How can you reproduce this?
Comment 7 David Disseldorp 2012-10-26 20:28:38 UTC 
(In reply to comment #6)
> Dave: How can you reproduce this?

Pretty sure I hit this by adding a module argument "debug" key without a corresponding "=value" suffix to the pam_winbind config entry.
Comment 8 Karolin Seeger 2012-10-29 10:54:32 UTC 
Pushed to v3-6-test and autobuild-v4-0-test.
Will be included in 3.6.10 and 4.0.0rc4.
Closing out bug report.

Thanks!