Re: [devel] [PATCH] AltHa: avoid kernel stack overflow

["Anton V. Boyarshinov" <boyarsh@altlinux.org>]

В Fri, 14 Jan 2022 19:18:23 +0400
Alexey Sheplyakov <asheplyakov@basealt.ru> пишет:

> From: Alexey Sheplyakov <asheplyakov@altlinux.org>
 
Thank you! I've planned to do it, but too lazy.

 * The kernel stack on most architectures is 4096 bytes.
> * PATH_MAX is also 4096 (bytes).
> 
> Therefore
> 
> * Allocating PATH_MAX bytes on the stack definitely overflows
>   the (kernel) stack (perhaps this can be exploited by creating
>   a long enough path). To avoid the problem allocate `path_buffer`
>   on the heap (and free it on all code paths)
> 
> * CONFIG_FRAME_WARN=6144 (which is 1.5x the kernel stack size)
>   is wrong and dangerous (it papers over actual overflows and
>   creates a false sense of safety), use the default value instead
>   (2048 bytes, which is a half of the kernel stack)
> 
> Signed-off-by: Alexey Sheplyakov <asheplyakov@altlinux.org>
> ---
>  config                     |  2 +-
>  security/altha/altha_lsm.c | 21 ++++++++++++++++++---
>  2 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/config b/config
> index da093b3f8e64..5ef328887321 100644
> --- a/config
> +++ b/config
> @@ -10347,7 +10347,7 @@ CONFIG_DEBUG_INFO_BTF=y
>  CONFIG_PAHOLE_HAS_SPLIT_BTF=y
>  CONFIG_DEBUG_INFO_BTF_MODULES=y
>  # CONFIG_GDB_SCRIPTS is not set
> -CONFIG_FRAME_WARN=6144
> +CONFIG_FRAME_WARN=2048
>  # CONFIG_STRIP_ASM_SYMS is not set
>  # CONFIG_READABLE_ASM is not set
>  # CONFIG_HEADERS_INSTALL is not set
> diff --git a/security/altha/altha_lsm.c b/security/altha/altha_lsm.c
> index ccde83ebb26c..c670ad7ed458 100644
> --- a/security/altha/altha_lsm.c
> +++ b/security/altha/altha_lsm.c
> @@ -243,8 +243,13 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
>  	struct altha_list_struct *node;
>  	/* when it's not a shebang issued script interpreter */
>  	if (rstrscript_enabled && bprm->executable == bprm->interpreter) {
> -		char path_buffer[PATH_MAX];
>  		char *path_p;
> +		char *path_buffer;
> +
> +		path_buffer = kmalloc(PATH_MAX, GFP_KERNEL);
> +		if (!path_buffer)
> +			return -ENOMEM;
> +
>  		path_p = d_path(&bprm->file->f_path,path_buffer,PATH_MAX);
>  		down_read(&interpreters_sem);
>  		list_for_each_entry(node, &interpreters_list, list) {
> @@ -255,16 +260,24 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
>  				    ("AltHa/RestrScript: %s is blocked to run directly by %d\n",
>  				     bprm->filename, cur_uid);
>  				up_read(&interpreters_sem);
> +				kfree(path_buffer);
>  				return -EPERM;
>  			}
>  		}
>  		up_read(&interpreters_sem);
> +		kfree(path_buffer);
>  	}
>  	if (unlikely(nosuid_enabled &&
>  		     !uid_eq(bprm->cred->uid, bprm->cred->euid))) {
> -		char path_buffer[PATH_MAX];
>  		char *path_p;
> -		uid_t cur_uid = from_kuid(bprm->cred->user_ns, bprm->cred->uid);
> +		char *path_buffer;
> +		uid_t cur_uid;
> +
> +		path_buffer = kmalloc(PATH_MAX, GFP_KERNEL);
> +		if (!path_buffer)
> +			return -ENOMEM;
> +
> +		cur_uid = from_kuid(bprm->cred->user_ns, bprm->cred->uid);
>  		path_p = d_path(&bprm->file->f_path,path_buffer,PATH_MAX);
>  		down_read(&nosuid_exceptions_sem);
>  		list_for_each_entry(node, &nosuid_exceptions_list, list) {
> @@ -273,6 +286,7 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
>  				    ("AltHa/NoSUID: %s permitted to setuid from %d\n",
>  				     bprm->filename, cur_uid);
>  				up_read(&nosuid_exceptions_sem);
> +				kfree(path_buffer);
>  				return 0;
>  			}
>  		}
> @@ -281,6 +295,7 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
>  		    ("AltHa/NoSUID: %s prevented to setuid from %d\n",
>  		     bprm->filename, cur_uid);
>  		bprm->cred->euid = bprm->cred->uid;
> +		kfree(path_buffer);
>  	}
>  	return 0;
>  }