linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* + fix-possible-page_cache_shift-overflows.patch added to -mm tree
@ 2005-11-23 21:22 akpm
  2005-11-23 22:01 ` ntfs - was: " Anton Altaparmakov
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: akpm @ 2005-11-23 21:22 UTC (permalink / raw)
  To: akpm, aia21, blaisorblade, dhowells, dwmw2, green, hch, jdike,
	linux-fsdevel, miklos, neilb, reiserfs-dev, rmk, trond.myklebust,
	zippel, mm-commits


The patch titled

     fix possible PAGE_CACHE_SHIFT overflows

has been added to the -mm tree.  Its filename is

     fix-possible-page_cache_shift-overflows.patch


From: Andrew Morton <akpm@osdl.org>

We've had two instances recently of overflows when doing

	64_bit_value = (32_bit_value << PAGE_CACHE_SHIFT)

I did a tree-wide grep of `<<.*PAGE_CACHE_SHIFT' and this is the result.

- afs_rxfs_fetch_descriptor.offset is of type off_t, which seems broken.

- jffs2_read_inode_range() takes a uint32_t offset, which also seems wrong.

- hfsplus_releasepage():nidx is u32, which might be wrong.

- reiserfs map_block_for_writepage() takes an unsigned long for the block -
  it should take sector_t.  (It'll fail for huge filesystems with
  blocksize<PAGE_CACHE_SIZE)

- ntfs_read_compressed_block is doing fishy things with cur_page and
  cb_max_page.

- load_and_init_attrdef()'s handling of `index' needs to be checked.  Ditto
  load_and_init_upcase().  Ditto
  ntfs_attr_extend_initialized():initialized_size

- cramfs_read() needs to use sector_t (I think cramsfs is busted on large
  filesystems anyway)

- affs is busted all over the place.

- I generally didn't fix 32-bit overflows in directory operations.

- hfs_releasepage():nidx may be wrong.

- fuse_send_readpages() is limited to 4G

- arm's __flush_dcache_page() is peculiar.  What if the page lies beyond 4G?

- gss_wrap_req_priv() needs checking (snd_buf->page_base)


Cc: Oleg Drokin <green@linuxhacker.ru>
Cc: David Howells <dhowells@redhat.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: <reiserfs-dev@namesys.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Anton Altaparmakov <aia21@cantab.net>
Cc: Jeff Dike <jdike@addtoit.com>
Cc: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
Cc: Roman Zippel <zippel@linux-m68k.org>
Cc: <linux-fsdevel@vger.kernel.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Russell King <rmk@arm.linux.org.uk>
Cc: Trond Myklebust <trond.myklebust@fys.uio.no>
Cc: Neil Brown <neilb@cse.unsw.edu.au>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 fs/afs/dir.c             |    2 +-
 fs/buffer.c              |    6 +++---
 fs/freevxfs/vxfs_immed.c |    4 ++--
 fs/hostfs/hostfs_kern.c  |    2 +-
 fs/jffs/inode-v23.c      |    4 ++--
 fs/mpage.c               |    4 ++--
 fs/romfs/inode.c         |    6 +++---
 fs/smbfs/file.c          |    4 ++--
 fs/sysv/dir.c            |    4 ++--
 9 files changed, 18 insertions(+), 18 deletions(-)

diff -puN fs/afs/dir.c~fix-possible-page_cache_shift-overflows fs/afs/dir.c
--- 25/fs/afs/dir.c~fix-possible-page_cache_shift-overflows	Wed Nov 23 13:22:12 2005
+++ 25-akpm/fs/afs/dir.c	Wed Nov 23 13:22:12 2005
@@ -137,7 +137,7 @@ static inline void afs_dir_check_page(st
 #endif
 
 	/* determine how many magic numbers there should be in this page */
-	latter = dir->i_size - (page->index << PAGE_CACHE_SHIFT);
+	latter = dir->i_size - ((loff_t)page->index << PAGE_CACHE_SHIFT);
 	if (latter >= PAGE_SIZE)
 		qty = PAGE_SIZE;
 	else
diff -puN fs/buffer.c~fix-possible-page_cache_shift-overflows fs/buffer.c
--- 25/fs/buffer.c~fix-possible-page_cache_shift-overflows	Wed Nov 23 13:22:12 2005
+++ 25-akpm/fs/buffer.c	Wed Nov 23 13:22:12 2005
@@ -1768,7 +1768,7 @@ static int __block_write_full_page(struc
 	 * handle that here by just cleaning them.
 	 */
 
-	block = page->index << (PAGE_CACHE_SHIFT - inode->i_blkbits);
+	block = (sector_t)page->index << (PAGE_CACHE_SHIFT - inode->i_blkbits);
 	head = page_buffers(page);
 	bh = head;
 
@@ -2610,7 +2610,7 @@ int block_truncate_page(struct address_s
 	pgoff_t index = from >> PAGE_CACHE_SHIFT;
 	unsigned offset = from & (PAGE_CACHE_SIZE-1);
 	unsigned blocksize;
-	pgoff_t iblock;
+	sector_t iblock;
 	unsigned length, pos;
 	struct inode *inode = mapping->host;
 	struct page *page;
@@ -2626,7 +2626,7 @@ int block_truncate_page(struct address_s
 		return 0;
 
 	length = blocksize - length;
-	iblock = index << (PAGE_CACHE_SHIFT - inode->i_blkbits);
+	iblock = (sector_t)index << (PAGE_CACHE_SHIFT - inode->i_blkbits);
 	
 	page = grab_cache_page(mapping, index);
 	err = -ENOMEM;
diff -puN fs/freevxfs/vxfs_immed.c~fix-possible-page_cache_shift-overflows fs/freevxfs/vxfs_immed.c
--- 25/fs/freevxfs/vxfs_immed.c~fix-possible-page_cache_shift-overflows	Wed Nov 23 13:22:12 2005
+++ 25-akpm/fs/freevxfs/vxfs_immed.c	Wed Nov 23 13:22:12 2005
@@ -99,8 +99,8 @@ static int
 vxfs_immed_readpage(struct file *fp, struct page *pp)
 {
 	struct vxfs_inode_info	*vip = VXFS_INO(pp->mapping->host);
-	u_int64_t		offset = pp->index << PAGE_CACHE_SHIFT;
-	caddr_t			kaddr;
+	u_int64_t	offset = (u_int64_t)pp->index << PAGE_CACHE_SHIFT;
+	caddr_t		kaddr;
 
 	kaddr = kmap(pp);
 	memcpy(kaddr, vip->vii_immed.vi_immed + offset, PAGE_CACHE_SIZE);
diff -puN fs/hostfs/hostfs_kern.c~fix-possible-page_cache_shift-overflows fs/hostfs/hostfs_kern.c
--- 25/fs/hostfs/hostfs_kern.c~fix-possible-page_cache_shift-overflows	Wed Nov 23 13:22:12 2005
+++ 25-akpm/fs/hostfs/hostfs_kern.c	Wed Nov 23 13:22:12 2005
@@ -913,7 +913,7 @@ int hostfs_link_readpage(struct file *fi
 	long long start;
 	int err;
 
-	start = page->index << PAGE_CACHE_SHIFT;
+	start = (long long)page->index << PAGE_CACHE_SHIFT;
 	buffer = kmap(page);
 	name = inode_name(page->mapping->host, 0);
 	if(name == NULL) return(-ENOMEM);
diff -puN fs/jffs/inode-v23.c~fix-possible-page_cache_shift-overflows fs/jffs/inode-v23.c
--- 25/fs/jffs/inode-v23.c~fix-possible-page_cache_shift-overflows	Wed Nov 23 13:22:12 2005
+++ 25-akpm/fs/jffs/inode-v23.c	Wed Nov 23 13:22:12 2005
@@ -757,7 +757,7 @@ jffs_do_readpage_nolock(struct file *fil
 
 	read_len = 0;
 	result = 0;
-	offset = page->index << PAGE_CACHE_SHIFT;
+	offset = (loff_t)page->index << PAGE_CACHE_SHIFT;
 
 	kmap(page);
 	buf = page_address(page);
@@ -1545,7 +1545,7 @@ jffs_commit_write(struct file *filp, str
 {
        void *addr = page_address(page) + from;
        /* XXX: PAGE_CACHE_SHIFT or PAGE_SHIFT */
-       loff_t pos = (page->index<<PAGE_CACHE_SHIFT) + from;
+       loff_t pos = ((loff_t)page->index<<PAGE_CACHE_SHIFT) + from;
 
        return jffs_file_write(filp, addr, to-from, &pos);
 } /* jffs_commit_write() */
diff -puN fs/mpage.c~fix-possible-page_cache_shift-overflows fs/mpage.c
--- 25/fs/mpage.c~fix-possible-page_cache_shift-overflows	Wed Nov 23 13:22:12 2005
+++ 25-akpm/fs/mpage.c	Wed Nov 23 13:22:12 2005
@@ -184,7 +184,7 @@ do_mpage_readpage(struct bio *bio, struc
 	if (page_has_buffers(page))
 		goto confused;
 
-	block_in_file = page->index << (PAGE_CACHE_SHIFT - blkbits);
+	block_in_file = (sector_t)page->index << (PAGE_CACHE_SHIFT - blkbits);
 	last_block = (i_size_read(inode) + blocksize - 1) >> blkbits;
 
 	bh.b_page = page;
@@ -466,7 +466,7 @@ __mpage_writepage(struct bio *bio, struc
 	 * The page has no buffers: map it to disk
 	 */
 	BUG_ON(!PageUptodate(page));
-	block_in_file = page->index << (PAGE_CACHE_SHIFT - blkbits);
+	block_in_file = (sector_t)page->index << (PAGE_CACHE_SHIFT - blkbits);
 	last_block = (i_size - 1) >> blkbits;
 	map_bh.b_page = page;
 	for (page_block = 0; page_block < blocks_per_page; ) {
diff -puN fs/romfs/inode.c~fix-possible-page_cache_shift-overflows fs/romfs/inode.c
--- 25/fs/romfs/inode.c~fix-possible-page_cache_shift-overflows	Wed Nov 23 13:22:12 2005
+++ 25-akpm/fs/romfs/inode.c	Wed Nov 23 13:22:12 2005
@@ -418,7 +418,7 @@ static int
 romfs_readpage(struct file *file, struct page * page)
 {
 	struct inode *inode = page->mapping->host;
-	unsigned long offset, avail, readlen;
+	loff_t offset, avail, readlen;
 	void *buf;
 	int result = -EIO;
 
@@ -429,8 +429,8 @@ romfs_readpage(struct file *file, struct
 		goto err_out;
 
 	/* 32 bit warning -- but not for us :) */
-	offset = page->index << PAGE_CACHE_SHIFT;
-	if (offset < inode->i_size) {
+	offset = (loff_t)page->index << PAGE_CACHE_SHIFT;
+	if (offset < i_size_read(inode)) {
 		avail = inode->i_size-offset;
 		readlen = min_t(unsigned long, avail, PAGE_SIZE);
 		if (romfs_copyfrom(inode, buf, ROMFS_I(inode)->i_dataoffset+offset, readlen) == readlen) {
diff -puN fs/smbfs/file.c~fix-possible-page_cache_shift-overflows fs/smbfs/file.c
--- 25/fs/smbfs/file.c~fix-possible-page_cache_shift-overflows	Wed Nov 23 13:22:12 2005
+++ 25-akpm/fs/smbfs/file.c	Wed Nov 23 13:22:12 2005
@@ -209,8 +209,8 @@ smb_updatepage(struct file *file, struct
 {
 	struct dentry *dentry = file->f_dentry;
 
-	DEBUG1("(%s/%s %d@%ld)\n", DENTRY_PATH(dentry), 
-	       count, (page->index << PAGE_CACHE_SHIFT)+offset);
+	DEBUG1("(%s/%s %d@%lld)\n", DENTRY_PATH(dentry), count,
+		((unsigned long long)page->index << PAGE_CACHE_SHIFT) + offset);
 
 	return smb_writepage_sync(dentry->d_inode, page, offset, count);
 }
diff -puN fs/sysv/dir.c~fix-possible-page_cache_shift-overflows fs/sysv/dir.c
--- 25/fs/sysv/dir.c~fix-possible-page_cache_shift-overflows	Wed Nov 23 13:22:12 2005
+++ 25-akpm/fs/sysv/dir.c	Wed Nov 23 13:22:12 2005
@@ -103,7 +103,7 @@ static int sysv_readdir(struct file * fi
 			offset = (char *)de - kaddr;
 
 			over = filldir(dirent, name, strnlen(name,SYSV_NAMELEN),
-					(n<<PAGE_CACHE_SHIFT) | offset,
+					((loff_t)n<<PAGE_CACHE_SHIFT) | offset,
 					fs16_to_cpu(SYSV_SB(sb), de->inode),
 					DT_UNKNOWN);
 			if (over) {
@@ -115,7 +115,7 @@ static int sysv_readdir(struct file * fi
 	}
 
 done:
-	filp->f_pos = (n << PAGE_CACHE_SHIFT) | offset;
+	filp->f_pos = ((loff_t)n << PAGE_CACHE_SHIFT) | offset;
 	unlock_kernel();
 	return 0;
 }
_

Patches currently in -mm which might be from akpm@osdl.org are

linus.patch
revert-floppy-fix-read-only-handling.patch
jffs2-debug-gcc-29x-fix.patch


^ permalink raw reply	[flat|nested] 11+ messages in thread

* ntfs - was: Re: + fix-possible-page_cache_shift-overflows.patch added to -mm tree
  2005-11-23 21:22 + fix-possible-page_cache_shift-overflows.patch added to -mm tree akpm
@ 2005-11-23 22:01 ` Anton Altaparmakov
  2005-11-24  1:00 ` Blaisorblade
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Anton Altaparmakov @ 2005-11-23 22:01 UTC (permalink / raw)
  To: akpm
  Cc: blaisorblade, dhowells, dwmw2, green, hch, jdike, linux-fsdevel,
	miklos, neilb, reiserfs-dev, rmk, trond.myklebust, zippel,
	mm-commits

On Wed, 23 Nov 2005, akpm@osdl.org wrote:
> 
> The patch titled
> 
>      fix possible PAGE_CACHE_SHIFT overflows
> 
> has been added to the -mm tree.  Its filename is
> 
>      fix-possible-page_cache_shift-overflows.patch
> 
> 
> From: Andrew Morton <akpm@osdl.org>
> 
> We've had two instances recently of overflows when doing
> 
> 	64_bit_value = (32_bit_value << PAGE_CACHE_SHIFT)
> 
> I did a tree-wide grep of `<<.*PAGE_CACHE_SHIFT' and this is the result.
[snip]
> - ntfs_read_compressed_block is doing fishy things with cur_page and
>   cb_max_page.

Those are both fine.  They deal with relative offsets rather than absolute 
page->index and the number of pages is very low (on a 4k PAGE_SIZE 
architecture the maximum number of pages is 16, thus cannot overflow when 
you shiftleft by PAGE_CACHE_SHIFT (16 * 4096 = 65536) and the offsets 
added are also relative to the page and the compression block thus a very 
maximum value of 65536 + 4095 (PAGE_SIZE - 1) + 65536 (maximum compression 
block size) = 135167 which is well within a signed 32-bit variable...

> - load_and_init_attrdef()'s handling of `index' needs to be checked.  Ditto

Towards the top of the function you see:

        i_size = i_size_read(ino);      
        if (i_size <= 0 || i_size > 0x7fffffff)
                goto iput_failed;

Thus the file size is limited to 31-bits thus index << PAGE_CACHE_SHIFT 
can never overflow 32-bits.

>   load_and_init_upcase().  Ditto

As above file size is limited towards top of the function:

        i_size = i_size_read(ino);
        if (!i_size || i_size & (sizeof(ntfschar) - 1) ||
                        i_size > 64ULL * 1024 * sizeof(ntfschar))
                goto iput_upcase_failed;

Since sizeof(ntfschar) is always 2 bytes, the file size is limited to 
128kiB thus index << PAGE_CACHE_SHIFT can never overflow.

>   ntfs_attr_extend_initialized():initialized_size

Yep, I can see a bug there!  I missed one (s64) cast when I had it in the 
four other places doing a << PAGE_CACHE_SHIFT around the same place.  )-:

-		ni->initialized_size = (index + 1) << PAGE_CACHE_SHIFT;
+		ni->initialized_size = (s64)(index + 1) << PAGE_CACHE_SHIFT;

Well caught!  Thanks!  Normally I am quite careful about this as I already 
have been hit in ntfs by such overflows (a long time ago when the new 
ntfs driver was in early stages of development)...

I will fix it tomorrow and push to my ntfs-2.6-devel.git tree so you 
will hopefully pick it up for your next -mm...

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: + fix-possible-page_cache_shift-overflows.patch added to -mm tree
  2005-11-23 21:22 + fix-possible-page_cache_shift-overflows.patch added to -mm tree akpm
  2005-11-23 22:01 ` ntfs - was: " Anton Altaparmakov
@ 2005-11-24  1:00 ` Blaisorblade
  2005-11-24  1:09   ` Andrew Morton
  2005-11-24  7:05 ` Miklos Szeredi
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Blaisorblade @ 2005-11-24  1:00 UTC (permalink / raw)
  To: akpm
  Cc: aia21, dhowells, dwmw2, green, hch, jdike, linux-fsdevel, miklos,
	neilb, reiserfs-dev, rmk, trond.myklebust, zippel, mm-commits

On Wednesday 23 November 2005 22:22, akpm@osdl.org wrote:
> The patch titled
>
>      fix possible PAGE_CACHE_SHIFT overflows
>
> has been added to the -mm tree.  Its filename is
>
>      fix-possible-page_cache_shift-overflows.patch

Given a look and the variable you're correcting s unused in that function... 
you can drop the hunk, I've locally queued a patch to remove the var 
altogether.

> diff -puN fs/hostfs/hostfs_kern.c~fix-possible-page_cache_shift-overflows
> fs/hostfs/hostfs_kern.c ---
> 25/fs/hostfs/hostfs_kern.c~fix-possible-page_cache_shift-overflows	Wed Nov
> 23 13:22:12 2005 +++ 25-akpm/fs/hostfs/hostfs_kern.c	Wed Nov 23 13:22:12
> 2005
> @@ -913,7 +913,7 @@ int hostfs_link_readpage(struct file *fi
>  	long long start;
>  	int err;
>
> -	start = page->index << PAGE_CACHE_SHIFT;
> +	start = (long long)page->index << PAGE_CACHE_SHIFT;
>  	buffer = kmap(page);
>  	name = inode_name(page->mapping->host, 0);
>  	if(name == NULL) return(-ENOMEM);

-- 
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade (Skype ID "PaoloGiarrusso", ICQ 215621894)
http://www.user-mode-linux.org/~blaisorblade

	

	
		
___________________________________ 
Yahoo! Mail: gratis 1GB per i messaggi e allegati da 10MB 
http://mail.yahoo.it


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: + fix-possible-page_cache_shift-overflows.patch added to -mm tree
  2005-11-24  1:00 ` Blaisorblade
@ 2005-11-24  1:09   ` Andrew Morton
  2005-11-24  1:37     ` Blaisorblade
  2005-11-24 10:02     ` Christoph Hellwig
  0 siblings, 2 replies; 11+ messages in thread
From: Andrew Morton @ 2005-11-24  1:09 UTC (permalink / raw)
  To: Blaisorblade
  Cc: aia21, dhowells, dwmw2, green, hch, jdike, linux-fsdevel, miklos,
	neilb, reiserfs-dev, rmk, trond.myklebust, zippel, mm-commits

Blaisorblade <blaisorblade@yahoo.it> wrote:
>
> On Wednesday 23 November 2005 22:22, akpm@osdl.org wrote:
> > The patch titled
> >
> >      fix possible PAGE_CACHE_SHIFT overflows
> >
> > has been added to the -mm tree.  Its filename is
> >
> >      fix-possible-page_cache_shift-overflows.patch
> 
> Given a look and the variable you're correcting s unused in that function... 
> you can drop the hunk, I've locally queued a patch to remove the var 
> altogether.
> 
> > diff -puN fs/hostfs/hostfs_kern.c~fix-possible-page_cache_shift-overflows
> > fs/hostfs/hostfs_kern.c ---
> > 25/fs/hostfs/hostfs_kern.c~fix-possible-page_cache_shift-overflows	Wed Nov
> > 23 13:22:12 2005 +++ 25-akpm/fs/hostfs/hostfs_kern.c	Wed Nov 23 13:22:12
> > 2005
> > @@ -913,7 +913,7 @@ int hostfs_link_readpage(struct file *fi
> >  	long long start;
> >  	int err;
> >
> > -	start = page->index << PAGE_CACHE_SHIFT;
> > +	start = (long long)page->index << PAGE_CACHE_SHIFT;
> >  	buffer = kmap(page);
> >  	name = inode_name(page->mapping->host, 0);
> >  	if(name == NULL) return(-ENOMEM);
> 

OK.   But hostfs_commit_write() is wrong.

	long long start;
	int err = 0;

	start = (long long) (page->index << PAGE_CACHE_SHIFT) + from;


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: + fix-possible-page_cache_shift-overflows.patch added to -mm tree
  2005-11-24  1:09   ` Andrew Morton
@ 2005-11-24  1:37     ` Blaisorblade
  2005-11-24 10:02     ` Christoph Hellwig
  1 sibling, 0 replies; 11+ messages in thread
From: Blaisorblade @ 2005-11-24  1:37 UTC (permalink / raw)
  To: Andrew Morton
  Cc: aia21, dhowells, dwmw2, green, hch, jdike, linux-fsdevel, miklos,
	neilb, reiserfs-dev, rmk, trond.myklebust, zippel, mm-commits

On Thursday 24 November 2005 02:09, Andrew Morton wrote:
> Blaisorblade <blaisorblade@yahoo.it> wrote:
> > On Wednesday 23 November 2005 22:22, akpm@osdl.org wrote:

> OK.   But hostfs_commit_write() is wrong.
>
> 	long long start;
> 	int err = 0;
>
> 	start = (long long) (page->index << PAGE_CACHE_SHIFT) + from;

Right about the cast done after the shift (fixed locally), don't know this 
late if anything else is wrong.

Adding from seems wrong for purposes of increasing inode->i_size, BUT 
write_file() increases start of the right amount (to - from).

I've added a comment to that puzzle though.
-- 
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade (Skype ID "PaoloGiarrusso", ICQ 215621894)
http://www.user-mode-linux.org/~blaisorblade

	

	
		
___________________________________ 
Yahoo! Mail: gratis 1GB per i messaggi e allegati da 10MB 
http://mail.yahoo.it


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: + fix-possible-page_cache_shift-overflows.patch added to -mm tree
  2005-11-23 21:22 + fix-possible-page_cache_shift-overflows.patch added to -mm tree akpm
  2005-11-23 22:01 ` ntfs - was: " Anton Altaparmakov
  2005-11-24  1:00 ` Blaisorblade
@ 2005-11-24  7:05 ` Miklos Szeredi
  2005-11-24  7:46 ` David Woodhouse
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Miklos Szeredi @ 2005-11-24  7:05 UTC (permalink / raw)
  To: akpm
  Cc: aia21, blaisorblade, dhowells, dwmw2, green, hch, jdike,
	linux-fsdevel, neilb, reiserfs-dev, rmk, trond.myklebust, zippel,
	mm-commits

> - fuse_send_readpages() is limited to 4G

It's actually limited to much less than that.

: static int fuse_send_readpages(struct fuse_req *req, struct file *file,
: 			       struct inode *inode)
: {
: 	loff_t pos = (loff_t) req->pages[0]->index << PAGE_CACHE_SHIFT;
: 	size_t count = req->num_pages << PAGE_CACHE_SHIFT;

req->num_pages is limited to FUSE_MAX_PAGES_PER_REQ (32), so there
cannot be an overflow.

Miklos

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: + fix-possible-page_cache_shift-overflows.patch added to -mm tree
  2005-11-23 21:22 + fix-possible-page_cache_shift-overflows.patch added to -mm tree akpm
                   ` (2 preceding siblings ...)
  2005-11-24  7:05 ` Miklos Szeredi
@ 2005-11-24  7:46 ` David Woodhouse
  2005-11-24 11:24 ` Oleg Drokin
  2005-11-24 21:08 ` Roman Zippel
  5 siblings, 0 replies; 11+ messages in thread
From: David Woodhouse @ 2005-11-24  7:46 UTC (permalink / raw)
  To: akpm
  Cc: aia21, blaisorblade, dhowells, green, hch, jdike, linux-fsdevel,
	miklos, neilb, reiserfs-dev, rmk, trond.myklebust, zippel,
	mm-commits

On Wed, 2005-11-23 at 13:22 -0800, akpm@osdl.org wrote:
> - jffs2_read_inode_range() takes a uint32_t offset, which also seems
> wrong.

JFFS and JFFS2 are both limited to 4GiB file size.

-- 
dwmw2



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: + fix-possible-page_cache_shift-overflows.patch added to -mm tree
  2005-11-24  1:09   ` Andrew Morton
  2005-11-24  1:37     ` Blaisorblade
@ 2005-11-24 10:02     ` Christoph Hellwig
  1 sibling, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2005-11-24 10:02 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Blaisorblade, aia21, dhowells, dwmw2, green, hch, jdike,
	linux-fsdevel, miklos, neilb, reiserfs-dev, rmk, trond.myklebust,
	zippel, mm-commits

On Wed, Nov 23, 2005 at 05:09:03PM -0800, Andrew Morton wrote:
> > > +	start = (long long)page->index << PAGE_CACHE_SHIFT;

instead of a construct like this please always use page_offset()


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: + fix-possible-page_cache_shift-overflows.patch added to -mm tree
  2005-11-23 21:22 + fix-possible-page_cache_shift-overflows.patch added to -mm tree akpm
                   ` (3 preceding siblings ...)
  2005-11-24  7:46 ` David Woodhouse
@ 2005-11-24 11:24 ` Oleg Drokin
  2005-11-26  0:54   ` Hans Reiser
  2005-11-24 21:08 ` Roman Zippel
  5 siblings, 1 reply; 11+ messages in thread
From: Oleg Drokin @ 2005-11-24 11:24 UTC (permalink / raw)
  To: akpm
  Cc: aia21, blaisorblade, dhowells, dwmw2, hch, jdike, linux-fsdevel,
	miklos, neilb, reiserfs-dev, rmk, trond.myklebust, zippel,
	mm-commits

Hello!

On Wed, Nov 23, 2005 at 01:22:16PM -0800, akpm@osdl.org wrote:

> - reiserfs map_block_for_writepage() takes an unsigned long for the block -
>   it should take sector_t.  (It'll fail for huge filesystems with
>   blocksize<PAGE_CACHE_SIZE)

reiserfs is almost never used with blocksize other than 4k (And support
for block size smaller than 4k appeared only around 3 years ago).
And the only supported architectures that have PAGE_CACHE_SIZE bigger
than 4k are 64bit ones, so they are safe still.
Also sector_t is not very suitable for the job, because it is still 32 bit
with common config settings (no large block devices support).
So before proposing, even cosmetic, patch here, I want to understand - do we
allow files to be bigger than (u64)((sector_t)-1)*fs_block_size?
If we do, then we are in trouble even with sector_t. (And I do not see
what would prevent us to have a file that is bigger than 4T on ext2 with
512 byte blocksize, for example).

Also thanks to Andrews, I now see another overflow in reiserfs that should
lead to data corruptions with files that are bigger than 4G under certain
circumstances when using mmap.
In map_block_for_writepage():
        loff_t byte_offset = (block << inode->i_sb->s_blocksize_bits) + 1;

Please consider patch below:

--- linux-2.6.14/fs/reiserfs/inode.c.orig	2005-11-24 13:26:26.000000000 +0200
+++ linux-2.6.14/fs/reiserfs/inode.c	2005-11-24 13:28:35.000000000 +0200
@@ -2194,7 +2194,7 @@ static int map_block_for_writepage(struc
 	INITIALIZE_PATH(path);
 	int pos_in_item;
 	int jbegin_count = JOURNAL_PER_BALANCE_CNT;
-	loff_t byte_offset = (block << inode->i_sb->s_blocksize_bits) + 1;
+	loff_t byte_offset = ((loff_t)block << inode->i_sb->s_blocksize_bits)+1;
 	int retval;
 	int use_get_block = 0;
 	int bytes_copied = 0;


Bye,
    Oleg

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: + fix-possible-page_cache_shift-overflows.patch added to -mm tree
  2005-11-23 21:22 + fix-possible-page_cache_shift-overflows.patch added to -mm tree akpm
                   ` (4 preceding siblings ...)
  2005-11-24 11:24 ` Oleg Drokin
@ 2005-11-24 21:08 ` Roman Zippel
  5 siblings, 0 replies; 11+ messages in thread
From: Roman Zippel @ 2005-11-24 21:08 UTC (permalink / raw)
  To: akpm
  Cc: aia21, blaisorblade, dhowells, dwmw2, green, hch, jdike,
	linux-fsdevel, miklos, neilb, reiserfs-dev, rmk, trond.myklebust,
	mm-commits

Hi,

On Wednesday 23 November 2005 22:22, akpm@osdl.org wrote:

> - hfsplus_releasepage():nidx is u32, which might be wrong.
> - hfs_releasepage():nidx may be wrong.

No, it's correct.

> - affs is busted all over the place.

affs is limited in file size anyway.

bye, Roman

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: + fix-possible-page_cache_shift-overflows.patch added to -mm tree
  2005-11-24 11:24 ` Oleg Drokin
@ 2005-11-26  0:54   ` Hans Reiser
  0 siblings, 0 replies; 11+ messages in thread
From: Hans Reiser @ 2005-11-26  0:54 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: akpm, aia21, blaisorblade, dhowells, dwmw2, hch, jdike,
	linux-fsdevel, miklos, neilb, reiserfs-dev, rmk, trond.myklebust,
	zippel, mm-commits

Oleg Drokin wrote:

>
>
>Please consider patch below:
>
>--- linux-2.6.14/fs/reiserfs/inode.c.orig	2005-11-24 13:26:26.000000000 +0200
>+++ linux-2.6.14/fs/reiserfs/inode.c	2005-11-24 13:28:35.000000000 +0200
>@@ -2194,7 +2194,7 @@ static int map_block_for_writepage(struc
> 	INITIALIZE_PATH(path);
> 	int pos_in_item;
> 	int jbegin_count = JOURNAL_PER_BALANCE_CNT;
>-	loff_t byte_offset = (block << inode->i_sb->s_blocksize_bits) + 1;
>+	loff_t byte_offset = ((loff_t)block << inode->i_sb->s_blocksize_bits)+1;
> 	int retval;
> 	int use_get_block = 0;
> 	int bytes_copied = 0;
>
>
>Bye,
>    Oleg
>
>
>  
>
Thanks Oleg.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2005-11-26  0:54 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-11-23 21:22 + fix-possible-page_cache_shift-overflows.patch added to -mm tree akpm
2005-11-23 22:01 ` ntfs - was: " Anton Altaparmakov
2005-11-24  1:00 ` Blaisorblade
2005-11-24  1:09   ` Andrew Morton
2005-11-24  1:37     ` Blaisorblade
2005-11-24 10:02     ` Christoph Hellwig
2005-11-24  7:05 ` Miklos Szeredi
2005-11-24  7:46 ` David Woodhouse
2005-11-24 11:24 ` Oleg Drokin
2005-11-26  0:54   ` Hans Reiser
2005-11-24 21:08 ` Roman Zippel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).