Bug 8564 - pam_winbind segfault in pam_sm_authenticate()
pam_winbind segfault in pam_sm_authenticate()
Status: RESOLVED FIXED
Product: Samba 3.6
Classification: Unclassified
Component: Winbind
unspecified
All All
: P5 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
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

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!