[Tarantool-patches] [PATCH luajit 3/3] Fix another potential file descriptor leak in luaL_loadfile*().

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

From: Mike Pall <mike>

Reported by Peter Cawley.

(cherry picked from commit ab39082fddfca0de268a106a3b6d736eef032328)

`loadfile()` doesn't close the fd in case when `fopen()` results
successfully, but then the file can't be read (since it is a directory,
for example) or parser failure occurs for some reason.

This patch fixes that behaviour by moving the error formatting after the
cleanup of the descriptor.

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#11278
---
 src/lj_load.c                                 | 16 ++++-----
 .../lj-1249-loadfile-fd-leak.test.lua         | 34 ++++++++++++++++++-
 2 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/src/lj_load.c b/src/lj_load.c
index a6d0b464..fdbc54cb 100644
--- a/src/lj_load.c
+++ b/src/lj_load.c
@@ -87,6 +87,7 @@ LUALIB_API int luaL_loadfilex(lua_State *L, const char *filename,
   FileReaderCtx ctx;
   int status;
   const char *chunkname;
+  int err = 0;
   if (filename) {
     chunkname = lua_pushfstring(L, "@%s", filename);
     ctx.fp = fopen(filename, "rb");
@@ -100,17 +101,16 @@ LUALIB_API int luaL_loadfilex(lua_State *L, const char *filename,
     chunkname = "=stdin";
   }
   status = lua_loadx(L, reader_file, &ctx, chunkname, mode);
-  if (ferror(ctx.fp)) {
-    L->top -= filename ? 2 : 1;
-    lua_pushfstring(L, "cannot read %s: %s", chunkname+1, strerror(errno));
-    if (filename)
-      fclose(ctx.fp);
-    return LUA_ERRFILE;
-  }
+  if (ferror(ctx.fp)) err = errno;
   if (filename) {
+    fclose(ctx.fp);
     L->top--;
     copyTV(L, L->top-1, L->top);
-    fclose(ctx.fp);
+  }
+  if (err) {
+    L->top--;
+    lua_pushfstring(L, "cannot read %s: %s", chunkname+1, strerror(err));
+    return LUA_ERRFILE;
   }
   return status;
 }
diff --git a/test/tarantool-tests/lj-1249-loadfile-fd-leak.test.lua b/test/tarantool-tests/lj-1249-loadfile-fd-leak.test.lua
index c1a45724..fe406fd1 100644
--- a/test/tarantool-tests/lj-1249-loadfile-fd-leak.test.lua
+++ b/test/tarantool-tests/lj-1249-loadfile-fd-leak.test.lua
@@ -6,7 +6,7 @@ local tap = require('tap')
 -- See also, https://github.com/LuaJIT/LuaJIT/issues/1249.
 local test = tap.test('lj-1249-loadfile-fd-leak')
 
-test:plan(2)
+test:plan(4)
 
 local allocinject = require('allocinject')
 
@@ -24,4 +24,36 @@ test:ok(not res, 'correct status, OOM on filename creation')
 test:like(errmsg, 'not enough memory',
           'correct error message, OOM on filename creation')
 
+-- Now try to read the directory. It can be opened but not read as
+-- a file on Linux-like systems.
+
+-- On macOS and BSD-like systems, the content of the directory may
+-- be read and contain some internal data, which we are not
+-- interested in.
+test:skipcond({
+  ['Disabled on non-Linux systems'] = jit.os ~= 'Linux',
+})
+
+local DIRNAME = '/dev'
+local GCSTR_OBJSIZE = 24
+
+-- Now the OOM error should be obtained on the creation of the
+-- error message that the given file (or directory) is not
+-- readable. But the string for the file name should be allocated
+-- without OOM, so set the corresponding limit for the string
+-- object.
+-- Don't forget to count the leading '@' and trailing '\0'.
+allocinject.enable_null_limited_alloc(#DIRNAME + GCSTR_OBJSIZE + 1 + 1)
+
+-- Error since can't read the directory (but actually the OOM on
+-- parsing preparation is raised before, due to allocation limit).
+res, errmsg = pcall(loadfile, DIRNAME)
+
+allocinject.disable()
+
+-- Sanity checks.
+test:ok(not res, 'correct status, OOM on error message creation')
+test:like(errmsg, 'not enough memory',
+          'correct error message, OOM on error message creation')
+
 test:done(true)
-- 
2.49.0