[Devel] [PATCH RHEL8 COMMIT] ve/tty: Fix NULL pointer dereference at vtty_open_master error path.

Konstantin Khorenko khorenko at virtuozzo.com
Mon Jul 26 20:15:28 MSK 2021


The commit is pushed to "branch-rh8-4.18.0-305.3.1.vz8.7.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh8-4.18.0-305.3.1.el8
------>
commit e5f37561b6f4f45f77e8ff6f08fdca5505db83af
Author: Valeriy Vdovin <valeriy.vdovin at virtuozzo.com>
Date:   Mon Jul 26 20:15:28 2021 +0300

    ve/tty: Fix NULL pointer dereference at vtty_open_master error path.
    
    Ported function vtty_open_master crashes the kernel with a NULL pointer
    dereference at it's error path, while deconstructing a file object.
    This started since ms commit 72c2d53192004845cbc19cd8a30b3212a9288140
    'file->f_op is never NULL...'
    
    Actually vtty_open_master assigned NULL to file->f_op. Let's fix that
    by removing this NULL assignment and ensuring that the remaining f_op
    operations could safely exist in this context. For that lets:
    1. make sure that file->f_op->release (tty_release) is safe to be called with
    file->private_data being NULL, because at error path it's already destroyed.
    2. make sure that f_op->fasync never gets called by disabling the FASYNC
    flash manually before fput.
    Currently, __fput only calls two file->f_op's so, this should suffice.
    
    Fixes: eb2220696477f117d742f89e91e1766f4a1a29fe
    
    https://jira.sw.ru/browse/PSBM-132299
    
    Signed-off-by: Valeriy Vdovin <valeriy.vdovin at virtuozzo.com>
---
 drivers/tty/pty.c    |  9 ++++++++-
 drivers/tty/tty_io.c | 11 ++++++++++-
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 8d2e8fa4ac6d..2f99183cd561 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -1129,7 +1129,14 @@ int vtty_open_master(envid_t veid, int idx)
 	mutex_unlock(&tty_mutex);
 	tty_free_file(file);
 err_fput:
-	file->f_op = NULL;
+	/*
+	 * __fput will try to call file->f_op->fasync and file->f_op->release
+	 * We don't want that.
+	 * fasync ( will not get called without FASYNC flag.
+	 * release (tty_release in our case) has private_data == NULL checked
+	 * for early return.
+	 */
+	file->f_flags &= ~FASYNC;
 	fput(file);
 err_put_unused_fd:
 	put_unused_fd(fd);
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index f0e8d10462a0..9b4c2f1af10a 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1642,13 +1642,22 @@ EXPORT_SYMBOL_GPL(tty_release_struct);
 
 int tty_release(struct inode *inode, struct file *filp)
 {
-	struct tty_struct *tty = file_tty(filp);
+	struct tty_struct *tty;
 	struct tty_struct *o_tty = NULL;
 	int	do_sleep, final;
 	int	idx;
 	long	timeout = 0;
 	int	once = 1;
 
+	/*
+	 * filp can be released at error path with private_data already
+	 * reverted to NULL, see vtty_open_master.
+	 */
+	if (!filp->private_data)
+		return 0;
+
+	tty = file_tty(filp);
+
 	if (tty_paranoia_check(tty, inode, __func__))
 		return 0;
 


More information about the Devel mailing list