SV: SIGBUS Panic in smbd

[Michael Stockman]

Hello,

> >
> > This sounds similar to the bug I found in the HEAD branch wrt
password
> > caching.  There is code in passdb/pass_check.c that can change the
> > value of the pw_passwd field in the pass_check() function
depending on
> > which weird password shadowing functions the host OS supports.
> >
> > I thought it was only a problem in the new password caching code
but
> > perhaps it is present in SAMBA_2_0 as well.
>
> Hmmm. But even if is does change the pw_passwd field,
> that field is supposed to be a pointer into a static
> area, so it shouldn't matter so long as no-one calls
> free() on it (which in the 2.0.x branch, only the
> conditional code in HAVE_GETPWANAM does - I've now
> removed that).

It could matter, as at least on my linux where the structure returned
by getpwnam() is the same on each call -> there is a risk of memory
corruption or strange behaviour, if HAVE_GETPWANAM is defined. Assume
that the passwd_adjunct->pwa_passwd points to a shorter memory area
than passwd->pw_passwd, getpwnam could write out of bounds. If that
doesn't happen (and I don't _think_ it will) the contents of the
passwd->pw_passwd will change on each call to getpwanam since it
points to the same memory as the passwd_adjunct->pwa_passwd
(unexpected?). In this case that doesn't matter but it could cause
confusion if these functions are used at other places, and regardless
it is dirty (microsoftish) programming and definiatly a bug. We must
differ between assigning pointers to each other and changing the
contents of strings (it isn't the same thing).

> I still cannot see why this thing is crashing Ken's
> system, unless the underlying passwd functions are
> keeping a private malloced area that they are doing
> a free() on on second and subsequent calls....

That could be, and since we don't know and it could be different
between different systems, we shouldn't free the pointers or reassign
them to either static or allocated memory (they are owned by the
library, not us). I do still propose strcpy as the dirty hack and a
struct of our own as the fix.

I will start looking into the unix<->nt struct this afternoon or
tomorrow.

> Ken - do you have purify on that box ?
>
> I'll look into making sure the pw_passwd field is
> restored after use (although this gets a bit
> tricky).

This would be an almost good approach, but quite unmanagable I think
(for the same reason I think that getpwnam and getpwanam deals in
static memory, it is virtually impossible to know when to free/reset
the pointers unless you make a function for that which must be called
when we're done with the struct).

> Jeremy.

Best regards
  Michael Stockman
  pgmtekn-micke at algonet.se