[Devel] [PATCH RHEL7 COMMIT] fs: avoid holding extra reference on original path

[Kirill Tkhai]

The commit is pushed to "branch-rh7-3.10.0-327.36.1.vz7.18.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-327.36.1.vz7.18.3
------>
commit 247122498983e301268504a0b6af65e469b27de6
Author: Maxim Patlasov <mpatlasov at virtuozzo.com>
Date:   Mon Oct 3 09:27:27 2016 +0000

    fs: avoid holding extra reference on original path
    
    When vfs_open() opens ordinary file (not overlayfs one), do_dentry_open() will
    call path_get(&f->f_path) anyway, so it doesn't make sense to acquire extra
    references by another path_get().
    
    The above is enough to satisfy LTP syscalls/fcntl/fcntl24 when called on top
    of ordinary fs (not overlayfs), but for overlayfs it's also necessary to ensure
    that fs/locks.c::generic_add_lease() passes original (overlayfs) dentry to
    check_conflicting_open(). This goes in accordance with mainstream:
    
    > commit 6343a2120862f7023006c8091ad95c1f16a32077
    > Author: Miklos Szeredi <mszeredi at redhat.com>
    > Date:   Fri Jul 1 14:56:07 2016 +0200
    >
    >     locks: use file_inode()
    >
    >     (Another one for the f_path debacle.)
    >
    >     ltp fcntl33 testcase caused an Oops in selinux_file_send_sigiotask.
    >
    >     The reason is that generic_add_lease() used filp->f_path.dentry->inode
    >     while all the others use file_inode().  This makes a difference for files
    >     opened on overlayfs since the former will point to the overlay inode the
    >     latter to the underlying inode.
    >
    >     So generic_add_lease() added the lease to the overlay inode and
    >     generic_delete_lease() removed it from the underlying inode.  When the file
    >     was released the lease remained on the overlay inode's lock list, resulting
    >     in use after free.
    >
    >     Reported-by: Eryu Guan <eguan at redhat.com>
    >     Fixes: 4bacc9c9234c ("overlayfs: Make f_path always point to the overlay and f_inode to the underlay")
    >     Cc: <stable at vger.kernel.org>
    >     Signed-off-by: Miklos Szeredi <mszeredi at redhat.com>
    >     Reviewed-by: Jeff Layton <jlayton at redhat.com>
    >     Signed-off-by: J. Bruce Fields <bfields at redhat.com>
    >
    > diff --git a/fs/locks.c b/fs/locks.c
    > index 7c5f91b..ee1b15f 100644
    > --- a/fs/locks.c
    > +++ b/fs/locks.c
    > @@ -1628,7 +1628,7 @@ generic_add_lease(struct file *filp, long arg, struct file_lock **flp, void **pr
    >  {
    >         struct file_lock *fl, *my_fl = NULL, *lease;
    >         struct dentry *dentry = filp->f_path.dentry;
    > -       struct inode *inode = dentry->d_inode;
    > +       struct inode *inode = file_inode(filp);
    >         struct file_lock_context *ctx;
    >         bool is_deleg = (*flp)->fl_flags & FL_DELEG;
    >         int error;
    
    https://jira.sw.ru/browse/PSBM-52817
    
    Signed-off-by: Maxim Patlasov <mpatlasov at virtuozzo.com>
    Reviewed-by: Kirill Tkhai <ktkhai at virtuozzo.com>
---
 fs/locks.c | 5 +++--
 fs/open.c  | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index a5ab0c0..f6c89d7 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1560,8 +1560,9 @@ static int
 generic_add_lease(struct file *filp, long arg, struct file_lock **flp, void **priv)
 {
 	struct file_lock *fl, **before, **my_before = NULL, *lease;
-	struct dentry *dentry = filp->f_path.dentry;
-	struct inode *inode = dentry->d_inode;
+	struct dentry *dentry = filp->f_original_path.mnt ?
+		filp->f_original_path.dentry: filp->f_path.dentry;
+	struct inode *inode = filp->f_path.dentry->d_inode;
 	bool is_deleg = (*flp)->fl_flags & FL_DELEG;
 	int error;
 
diff --git a/fs/open.c b/fs/open.c
index 25dbc85..84eb289 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -896,7 +896,7 @@ int vfs_open(const struct path *path, struct file *filp,
 	int do_cleanup = 0;
 	int ret;
 
-	if (!filp->f_original_path.mnt) {
+	if (!filp->f_original_path.mnt && dentry_open) {
 		filp->f_original_path = *path;
 		path_get(&filp->f_original_path);
 		do_cleanup = 1;