String overflow for unistd.read(). about luaposix HOT 6 CLOSED

You don't say what hardware or OS you are on. How many bits are in your size_t? What type did you use for number in your LuaJIT build?

Do you have a minimal program to reproduce the error? Does it only occur on LuaJIT?

Are you interested in working on a patch?

from luaposix.

I will investigate more and answer your questions if needed, but isn't size_t an unsigned type, or at least cannot be guaranteed to be a signed type ? If so, how could the code in question be correct, checking for a negative value ?

As a reference, here read returns ssize_t.

from luaposix.

Can confirm that by fixing the "overflow" I could see the actual error underneath: a bad file descriptor error from read() (which is its own mystery, but orthogonal to this issue).

To reproduce:

local unistd = require "posix.unistd"
local stdio = require "posix.stdio"

local f = io.tmpfile()
local fd = stdio.fileno(f)
f:close()
local str = assert(unistd.read(fd, 4096))

On Arch Linux 64 bits:

luajit: string length overflow
stack traceback:
	[C]: in function 'read'
	bug.lua:7: in main chunk
	[C]: at 0x55e4b41feed0

lua5.3: memory allocation error: block too big
stack traceback:
	[C]: in function 'posix.unistd.read'
	bug.lua:7: in main chunk
	[C]: in ?

Test fix:

index 20e5803..d8b8be7 100644
--- a/ext/posix/unistd.c
+++ b/ext/posix/unistd.c
@@ -930,7 +930,8 @@ static int
 Pread(lua_State *L)
 {
        int fd = checkint(L, 1);
-       size_t count = (size_t)checkinteger(L, 2), ret;
+       size_t count = (size_t)checkinteger(L, 2);
+       ssize_t ret;
        void *ud, *buf;
        lua_Alloc lalloc;

@@ -943,6 +944,7 @@ Pread(lua_State *L)
                return pusherror(L, "lalloc");

        ret = read(fd, buf, count);
+       if(ret < 0) printf("read => %jd\n", ret);
        if (ret >= 0)
                lua_pushlstring(L, buf, ret);
        lalloc(ud, buf, count, 0);

Result:

read => -1
luajit: bug.lua:7: Bad file descriptor
stack traceback:
	[C]: in function 'assert'
	bug.lua:7: in main chunk
	[C]: at 0x563a6735bed0

from luaposix.

Awesome catch, thanks for reporting and especially for also providing a fix. I expect to be able to integrate your fix the weekend after next (I'm on my phone without a computer until then). If you'd like it to be upstreamed sooner, a PR with your fix, along with a test case that passes GitHub CI to exercise it so we don't accidentally reintroduce the problem in future would be great! I can hit merge from my phone in that case 👍

from luaposix.

The bad descriptor is a symptom of this issue: #217

Different implementations of Lua wrap file objects differently, and until we figure out how to portably wrap a libc descriptor up in a Lua file object, you can't use luaposix files in Lua io calls, and vice versa 😖

from luaposix.

I expect to be able to integrate your fix the weekend after next [...]

Thanks. Even if it's just changing a variable type, I don't want to mess with GitHub and I'm not in a hurry.

The bad descriptor is a symptom of this issue: #217

I encountered that issue, but I'm only using stdio.fileno, which should be fine.

(In my case, the error was on reading a pipe.)

from luaposix.