linux-kernel - Re: [PATCH] hfsplus: Add record offset check

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, 14 Jul 2011 06:06:54 +0100 (BST)
From:	Hin-Tak Leung <hintak_leung@...oo.co.uk>
To:	Pekka Enberg <penberg@...nel.org>
Cc:	linux-fsdevel@...r.kernel.org, Naohiro Aota <naota@...sp.net>,
	linux-kernel@...r.kernel.org, Christoph Hellwig <hch@...era.com>
Subject: Re: [PATCH] hfsplus: Add record offset check



--- On Wed, 13/7/11, Pekka Enberg <penberg@...nel.org> wrote:

> From: Pekka Enberg <penberg@...nel.org>
> Subject: Re: [PATCH] hfsplus: Add record offset check
> To: "Hin-Tak Leung" <hintak_leung@...oo.co.uk>
> Cc: linux-fsdevel@...r.kernel.org, "Naohiro Aota" <naota@...sp.net>, linux-kernel@...r.kernel.org, "Christoph Hellwig" <hch@...era.com>
> Date: Wednesday, 13 July, 2011, 7:06
> On Wed, Jul 13, 2011 at 6:58 AM,
> Hin-Tak Leung <hintak_leung@...oo.co.uk>
> wrote:
> >> Corrupted disk may return record offset which is
> larger
> >> than node size
> >> and cause general protection fault like below:
> >
> > <snipped>
> >
> >> This patch add guard for this situation.
> >>
> >> Signed-off-by: Naohiro Aota <naota@...sp.net>
> >
> > Nacked. This isn't acceptable. Explained above.
> 
> 'recoff' is read from disk which can be easily fuzzed to
> have an
> offset that's larger than node_size, no? The kernel
> shouldn't oops in
> that cases so what's the problem with the patch? (The
> changelog is
> terribly vague, though, and needs to be fixed).

You have put your finger one problem of the patch - 'The changelog is terribly vague, though, and needs to be fixed'.

The other issue is that, while the kernel should not oops no matter what, the patch (or the changelog itself) made no attempts at explaining why this specific approach. e.g. What happens after 'return 0'? For example.





--
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/