[Devel] [PATCH vz7] ve/module: calculate sysfs max path size at compile time to avoid static variable

[Alexander Atanasov]

in commit 3f1147ffecc3 ("ve/module: export sysfs dentries in containers")

    2) Buffer for path is made static in assumption, that modules load and unload
    my happen from time to time and there is not need to allocate this buffer each
    time we need to expose or hide module sysfs dentries (allocating on stack
    gives "the frame size of 4104 bytes is larger than 2048 bytes" warning)

This change made the path variable static to avoid large stack usage.
But its contents are searched (strchr) and modified (*sep++ = 0;) in
kernfs_perms_set. While there is lock in sysfs_set_def_perms it does not
protect the contents of the path variable just multiple calls to kernfs_perms_set.
And sysfs setup/teardown is made without holding module_mutex.
So concurrent loads/unloads can trash the variable and trigger
write (0) to a random address.

To solve this calculate max possible size of the path and allocate
it back on the stack. While when dealing with user supplied paths
using PATH_MAX is a must. But for a kernel only called function making
predictable lenght calculation is ok when no user supplied parts are used.
In case the calculated path becomes too small display a warning
to help increasing it accordingly easy.

https://jira.sw.ru/browse/PSBM-143836
Signed-off-by: Alexander Atanasov <alexander.atanasov at virtuozzo.com>
---
 kernel/module.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index 8d54b575ec1f..8d20fe32f452 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1797,11 +1797,23 @@ out:
 
 static ssize_t module_sysfs_perm_set_ve(struct module *mod, char *subdir, int mask)
 {
-	static char path[PATH_MAX];
+	/*
+	 * To avoid using 4K stack space we can calculate max possible path
+	 * If new users came this must be updated but it will be quickly catched
+	 * since they would not see their new subdirs.
+	 * currently the only subdir is holders
+	 */
+#define PERM_PATH_LEN (sizeof("module") + 2 + sizeof("holders") + MODULE_NAME_LEN + 1)
+#define WARNFMT "sysfs path too long for %s (%s) increase PERM_PATH_LEN\n"
+
+	char path[PERM_PATH_LEN+1];
 
 	if (snprintf(path, sizeof(path) - 1, "module/%s/%s",
-		     mod->name, (subdir) ? subdir : "") >= sizeof(path) - 1)
+		     mod->name, (subdir) ? subdir : "") >= sizeof(path) - 1) {
+		WARN(1, WARNFMT, mod->name, subdir ?  subdir : "");
 		return -E2BIG;
+	}
+#undef WARNFMT
 
 	return sysfs_set_def_perms(path, mask);
 }
-- 
2.31.1