From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.linuxfoundation.org ([140.211.169.12]:50635 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752415AbcGXXEz (ORCPT ); Sun, 24 Jul 2016 19:04:55 -0400 Subject: Patch "apparmor: fix oops, validate buffer size in apparmor_setprocattr()" has been added to the 4.6-stable tree To: vegard.nossum@oracle.com, casey@schaufler-ca.com, eparis@parisplace.org, gregkh@linuxfoundation.org, james.l.morris@oracle.com, john.johansen@canonical.com, paul@paul-moore.com, sds@tycho.nsa.gov, tyhicks@canonical.com, viro@zeniv.linux.org.uk Cc: , From: Date: Sun, 24 Jul 2016 16:05:10 -0700 Message-ID: <1469401510197208@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=ANSI_X3.4-1968 Content-Transfer-Encoding: 8bit Sender: stable-owner@vger.kernel.org List-ID: This is a note to let you know that I've just added the patch titled apparmor: fix oops, validate buffer size in apparmor_setprocattr() to the 4.6-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary The filename of the patch is: apparmor-fix-oops-validate-buffer-size-in-apparmor_setprocattr.patch and it can be found in the queue-4.6 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let know about it. >>From 30a46a4647fd1df9cf52e43bf467f0d9265096ca Mon Sep 17 00:00:00 2001 From: Vegard Nossum Date: Thu, 7 Jul 2016 13:41:11 -0700 Subject: apparmor: fix oops, validate buffer size in apparmor_setprocattr() From: Vegard Nossum commit 30a46a4647fd1df9cf52e43bf467f0d9265096ca upstream. When proc_pid_attr_write() was changed to use memdup_user apparmor's (interface violating) assumption that the setprocattr buffer was always a single page was violated. The size test is not strictly speaking needed as proc_pid_attr_write() will reject anything larger, but for the sake of robustness we can keep it in. SMACK and SELinux look safe to me, but somebody else should probably have a look just in case. Based on original patch from Vegard Nossum modified for the case that apparmor provides null termination. Fixes: bb646cdb12e75d82258c2f2e7746d5952d3e321a Reported-by: Vegard Nossum Cc: Al Viro Cc: John Johansen Cc: Paul Moore Cc: Stephen Smalley Cc: Eric Paris Cc: Casey Schaufler Signed-off-by: John Johansen Reviewed-by: Tyler Hicks Signed-off-by: James Morris Signed-off-by: Greg Kroah-Hartman --- security/apparmor/lsm.c | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -523,34 +523,34 @@ static int apparmor_setprocattr(struct t { struct common_audit_data sa; struct apparmor_audit_data aad = {0,}; - char *command, *args = value; + char *command, *largs = NULL, *args = value; size_t arg_size; int error; if (size == 0) return -EINVAL; - /* args points to a PAGE_SIZE buffer, AppArmor requires that - * the buffer must be null terminated or have size <= PAGE_SIZE -1 - * so that AppArmor can null terminate them - */ - if (args[size - 1] != '\0') { - if (size == PAGE_SIZE) - return -EINVAL; - args[size] = '\0'; - } - /* task can only write its own attributes */ if (current != task) return -EACCES; - args = value; + /* AppArmor requires that the buffer must be null terminated atm */ + if (args[size - 1] != '\0') { + /* null terminate */ + largs = args = kmalloc(size + 1, GFP_KERNEL); + if (!args) + return -ENOMEM; + memcpy(args, value, size); + args[size] = '\0'; + } + + error = -EINVAL; args = strim(args); command = strsep(&args, " "); if (!args) - return -EINVAL; + goto out; args = skip_spaces(args); if (!*args) - return -EINVAL; + goto out; arg_size = size - (args - (char *) value); if (strcmp(name, "current") == 0) { @@ -576,10 +576,12 @@ static int apparmor_setprocattr(struct t goto fail; } else /* only support the "current" and "exec" process attributes */ - return -EINVAL; + goto fail; if (!error) error = size; +out: + kfree(largs); return error; fail: @@ -588,9 +590,9 @@ fail: aad.profile = aa_current_profile(); aad.op = OP_SETPROCATTR; aad.info = name; - aad.error = -EINVAL; + aad.error = error = -EINVAL; aa_audit_msg(AUDIT_APPARMOR_DENIED, &sa, NULL); - return -EINVAL; + goto out; } static int apparmor_task_setrlimit(struct task_struct *task, Patches currently in stable-queue which might be from vegard.nossum@oracle.com are queue-4.6/apparmor-fix-oops-validate-buffer-size-in-apparmor_setprocattr.patch