Bug 251853 - spnego_malloc causes nsupdate to crash badly
spnego_malloc causes nsupdate to crash badly
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: bind (Show other bugs)
rawhide
All Linux
low Severity low
: ---
: ---
Assigned To: Adam Tkac
Fedora Extras Quality Assurance
: Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-08-12 12:23 EDT by Simo Sorce
Modified: 2013-04-30 19:36 EDT (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-08-23 07:16:44 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
Remove completely wrong alloc/free functions (2.09 KB, patch)
2007-08-12 12:23 EDT, Simo Sorce
no flags Details | Diff
Patch which conforms BIND coding standards (1.46 KB, patch)
2007-08-22 09:05 EDT, Adam Tkac
no flags Details | Diff
Patch which uses real malloc to create gss_buffer structures (1.22 KB, patch)
2007-08-23 06:44 EDT, Stepan Kasal
no flags Details | Diff

  None (edit)
Description Simo Sorce 2007-08-12 12:23:42 EDT
Description of problem:
nsupdate segfaults when using gssapi to do GSS-TSIG based updates

Version-Release number of selected component (if applicable):
9.5.0a6

How reproducible:
always

Steps to Reproduce:
1. Configure a server to accept GSS-TSIG updates
2. Do an update via nsupdate after a kinit
  
Actual results:
Get a SIGABORT from glibc 

Expected results:
no abort

Additional info:
I traced the problem to someone being on crack when coding lib/dns/spnego.c

In that file malloc/free/realloc are redefined to use
spnego_malloc/spnego_free/spnego_realloc

These functions will not returned the actual address allocated but an address
_inside_ the allocated space. The problem is that later on things like
gss_release_buffer() are called to free this memory.

as libgssapi does not use spnego_free (how could it) to release the memory what
happens is that actually the code tries to free the wrong address. (just
valgrind nsupdate and you will find that memory is allocated with spnego_malloc
and later on a free() with an address 16 bytes inside an allocated chunck is
performed by gss_release_buffer).

I "cured" the problem simply removing spnego_[malloc/free/realloc] and using the
standard glibc fiunctions.

Patch attached.
Comment 1 Simo Sorce 2007-08-12 12:23:42 EDT
Created attachment 161140 [details]
Remove completely wrong alloc/free functions
Comment 2 Adam Tkac 2007-08-13 12:16:10 EDT
Temporary fixed with your patch, final decision is on upstream
Comment 3 Adam Tkac 2007-08-14 06:09:58 EDT
After discussion with upstream patch will be nicely.

"We should be calling gss_release_buffer() on buffers we got from the GSS
library, spnego_free() on buffers allocated with spnego_malloc(), and should
never mix the two."

I'm going to create proposed patch
Comment 4 Adam Tkac 2007-08-22 09:05:12 EDT
Created attachment 162048 [details]
Patch which conforms BIND coding standards
Comment 5 Stepan Kasal 2007-08-23 06:44:55 EDT
Created attachment 168579 [details]
Patch which uses real malloc to create gss_buffer structures

This patch fixes the problem on the other end: when the gss_buffer_desc.value
is allocated, the real malloc is used.
I have an uneasy feeeling when a standard gss structure flows around containing
non-freeable pointers inside.
If I get it right, in most cases the gss_buffer is filled by a gss library
call, which, naturally, uses the real malloc.  Later on, gss_release_buffer may
be called.  When we have to alloc the space ourselves, we should use the
standard way.
Please note that I do not understand all code in spnego.c, so I'm not sure if
this is the only case where such a broken gss_buffer is created.  From that
perspective, Simo's original patch is still the safest one, so I un-pbsoleted
it.
Comment 6 Adam Tkac 2007-08-23 07:02:19 EDT
(In reply to comment #5)
Yes, better patch than mine. But in the end I solve this issue by
'--disable-isc-spnego' configure option. This options fire away nasty hacks in
spnego.c (this spnego isn't compiled with this option) and use code from gssapi
library
Comment 7 Adam Tkac 2007-08-23 07:16:44 EDT
fixed in 9.5.0-11.a6 (start using gssapi spnego, not ISC's)

Note You need to log in before you can comment on or make changes to this bug.