linux-kernel - Re: [PATCH] binfmt_elf_fdpic: fix clear

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:	Fri, 28 May 2010 13:38:41 -0400
From:	Mike Frysinger <vapier.adi@...il.com>
To:	Takuya Yoshikawa <yoshikawa.takuya@....ntt.co.jp>
Cc:	akpm@...ux-foundation.org, viro@...iv.linux.org.uk,
	d.hatayama@...fujitsu.com, dhowells@...hat.com,
	lethal@...ux-sh.org, takuya.yoshikawa@...il.com,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] binfmt_elf_fdpic: fix clear_user() error handling

On Fri, May 28, 2010 at 03:56, Takuya Yoshikawa wrote:
> Hi, I found some places in bin_elf_fdpic at which clear_user() is
> incorrectly handled, by chance, when I was trying to check how to
> use clear_user().
>
> IIUC, the following commit was not correct.
>
>  commit ab4ad55512e95b68ca3e25516068e18874f89252
>  bin_elf_fdpic: check the return value of clear_user
>
> Although I don't have an appropriate test box for this, I wrote a
> simple patch to fix this. So if this is worth fixing, please pick
> this up.

the intention was that these functions return 0 only on success, and
non-zero otherwise.  along those lines, the patch does what was
intended.  unfortunately, the logic calling these funcs only checks
for negative values.

> clear_user() returns the number of bytes, unsigned long, that could not
> be copied. So we should return -EFAULT rather than directly return the results.
>
> Without this patch, positive values may be passed to elf_fdpic_map_file() and
> the following error handlings do not function as expected.

on nommu systems, this is generally not an issue because clear_user()
is basically a memset().  but it's good to handle every case.

Acked-by: Mike Frysinger <vapier@...too.org>
-mike
--
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/