linux-kernel - [PATCH 5.16 184/203] iio: buffer: Fix file related error handling in IIO_BUFFER_GET_FD

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20220214092516.498129131@linuxfoundation.org>
Date:   Mon, 14 Feb 2022 10:27:08 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     linux-kernel@...r.kernel.org
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        stable@...r.kernel.org, stable@...nel.org,
        Jonathan Cameron <jic23@...nel.org>,
        Alexandru Ardelean <ardeleanalex@...il.com>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Nuno Sa <Nuno.Sa@...log.com>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Mathias Krause <minipli@...ecurity.net>,
        Jonathan Cameron <Jonathan.Cameron@...wei.com>
Subject: [PATCH 5.16 184/203] iio: buffer: Fix file related error handling in IIO_BUFFER_GET_FD_IOCTL

From: Mathias Krause <minipli@...ecurity.net>

commit c72ea20503610a4a7ba26c769357d31602769c01 upstream.

If we fail to copy the just created file descriptor to userland, we
try to clean up by putting back 'fd' and freeing 'ib'. The code uses
put_unused_fd() for the former which is wrong, as the file descriptor
was already published by fd_install() which gets called internally by
anon_inode_getfd().

This makes the error handling code leaving a half cleaned up file
descriptor table around and a partially destructed 'file' object,
allowing userland to play use-after-free tricks on us, by abusing
the still usable fd and making the code operate on a dangling
'file->private_data' pointer.

Instead of leaving the kernel in a partially corrupted state, don't
attempt to explicitly clean up and leave this to the process exit
path that'll release any still valid fds, including the one created
by the previous call to anon_inode_getfd(). Simply return -EFAULT to
indicate the error.

Fixes: f73f7f4da581 ("iio: buffer: add ioctl() to support opening extra buffers for IIO device")
Cc: stable@...nel.org
Cc: Jonathan Cameron <jic23@...nel.org>
Cc: Alexandru Ardelean <ardeleanalex@...il.com>
Cc: Lars-Peter Clausen <lars@...afoo.de>
Cc: Nuno Sa <Nuno.Sa@...log.com>
Reported-by: Dan Carpenter <dan.carpenter@...cle.com>
Signed-off-by: Mathias Krause <minipli@...ecurity.net>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
---
 drivers/iio/industrialio-buffer.c |   14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -1569,9 +1569,17 @@ static long iio_device_buffer_getfd(stru
 	}
 
 	if (copy_to_user(ival, &fd, sizeof(fd))) {
-		put_unused_fd(fd);
-		ret = -EFAULT;
-		goto error_free_ib;
+		/*
+		 * "Leak" the fd, as there's not much we can do about this
+		 * anyway. 'fd' might have been closed already, as
+		 * anon_inode_getfd() called fd_install() on it, which made
+		 * it reachable by userland.
+		 *
+		 * Instead of allowing a malicious user to play tricks with
+		 * us, rely on the process exit path to do any necessary
+		 * cleanup, as in releasing the file, if still needed.
+		 */
+		return -EFAULT;
 	}
 
 	return 0;