On Tue, 2011-12-13 at 11:01 -0500, Stephen Gallagher wrote:
On Tue, 2011-12-13 at 10:59 -0500, Simo Sorce wrote:
> On Tue, 2011-12-13 at 10:36 -0500, Stephen Gallagher wrote:
> > On Tue, 2011-12-13 at 10:32 -0500, Simo Sorce wrote:
> > > > Looking at this closely, I think what probably happened is that the
> > > > child died while waiting for the ping_check to time out, so that when
it
> > > > finally returned, the memory for the mt_svc object had been freed.
> > > > Because we then use talloc_get_type() to retrieve that memory, it
> > > > detects that it's invalid and simply returns NULL (which we then
> > > > dereference and crash on).
> > >
> > > maybe we should use talloc_get_type_abort() here ...
> >
> > Nack. I really, really don't want it to be possible for the monitor to
> > crash.
>
> In this case it makes no difference. If you reference freed memory or
> 'random' memory you will end up crashing as well.
>
> The point is that we should never get to a point where talloc_get_type()
> fails and return NULL, unless it is legal to pass NULL in as input.
>
> I am all for not having the monitor crash of course.
>
> Simo.
>
Well, in this particular case, our answer if it returns NULL will be to
just exit and do nothing, which will keep the monitor running.
Only because of chance, you may as well get a segfault as the pointer
may be pointing to reclaimed memory that is not accessible by the
process anymore.
But as I said in my earlier comments, I think I identified the
reason
why it was coming up with NULL anyway.
Yes, and I totally agree with your analysis, I just do not like to rely
on something working because it recognized it was accessing freed memory
and returned NULL as it is not safe.
Simo.
--
Simo Sorce * Red Hat, Inc * New York