lkml.org 
[lkml]   [2008]   [Jan]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] Handle i_size > s_maxbytes correctly
On Sat 22-12-07 00:12:06, Andrew Morton wrote:
Sorry for a late reply but I was on vacation.

> On Thu, 20 Dec 2007 18:51:04 +0100 Jan Kara <jack@suse.cz> wrote:
>
> > Although we don't allow writes over s_maxbytes, it can happen that a file's
> > size is larger than s_maxbytes. For example we can write the file from a
> > computer with a different architecture (which has larger s_maxbytes), boot
> > a kernel with a different set of config options (CONFIG_LBD...), or if two
> > nodes in a [Ocfs2, and likely Gfs2] cluster have mounted the same file
> > system and have different s_maxbytes. Thus we have to make sure we don't
> > crash / corrupt data when seeing such file (page offset of the last page
> > needn't fit into pgoff_t). Firstly, we make read() and mmap() return error
> > when user tries to access the file above s_maxbytes, secondly we introduce
> > a function i_size_read_trunc() which returns min(i_size, s_maxbytes) and
> > use it when determining maximal page offset we are interested in.
> >
> > ...
> >
> > --- a/fs/buffer.c
> > +++ b/fs/buffer.c
> > @@ -1623,7 +1623,7 @@ static int __block_write_full_page(struct inode *inode, struct page *page,
> >
> > BUG_ON(!PageLocked(page));
> >
> > - last_block = (i_size_read(inode) - 1) >> inode->i_blkbits;
> > + last_block = (i_size_read_trunc(inode) - 1) >> inode->i_blkbits;
> >
> > ...
> >
> > +/* Truncate i_size at s_maxbytes so that pagecache doesn't have problems */
> > +static inline loff_t i_size_read_trunc(const struct inode *inode)
> > +{
> > + loff_t i_size = i_size_read(inode);
> > +
> > + if (unlikely(inode->i_sb->s_maxbytes < i_size))
> > + return inode->i_sb->s_maxbytes;
> > + return i_size;
> > +}
> > +
>
> 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.
>
> 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.
Looking at the patch from the distance of two weeks I agree this is a
flaw...

> 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?
As Mark wrote, just failing the open does not solve the problem for
clustered filesystems. I'll try to come up with something that would be
a less fragile solution (and won't increase the text size that much).

Thanks for your comments

Honza
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR


\
 
 \ /
  Last update: 2008-01-07 17:43    [W:0.062 / U:1.852 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site