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]
Date:	Thu, 01 Jan 2009 14:08:00 +0300
From:	Alexey Zaytsev <alexey.zaytsev@...il.com>
To:	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>
Cc:	James.Bottomley@...senpartnership.com,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, linux-scsi@...r.kernel.org,
	jens.axboe@...cle.com
Subject: Re: [PATCH] Fix sg_io_hdr.info corruption.

[resending in case you missed the one I sent with broken headers]

On Wed, Dec 31, 2008 at 12:08, FUJITA Tomonori <fujita.tomonori@....ntt.co.jp> wrote:
> On Tue, 30 Dec 2008 15:46:03 -0800
> Andrew Morton <akpm@...ux-foundation.org> wrote:
[...]
>> the code has been like this for years and years.  Why hasn't anyone
>> noticed?
>
> The members from 'status' in struct sg_io_hdr to the last are used to
> transfer information from kernel to user space. The values that user
> space sets are just ignored.
>

Then probably there is no need to copy those fields, right?
There should be no data leak from the kernel, as sgio is
allocated on the userspace stack, and the appropriate ioctl
handler should set/zero all those fields anyway, as it expects
them to come directly from the user (did not check).
So, in the worst case the user gets his own garbage insted of
the values he left in the fields that the kernel was supposed
to set.

If so, please drop my previous patch and take this one.

From: Alexey Zaytsev <alexey.zaytsev@...il.com>
Subject: [PATCH] Don't perform unneeded copy.

FUJITA Tomonori <fujita.tomonori@....ntt.co.jp> says:

The members from 'status' in struct sg_io_hdr to the last are used to
transfer information from kernel to user space. The values that user
space sets are just ignored.

Signed-off-by: Alexey Zaytsev <alexey.zaytsev@...il.com>
---
 fs/compat_ioctl.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index 5235c67..23b1f5a 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -782,12 +782,6 @@ static int sg_ioctl_trans(unsigned int fd, unsigned int cmd, unsigned long arg)
 	if (put_user(compat_ptr(data), &sgio->usr_ptr))
 		return -EFAULT;
 
-	if (copy_in_user(&sgio->status, &sgio32->status,
-			 (4 * sizeof(unsigned char)) +
-			 (2 * sizeof(unsigned (short))) +
-			 (3 * sizeof(int))))
-		return -EFAULT;
-
 	err = sys_ioctl(fd, cmd, (unsigned long) sgio);
 
 	if (err >= 0) {

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists