Skip to content

Commit c72ea20

Browse files
minipli-ossgregkh
authored andcommitted
iio: buffer: Fix file related error handling in IIO_BUFFER_GET_FD_IOCTL
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: f73f7f4 ("iio: buffer: add ioctl() to support opening extra buffers for IIO device") Cc: [email protected] Cc: Jonathan Cameron <[email protected]> Cc: Alexandru Ardelean <[email protected]> Cc: Lars-Peter Clausen <[email protected]> Cc: Nuno Sa <[email protected]> Reported-by: Dan Carpenter <[email protected]> Signed-off-by: Mathias Krause <[email protected]> Reviewed-by: Jonathan Cameron <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent bca828c commit c72ea20

File tree

1 file changed

+11
-3
lines changed

1 file changed

+11
-3
lines changed

drivers/iio/industrialio-buffer.c

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1569,9 +1569,17 @@ static long iio_device_buffer_getfd(struct iio_dev *indio_dev, unsigned long arg
15691569
}
15701570

15711571
if (copy_to_user(ival, &fd, sizeof(fd))) {
1572-
put_unused_fd(fd);
1573-
ret = -EFAULT;
1574-
goto error_free_ib;
1572+
/*
1573+
* "Leak" the fd, as there's not much we can do about this
1574+
* anyway. 'fd' might have been closed already, as
1575+
* anon_inode_getfd() called fd_install() on it, which made
1576+
* it reachable by userland.
1577+
*
1578+
* Instead of allowing a malicious user to play tricks with
1579+
* us, rely on the process exit path to do any necessary
1580+
* cleanup, as in releasing the file, if still needed.
1581+
*/
1582+
return -EFAULT;
15751583
}
15761584

15771585
return 0;

0 commit comments

Comments
 (0)