Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 23 Sep 2014 19:25:47 -1000
From: Scott Valentine <scottvalen@...mail.com>
To: musl@...ts.openwall.com
CC: Justin Cormack <justin@...cialbusservice.com>
Subject: Re: LUA + musl, garbage collection issue?

On Sunday, September 21, 2014 11:16:13 AM Justin Cormack wrote:
> On Sun, Sep 21, 2014 at 5:38 AM, Rich Felker <dalias@...c.org> wrote:
> > On Sat, Sep 20, 2014 at 04:41:14PM -1000, Scott Valentine wrote:
> >> I noticed that in order to free memory, it basically calls realloc
> >> with 0 as the new size. Is this something musl doesn't handle well?
> >>
> >> I'm trying a rebuild with a check for n == 0 in musl's realloc
> >> function to just free the pointer, and I'll report back.
> >>
> >> What is "the right thing to do" to fix this? Should lua not be using
> >> realloc to free memory, or should musl handle the case better, if,
> >> in fact this is the problem?
> >
> > This is a bug in lua; it's depending on a bug in glibc. POSIX attempts
> 
> As pointed out on lua-l but not copied here, Lua is not doing this, it
> does call free() in the 0 case, so something else is the issue...
> 
> (openwrt does use a patched Lua, might be worth testing with upstream).
> 
> Justin

I see now that it does indeed call free. I was looking at frealloc in the lua source.

In any case, this has been a nasty issue to track down. I have surely traced it to the following code block in luci (by process of elimination):

        local fp
        luci.http.setfilehandler(
                function(meta, chunk, eof)
                        if not fp then
                                if meta and meta.name == "image" then
                                        fp = io.open(image_tmp, "w")
                                else
                                        fp = io.popen(restore_tmp, "w")
                                end
                        end
                        if chunk then
                                fp:write(chunk)
                        end
                        if eof then
                                fp:close()
                        end
                end
        )


Here, "chunk" is a 2048 byte string, and the library calls are to nixio:


static int nixio_file_write(lua_State *L) {
        int fd = nixio__checkfd(L, 1);
        size_t len;
        ssize_t sent;
        const char *data = luaL_checklstring(L, 2, &len);

        if (lua_gettop(L) > 2) {
                int offset = luaL_optint(L, 3, 0);
                if (offset) {
                        if (offset < len) {
                                data += offset;
                                len -= offset;
                        } else {
                                len = 0;
                        }
                }

                unsigned int wlen = luaL_optint(L, 4, len);
                if (wlen < len) {
                        len = wlen;
                }
        }

        do {
                sent = write(fd, data, len);
        } while(sent == -1 && errno == EINTR);
        if (sent >= 0) {
                lua_pushinteger(L, sent);
                return 1;
        } else {
                return nixio__perror(L);
        }
}

So that all looks pretty innocuous... 

Right now, my current work around that actually fixes the issue is to #define BUFSIZ 8192 in musl's stdio.h file.

I have tried increasing LUAL_BUFFERSIZE in luaconf.h (it defaults to BUFSIZ) and reducing the size of "chunk" to as small as 512 bytes, but neither approach fixes the problem.
Also, I don't think LUAL_BUFFERSIZE is relevant to the problem. Any comment?

Either way, it is still unclear as to why the problem occurs, and I can only conjecture because I have no good debugging tools for a musl build.

But basically, I am thinking it partly has to do with the stdio write implementation in musl (i.e. commiting write as soon as the buffer is full).

I'm still not sure where to look to find a proper fix though, I mean essentially it is writing the file to RAM anyway (tmpfs), so the buffer commit should be pretty fast.
Ideas?

When I have time, I'll do a build against eglibc with a reduced BUFSIZ = 1024 (musl's default) and see if the problem is reproduced.

-Scott V.

-- 

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.