[bugzilla-daemon@xxxxxxxxxxxxx]

http://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2453





--- Comment #21 from Balint Reczey <balint.reczey@xxxxxxxxxxxx>  2008-04-30 06:30:17 GMT ---
I think that we cannot resolv the problem withLua's garbage collector, because
the tvb is freed by wireshark. 
Our problem 
http://www.lua.org/pil/28.5.html



(In reply to comment #18)
> Created an attachment (id=1758)
 --> (http://bugs.wireshark.org/bugzilla/attachment.cgi?id=1758) [details]
> remove the "outstanding_stuff" handling completely
> 
> I would remove the "outstanding_stuff" completely. I tend to think that
> implementing the __gc method can clear all the data. In pushXxx(L, xxx)
> functions (see epan/wslua/wslua.h) a new userdata is created with the function
> lua_newuserdata(). Then the pointer is actually not checked if it is NULL or
> not, but this is an other story. So we have a pointer and use this pointer to
> fill the content of the buffer.
> 
> In the __gc function this area could be filled with zeros, if it is needed at
> all.
> 
> What is not clear for me that what should be the pair of the
> luaL_getmetatable(), lua_setmetatable(), and the "push_code" parameter in __gc.
> 

(In reply to comment #20)
> (In reply to comment #17)
> > I don't really understand why should be there *anything* which has to be
> > cleared. Lua will manage the memory and call the __gc if something has to be
> > cleared. What can be cleared are the actual userdata and not the pointer itself
> > (see epan/wslua/wslua.h)
> > 
> > If I understand the situation correctly maybe the __gc function should be
> > implemented instead of reinventing the wheel with this "outstanding_stuff"
> > thing.
> > 
> 
> The outstanding stuff is there to avoid leaving "pointers to junk" in the lua
> VM.
> 
> if the lua user copies a tvb and keeps it after the current packet that pointer
> will become invalid as soon as the ws core cleans it up before the next packet.
> By NULLifing it we avoid it to crash if used later.
> 
> if for example we have __gc freeing the tvb itself there's probably another 30
> places Wireshark could crash when trying to use the freed tvb.
> 
> So the whole "outstanding_X thing" is there simply because the lifespan of the
> Lua variable containing the object and the object itself is not related.
> 
> I agree that the best solution would be to forcefully purge variables from the
> Lua VM (e.g. turn them into nil) when the underlying object is being freed or
> it has become invalid; But unfortunately I'm not aware of any way to achieve
> this.
> 

Unfortunatelly that method does not seem to work.

wslua_tvb.c:

void clear_outstanding_tvbs(void) {
    while (outstanding_stuff->len) {
        void** p = (void**)g_ptr_array_remove_index_fast(outstanding_stuff,0);
        *p = NULL;
        ^^^^^^^^^^
        We could use p there which is Tvb*, but not *p which may 
        point to a freed memory location, like in the valgrind example.
        It can be easily checked using DDD.

    }
}

void* push_Tvb(lua_State* L, Tvb tvb) {
    void** p = (void**)pushTvb(L,tvb);
    ^^^^^^^^^^^^^^^^^^^^^^^^ 

       pushTvb returns Tvb* and we cast it to void** which does not 
       cause any trouble since we return p (and we do not use int anywhere)


    g_ptr_array_add(outstanding_stuff,p);
        return p;
}

I think we should disable the outstanding stuff for now because it causes
random crashes and warn users not to keep references to buffers/trees/pinfos
from previous packets.

As a final solution, i'll try to fix the problem of junk pointers by using
light userdata.


-- 
Configure bugmail: http://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.