| Message ID | 20191106190239.20860-1-agruenba@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fs: Fix overflow in block_page_mkwrite | expand |
On Wed, Nov 06, 2019 at 08:02:39PM +0100, Andreas Gruenbacher wrote: > On architectures where ssize_t is wider than pgoff_t, the expression > ((page->index + 1) << PAGE_SHIFT) can overflow. Rewrite to use the page > offset, which we already compute here anyway. Looks good modulo the s/ssize_t/loff_t/ mentioned in the other patch: Reviewed-by: Christoph Hellwig <hch@lst.de>
On Wed 06-11-19 20:02:39, Andreas Gruenbacher wrote: > On architectures where ssize_t is wider than pgoff_t, the expression > ((page->index + 1) << PAGE_SHIFT) can overflow. Rewrite to use the page > offset, which we already compute here anyway. > > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> This patch seems to have fallen through the cracks? Al? Honza > --- > fs/buffer.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/fs/buffer.c b/fs/buffer.c > index 86a38b979323..da3f33b70249 100644 > --- a/fs/buffer.c > +++ b/fs/buffer.c > @@ -2459,21 +2459,21 @@ int block_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf, > struct page *page = vmf->page; > struct inode *inode = file_inode(vma->vm_file); > unsigned long end; > - loff_t size; > + loff_t offset, size; > int ret; > > lock_page(page); > size = i_size_read(inode); > - if ((page->mapping != inode->i_mapping) || > - (page_offset(page) > size)) { > + offset = page_offset(page); > + if (page->mapping != inode->i_mapping || offset > size) { > /* We overload EFAULT to mean page got truncated */ > ret = -EFAULT; > goto out_unlock; > } > > /* page is wholly or partially inside EOF */ > - if (((page->index + 1) << PAGE_SHIFT) > size) > - end = size & ~PAGE_MASK; > + if (offset > size - PAGE_SIZE) > + end = offset_in_page(size); > else > end = PAGE_SIZE; > > -- > 2.20.1 >
Hi Jan, On Wed, Dec 18, 2019 at 1:43 PM Jan Kara <jack@suse.cz> wrote: > On Wed 06-11-19 20:02:39, Andreas Gruenbacher wrote: > > On architectures where ssize_t is wider than pgoff_t, the expression > > ((page->index + 1) << PAGE_SHIFT) can overflow. Rewrite to use the page > > offset, which we already compute here anyway. > > > > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> > > This patch seems to have fallen through the cracks? Al? There's been a v2 of this patch [*] and I'm about to send out a v3, so please ignore this first version. Thanks, Andreas [*] https://lore.kernel.org/linux-fsdevel/20191129142045.7215-1-agruenba@redhat.com/
diff --git a/fs/buffer.c b/fs/buffer.c index 86a38b979323..da3f33b70249 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -2459,21 +2459,21 @@ int block_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf, struct page *page = vmf->page; struct inode *inode = file_inode(vma->vm_file); unsigned long end; - loff_t size; + loff_t offset, size; int ret; lock_page(page); size = i_size_read(inode); - if ((page->mapping != inode->i_mapping) || - (page_offset(page) > size)) { + offset = page_offset(page); + if (page->mapping != inode->i_mapping || offset > size) { /* We overload EFAULT to mean page got truncated */ ret = -EFAULT; goto out_unlock; } /* page is wholly or partially inside EOF */ - if (((page->index + 1) << PAGE_SHIFT) > size) - end = size & ~PAGE_MASK; + if (offset > size - PAGE_SIZE) + end = offset_in_page(size); else end = PAGE_SIZE;
On architectures where ssize_t is wider than pgoff_t, the expression ((page->index + 1) << PAGE_SHIFT) can overflow. Rewrite to use the page offset, which we already compute here anyway. Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> --- fs/buffer.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)