From patchwork Fri Mar 31 12:57:35 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Johansen X-Patchwork-Id: 745631 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) by ozlabs.org (Postfix) with ESMTP id 3vvhQM1bLqz9s03; Fri, 31 Mar 2017 23:58:51 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1ctw8d-0000vj-VX; Fri, 31 Mar 2017 12:58:47 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.76) (envelope-from ) id 1ctw7z-0000hu-28 for kernel-team@lists.ubuntu.com; Fri, 31 Mar 2017 12:58:07 +0000 Received: from static-50-53-32-2.bvtn.or.frontiernet.net ([50.53.32.2] helo=canonical.com) by youngberry.canonical.com with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.76) (envelope-from ) id 1ctw7y-0006AV-G5 for kernel-team@lists.ubuntu.com; Fri, 31 Mar 2017 12:58:06 +0000 From: John Johansen To: kernel-team@lists.ubuntu.com Subject: [PATCH 02/11] UBUNTU: SAUCE: apparmor: fix replacement race in reading rawdata Date: Fri, 31 Mar 2017 05:57:35 -0700 Message-Id: <20170331125744.16986-3-john.johansen@canonical.com> X-Mailer: git-send-email 2.9.3 In-Reply-To: <20170331125744.16986-1-john.johansen@canonical.com> References: <20170331125744.16986-1-john.johansen@canonical.com> X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.14 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: kernel-team-bounces@lists.ubuntu.com The reading of rawdata is subject to a replacement race when the rawdata is read in chunks smaller than the data size. For each read the profile proxy is rechecked for the newest profile; Which means if a profile is replaced between reads later chunks will contain data from the new version of the profile while the earlier reads will contain data from the previous version. This can result in data that is inconsistent and corrupt. Instead of rechecking for the current profile at each read. Get the current profile at the time of the open and use the rawdata of the profile for the lifetime that the file handle is open. BugLink: http://bugs.launchpad.net/bugs/1638996 Signed-off-by: John Johansen Acked-by: Stefan Bader Acked-by: Tim Gardner Acked-by: Brad Figg Signed-off-by: Thadeu Lima de Souza Cascardo --- security/apparmor/apparmorfs.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 28c95b3..b8ba1bb 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -766,7 +766,7 @@ static const struct file_operations aa_fs_ns_name = { static int rawdata_release(struct inode *inode, struct file *file) { /* TODO: switch to loaddata when profile switched to symlink */ - aa_put_proxy(file->private_data); + aa_put_loaddata(file->private_data); return 0; } @@ -832,22 +832,24 @@ static const struct file_operations aa_fs_seq_raw_hash_fops = { static ssize_t rawdata_read(struct file *file, char __user *buf, size_t size, loff_t *ppos) { - struct aa_proxy *proxy = file->private_data; - struct aa_label *label = aa_get_label_rcu(&proxy->label); - struct aa_profile *profile = labels_profile(label); + struct aa_loaddata *rawdata = file->private_data; - ssize_t ret = simple_read_from_buffer(buf, size, ppos, profile->rawdata->data, profile->rawdata->size); - aa_put_label(label); - - return ret; + return simple_read_from_buffer(buf, size, ppos, rawdata->data, + rawdata->size); } static int rawdata_open(struct inode *inode, struct file *file) { + struct aa_proxy *proxy = inode->i_private; + struct aa_label *label; + struct aa_profile *profile; + if (!policy_view_capable(NULL)) return -EACCES; - - file->private_data = aa_get_proxy(inode->i_private); + label = aa_get_label_rcu(&proxy->label); + profile = labels_profile(label); + file->private_data = aa_get_loaddata(profile->rawdata); + aa_put_label(label); return 0; }