[Devel] [PATCH RHEL9 COMMIT] ve/module: Calculate sysfs max path size at compile time to avoid static variable

[Konstantin Khorenko]

The commit is pushed to "branch-rh9-5.14.0-162.6.1.vz9.18.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh9-5.14.0-162.6.1.vz9.18.2
------>
commit 17ced0fc816411d891b81d43b88496b50388da14
Author: Alexander Atanasov <alexander.atanasov at virtuozzo.com>
Date:   Fri Dec 16 18:07:09 2022 +0200

    ve/module: Calculate sysfs max path size at compile time to avoid static variable
    
    in commit 3f1147ffecc3 ("ve/module: export sysfs dentries in containers")
    (cherry-picked to vz9 commit 70ec52c1e0099e7c775cf9900619b7fca5fc6c1e)
    
        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 which can result
    in corrupted sysfs entries. Since snprintf is used there is no way
    strchr to go beyond the end of the static buffer.
    
    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.
    Do a compile time check for subdir name argument lenght and assert
    if it is longer than expected.
    
    https://jira.sw.ru/browse/PSBM-143836
    Signed-off-by: Alexander Atanasov <alexander.atanasov at virtuozzo.com>
    Feature: sysfs: per-CT entries visibility and permissions configuration
---
 kernel/module.c | 35 ++++++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index 27e4326915c2..ce5388c9849e 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1849,26 +1849,39 @@ static int mod_sysfs_init(struct module *mod)
 
 #ifdef CONFIG_VE
 
-static ssize_t module_sysfs_perm_set_ve(struct module *mod, char *subdir, int mask)
+static ssize_t module_sysfs_perm_set_ve_real(struct module *mod,
+						const 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 VE_LONGEST_SUBDIR "holders"
+#define PERM_PATH_LEN (sizeof("module") + 2 + sizeof(VE_LONGEST_SUBDIR) + \
+				MODULE_NAME_LEN + 1)
+	char path[PERM_PATH_LEN+1];
 
 	if (snprintf(path, sizeof(path) - 1, "module/%s/%s",
 		     mod->name, (subdir) ? subdir : "") >= sizeof(path) - 1)
 		return -E2BIG;
-
 	return sysfs_set_def_perms(path, mask);
+#undef PERM_PATH_LEN
 }
 
-static ssize_t module_sysfs_hide_dir_ve(struct module *mod, char *subdir)
-{
-	return module_sysfs_perm_set_ve(mod, subdir, -1);
-}
+#define module_sysfs_perm_set_ve(mod, subdir, mask)				\
+	({									\
+		BUILD_BUG_ON_MSG(sizeof(subdir) > sizeof(VE_LONGEST_SUBDIR),	\
+			"Please, update VE_LONGEST_SUBDIR to:" # subdir);	\
+		module_sysfs_perm_set_ve_real(mod, subdir, mask);		\
+	})
 
-static ssize_t module_sysfs_expose_dir_ve(struct module *mod, char *subdir)
-{
-	return module_sysfs_perm_set_ve(mod, subdir, MAY_READ | MAY_EXEC);
-}
+
+#define module_sysfs_hide_dir_ve(mod, subdir) 					\
+		module_sysfs_perm_set_ve(mod, subdir, -1)
+#define module_sysfs_expose_dir_ve(mod, subdir) 				\
+		module_sysfs_perm_set_ve(mod, subdir, MAY_READ | MAY_EXEC)
 
 static int module_sysfs_ve_init(struct module *mod)
 {