linux-kernel - Re: [PATCH] Handle i_size > s

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: <20071222200310.GQ13821@ca-server1.us.oracle.com>
Date:	Sat, 22 Dec 2007 12:03:10 -0800
From:	Mark Fasheh <mark.fasheh@...cle.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Jan Kara <jack@...e.cz>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Handle i_size > s_maxbytes correctly

On Sat, Dec 22, 2007 at 12:12:06AM -0800, Andrew Morton wrote:
> This patch takes the total text size of the affected nine files from 74167
> bytes up to 75066 on i386.  This is core, core kernel.  Ouch.

Yeah, as you note below - this should be un-inlined.

> It's also pretty fragile.  We now have i_size_read()s and
> i_size_read_trunc()s sprinkled all over the place with no obvious rules to
> determine when we should use one versus the other.

Hmm, I hadn't thought about that one, but what you say makes sense. If I was
a 3rd party looking at this, I'd be pretty confused as to which i_size
function to use, etc.

> uninlining i_size_read_trunc() is obviously the first thing to look at but
> the cost is still appreciable and boy the problem which is being fixed here
> is rare and obscure.
> 
> Can we look at alternatives please?  What about just failing the open
> attempt?

The problem is that Ocfs2 can have this happen while a file is open (if a 64
bit node extends it past what the 32 bit node can see while they both have
open file descriptors). Even in Ocfs2 by the way, this is a bit rare - most
of the time folks run nodes of the same architecture. We still allow mixed
arch clusters though.

We could stop the VFS changes at some open checks and leave the rest
internally in Ocfs2 [and GFS2 I'm betting]. Generally though, it seems like
the type of thing that the VFS could help more with.

Jan, what if we just define a helper function which checks that a particular
file access is within the bounds allowed by the file system:

int check_file_access_offsets(struct inode *inode, loff_t pos, size_t count);

Something like that would be potentially better documenting than another
i_size accessor. Also, we could try to just leave it in the higher level
functions (write, read, splice, fault, mkwrite). I'm still not convinced
that we'd need to do anything in readpage/writepage if we just catch the
problem higher up.

Thanks,
	--Mark

--
Mark Fasheh
Principal Software Developer, Oracle
mark.fasheh@...cle.com
--
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/