SIGBUS Panic in smbd

[Charles Hedrick]

I've seen a couple of misunderstandings in the posts here.  First, the
problem is not limited to the
getpwanam code.  It also happens in the shadow password code.

At least with the shadow code, the problem is caused by trying to free
the pw_passwd field,
with the password hashing code off.

Some of the discussion seems to be based on misconceptions of how
getpwnam and getspnam work.
These functions return pointers to a struct in static memory.  Some of
the entries in that struct are
strings.  Those strings are also allocated in static memory.  Thus it is
invalid to free any of this.
There is no problem simply copying the sp_pwdp field from the shadow
entry into the pw_passwd
field of the normal password struct.  sp_pwdp is also a pointer to
static memory.  One message
suggested that somehow doing that copy might reduce a buffer size.  That
shouldn't be the case.
Every time you call getpwnam or getspnam data is put into permanently
allocated static space inside
the function call.  If a previous call has changed the pw_passwd field
to point to something small
that doesn't matter.  The next call to getpwnam will recreate the passwd
struct, restoring the pointer
to getpwnam's static buffer.  (I guess I should say that I'm speaking
from knowledge of the way
several versions of libc work.  It's always possible that someone does
it differently.  But I strongly
doubt it.)

As long as the returns from getpwnam are used "soon", i.e. before any
further calls to getpwnam,
this kind of code is fine.  If that's not the case, then you should copy
both the struct and all the
strings it points to somewhere.  If that "somewhere" is malloc'ed then
you have to arrange to free it.

Another approach is to use the thread-safe versions, e.g. getpwnam_r.
(This is on Solaris.  I trust
Linux also has these.)  For these, you pass pointers to the buffers you
want it to use.  That gives
you more control over lifetimes of memory.  When I'm writing code that I
expect to be called
in arbitrary contexts (e.g. functions called by PAM), I always use these
calls.  I want to make sure I
don't overwrite the static buffers if the caller happens to be using
one.  In retrospect I think that's
the approach that should have been used originally, and the traditional
one should be phased out.
For systems that don't have getpwnam_r, an emulator could be written
that calls getpwnam
and then copies the data into the buffers supplied by the user to
getpwnam_r.

I am particularly concerned about having the hashing code under a
conditional.  The password
hash code looks just fine to me.  However it uses memory quite
differently than when hashing
is not in effect.  When hashing is off, you're passing around pointers
to static memory inside libc.
When hashing is on, you're passing around memory that the hash code has
malloced and will
worry about.  That means that the kinds of memory bugs you're going to
see in the two cases
are very different.  Debugging memory allocation is bad enough without
two ways to build the
program that will produce completely different sets of bugs.  I don't
care which approach you use
(in Solaris the hashing is redundant: the OS does it already), but I
strongly recommend that you
use only one approach, and do not even provide a conditional to choose
between them.