From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Fri, 14 Jan 2022 19:34:03 +0300 From: "Vladimir D. Seleznev" To: ALT Linux Team development discussions Message-ID: References: <20220114151823.239944-1-asheplyakov@basealt.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220114151823.239944-1-asheplyakov@basealt.ru> Subject: Re: [devel] [PATCH] AltHa: avoid kernel stack overflow X-BeenThere: devel@lists.altlinux.org X-Mailman-Version: 2.1.12 Precedence: list Reply-To: ALT Linux Team development discussions List-Id: ALT Linux Team development discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 14 Jan 2022 16:34:03 -0000 Archived-At: List-Archive: List-Post: On Fri, Jan 14, 2022 at 07:18:23PM +0400, Alexey Sheplyakov wrote: > From: Alexey Sheplyakov > > * 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 exploit this an attacker must have superuser privileges. But of course the bug should be fixed. Acked. > 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 > --- > 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; > } > -- > 2.32.0 -- WBR, Vladimir D. Seleznev