lua: fix memory leak in set method
authorPetr Štetiar <ynezz@true.cz>
Mon, 4 Nov 2019 22:08:26 +0000 (23:08 +0100)
committerPetr Štetiar <ynezz@true.cz>
Thu, 14 Nov 2019 16:11:34 +0000 (17:11 +0100)
scan-build from clang version 9 has reported following issue:

 uci.c:624:12: warning: Potential leak of memory pointed to by 's'
       return luaL_error(L, "Cannot set an uci option to an empty table value");
              ^~~~~~~~~~

valgrind confirmed it on the supplied test case:

 ==31013== 8 bytes in 1 blocks are definitely lost in loss record 1 of 1
 ==31013==    by 0x56C49B9: strdup (strdup.c:42)

Signed-off-by: Petr Štetiar <ynezz@true.cz>
lua/uci.c
tests/cram/lua/test_cases/set_with_empty_table_doesnt_leak.lua [new file with mode: 0644]
tests/cram/test_ucilua_testcases.t

index 42fe4b785901cf6f755470f9756a5998a700ca5c..ad082996a3c37f60b038f54cd5fab45d5ac22bd6 100644 (file)
--- a/lua/uci.c
+++ b/lua/uci.c
@@ -620,8 +620,10 @@ uci_lua_set(lua_State *L)
        case 4:
                /* Format: uci.set("p", "s", "o", "v") */
                if (lua_istable(L, nargs)) {
-                       if (lua_rawlen(L, nargs) < 1)
+                       if (lua_rawlen(L, nargs) < 1) {
+                               free(s);
                                return luaL_error(L, "Cannot set an uci option to an empty table value");
+                       }
                        lua_rawgeti(L, nargs, 1);
                        ptr.value = luaL_checkstring(L, -1);
                        lua_pop(L, 1);
diff --git a/tests/cram/lua/test_cases/set_with_empty_table_doesnt_leak.lua b/tests/cram/lua/test_cases/set_with_empty_table_doesnt_leak.lua
new file mode 100644 (file)
index 0000000..83e91d3
--- /dev/null
@@ -0,0 +1,2 @@
+local c = uci.cursor(os.getenv("CONFIG_DIR"))
+print(pcall(c.set, c, "network", "lan", "dns", {}))
index 279dfce9422a7c64500a13b9082cde8fc4d6de06..6a0216b3ded8d83dc7a0d0adcc5c9b1488b79b4e 100644 (file)
@@ -8,3 +8,10 @@ check that changes method doesnt leak memory:
   $ cp -R "$TESTDIR/config" .
   $ export CONFIG_DIR=$(pwd)/config
   $ ucilua $TESTDIR/lua/test_cases/changes_doesnt_leak.lua
+
+check that set method with empty table value doesn't leak memory:
+
+  $ cp -R "$TESTDIR/config" .
+  $ export CONFIG_DIR=$(pwd)/config
+  $ ucilua $TESTDIR/lua/test_cases/set_with_empty_table_doesnt_leak.lua
+  false\tCannot set an uci option to an empty table value (esc)