linux-kernel - Re: [PATCH] mm, fs: avoid page allocation beyond i

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: <20130821160817.940D3E0090@blue.fi.intel.com>
Date:	Wed, 21 Aug 2013 19:08:17 +0300 (EEST)
From:	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
To:	Steven Whitehouse <swhiteho@...hat.com>
Cc:	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Jan Kara <jack@...e.cz>, Al Viro <viro@...iv.linux.org.uk>,
	NeilBrown <neilb@...e.de>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm, fs: avoid page allocation beyond i_size on read

Steven Whitehouse wrote:
> Hi,
> 
> On Wed, 2013-08-21 at 18:37 +0300, Kirill A. Shutemov wrote:
> > I've noticed that we allocated unneeded page for cache on read beyond
> > i_size. Simple test case (I checked it on ramfs):
> > 
> > $ touch testfile
> > $ cat testfile
> > 
> > It triggers 'no_cached_page' code path in do_generic_file_read().
> > 
> > Looks like it's regression since commit a32ea1e. Let's fix it.
> > 
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>
> > Acked-by: NeilBrown <neilb@...e.de>
> > ---
> >  mm/filemap.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/mm/filemap.c b/mm/filemap.c
> > index 1905f0e..b1a4d35 100644
> > --- a/mm/filemap.c
> > +++ b/mm/filemap.c
> > @@ -1163,6 +1163,10 @@ static void do_generic_file_read(struct file *filp, loff_t *ppos,
> >  		loff_t isize;
> >  		unsigned long nr, ret;
> >  
> > +		isize = i_size_read(inode);
> > +		if (!isize || index > (isize - 1) >> PAGE_CACHE_SHIFT)
> > +			goto out;
> > +
> >  		cond_resched();
> >  find_page:
> >  		page = find_get_page(mapping, index);
> 
> Please don't do that... there is no reason to think that i_size will be
> correct at that moment. Why not just get readpage(s) to return the
> correct return code in that case?

I work on THP for page cache. Allocation and clearing a huge page for
nothing is pretty expensive.

I don't think the change is harmful. The worst case scenario is race with
write or truncate, but it's valid to return EOF in this case.

What scenario do you have in mind?

-- 
 Kirill A. Shutemov
--
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/