Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934743AbcJUJ0s (ORCPT ); Fri, 21 Oct 2016 05:26:48 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:47188 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934518AbcJUJWT (ORCPT ); Fri, 21 Oct 2016 05:22:19 -0400 From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Aihua Zhang , Miklos Szeredi Subject: [PATCH 4.8 53/57] vfs: move permission checking into notify_change() for utimes(NULL) Date: Fri, 21 Oct 2016 11:18:16 +0200 Message-Id: <20161021091437.846917163@linuxfoundation.org> X-Mailer: git-send-email 2.10.0 In-Reply-To: <20161021091435.435647262@linuxfoundation.org> References: <20161021091435.435647262@linuxfoundation.org> User-Agent: quilt/0.64 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2761 Lines: 98 4.8-stable review patch. If anyone has any objections, please let me know. ------------------ From: Miklos Szeredi commit f2b20f6ee842313a0d681dbbf7f87b70291a6a3b upstream. This fixes a bug where the permission was not properly checked in overlayfs. The testcase is ltp/utimensat01. It is also cleaner and safer to do the permission checking in the vfs helper instead of the caller. This patch introduces an additional ia_valid flag ATTR_TOUCH (since touch(1) is the most obvious user of utimes(NULL)) that is passed into notify_change whenever the conditions for this special permission checking mode are met. Reported-by: Aihua Zhang Signed-off-by: Miklos Szeredi Tested-by: Aihua Zhang Signed-off-by: Greg Kroah-Hartman --- fs/attr.c | 15 +++++++++++++++ fs/utimes.c | 17 +---------------- include/linux/fs.h | 1 + 3 files changed, 17 insertions(+), 16 deletions(-) --- a/fs/attr.c +++ b/fs/attr.c @@ -202,6 +202,21 @@ int notify_change(struct dentry * dentry return -EPERM; } + /* + * If utimes(2) and friends are called with times == NULL (or both + * times are UTIME_NOW), then we need to check for write permission + */ + if (ia_valid & ATTR_TOUCH) { + if (IS_IMMUTABLE(inode)) + return -EPERM; + + if (!inode_owner_or_capable(inode)) { + error = inode_permission(inode, MAY_WRITE); + if (error) + return error; + } + } + if ((ia_valid & ATTR_MODE)) { umode_t amode = attr->ia_mode; /* Flag setting protected by i_mutex */ --- a/fs/utimes.c +++ b/fs/utimes.c @@ -87,21 +87,7 @@ static int utimes_common(struct path *pa */ newattrs.ia_valid |= ATTR_TIMES_SET; } else { - /* - * If times is NULL (or both times are UTIME_NOW), - * then we need to check permissions, because - * inode_change_ok() won't do it. - */ - error = -EPERM; - if (IS_IMMUTABLE(inode)) - goto mnt_drop_write_and_out; - - error = -EACCES; - if (!inode_owner_or_capable(inode)) { - error = inode_permission(inode, MAY_WRITE); - if (error) - goto mnt_drop_write_and_out; - } + newattrs.ia_valid |= ATTR_TOUCH; } retry_deleg: inode_lock(inode); @@ -113,7 +99,6 @@ retry_deleg: goto retry_deleg; } -mnt_drop_write_and_out: mnt_drop_write(path->mnt); out: return error; --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -224,6 +224,7 @@ typedef int (dio_iodone_t)(struct kiocb #define ATTR_KILL_PRIV (1 << 14) #define ATTR_OPEN (1 << 15) /* Truncating from open(O_TRUNC) */ #define ATTR_TIMES_SET (1 << 16) +#define ATTR_TOUCH (1 << 17) /* * Whiteout is represented by a char device. The following constants define the