Hi all, We have a whole pile of problems in the bounds checking of file sizes in ext2. Some of these are trivial, some relate to compatibility (especially regarding the SIGXFSZ exceeded-file-size signal), some are major holes. The 2GB limit failure is the cause of spurious "block < 0" errors that people have been reporting. The patch below is against 2.2.10 and has been tested on both 32- and 64-bit platforms (alpha and intel). Problems: 1) On 32-bit architectures, the limit on file sizes should be 2^31-1. it is being enforced as 2^32-1. This has been there since the 64-bit file size bits were added. 2) There is no bounds checking at all on ftruncate()/truncate() file sizes. We don't check the offset maximum, we don't enforce the rlimit, we don't generate SIGXFSZ as required by SUS, and we don't set the ext2 COMPAT_LARGE_FILE bit when we set the large inode size. To fix 2), we need to use ext2_notify_change, but that is broken: 3) ext2_notify_change doesn't look at iattr->ia_valid when checking for new attributes. Single-Unix compatibility problems: 4) We must not generate SIGXFSZ except for in write() or [f]truncate() system calls. In particular, we can't generate the error if we are just attempting writing to a shared mmap beyond the file size rlimit. Right now we make the signal unconditionally when allocating blocks beyond the rlimit, regardless of whether we are just filling in a hole or actually extending the file. 5) An attempt to perform a write straddling the file size rlimit must cause a short write, and must not signal SIGXFSZ. Currently we produce a SIGXFSZ in this case. 6) [f]truncate must signal SIGXFSZ if the new file size is greater than the file size rlimit, and must fail with EFBIG or EINVAL (but without SIGXFSZ) if the new file size is greater than the file's offset maximum. Currently there is absolutely no protecting logic at all in truncate, because ext2 does not use its ext2_notify_change function (the hook in the ext2 super_operations is NULL). 7) The signed/unsigned logic in ext2_file_write is not sufficiently protected against negative byte counts in the syscall. Finally, just on 64-bit architectures: 8) We are setting the COMPAT_LARGE_FILE filesystem flag when a file rolls past 4GB, not past 2GB as it should be. The patch below is composed of: * Fixing the arithmetic in both the 32 and 64-bit file bounds checking * Re-enabling ext2_notify_change, and fixing it to check ia_valid; * Adding i_size checking to ext2_notify_change, with SIGXFSZ if we exceed the soft file size limit * Removing SIGXFSZ and soft file size limit checking from the block allocation code * Adding SIGXFSZ and soft file size limit checking to ext2_file_write * Adding short write behaviour to ext2_file_write when a write overlaps the soft file size limit. On Intel, we now get short writes at the soft file size limit or at 2^31-1, as required; writes beyond that point return EFBIG, and only writes which exceed the soft file size limit generate SIGXFSZ. Notes: The patched kernel delivers SIGXFSZ when extending beyond the soft file limit for both truncate and write, but not when extending beyond the file's offset maximum. This is compatible with current Linux behaviour and appears to be consistent with the SingleUnix Spec, but SUS is a little ambiguous on this front (one reading of SUS is that the hard limit causes SIGXFSZ only on write but not truncate... ugh). Comments on this are welcome. The exact behaviour of the patched kernel is displayed by a test program: ftp://ftp.uk.linux.org/pub/linux/sct/fs/tests/file-limits.c has a couple of dozen separate checks for the exact return, errno and signal behaviour of soft and hard limit overflows. The unpatched kernel fails almost all of them. :) Comments, and I'll do a version against 2.3 once I've got feedback on this one. --Stephen ---------------------------------------------------------------- --- fs/ext2/file.c.~1~ Mon Jun 21 11:59:59 1999 +++ fs/ext2/file.c Fri Jul 9 11:10:00 1999 @@ -50,7 +50,7 @@ (1LL << (bits - 2)) * (1LL << (bits - 2)) * (1LL << (bits - 2))) * \ (1LL << bits)) - 1) -static long long ext2_max_sizes[] = { +long long ext2_max_sizes[] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, EXT2_MAX_SIZE(10), EXT2_MAX_SIZE(11), EXT2_MAX_SIZE(12), EXT2_MAX_SIZE(13) }; @@ -163,10 +163,16 @@ struct super_block * sb; int err; int i,buffercount,write_error, new_buffer; - + unsigned long limit; + /* POSIX: mtime/ctime may not change for 0 count */ if (!count) return 0; + /* This makes the bounds-checking arithmetic later on much more + * sane. */ + if (((signed) count) < 0) + return -EINVAL; + write_error = buffercount = 0; if (!inode) { printk("ext2_file_write: inode = NULL\n"); @@ -192,29 +198,38 @@ pos = *ppos; if (pos != *ppos) return -EINVAL; -#if BITS_PER_LONG >= 64 - if (pos > ext2_max_sizes[EXT2_BLOCK_SIZE_BITS(sb)]) - return -EINVAL; -#endif } /* Check for overflow.. */ + #if BITS_PER_LONG < 64 - if (pos > (__u32) (pos + count)) { - count = ~pos; /* == 0xFFFFFFFF - pos */ - if (!count) + /* If the fd's pos is already greater than or equal to the file + * descriptor's offset maximum, then we need to return EFBIG for + * any non-zero count (and we already tested for zero above). */ + if (((unsigned) pos) >= 0x7FFFFFFFUL) + return -EFBIG; + + /* If we are about to overflow the maximum file size, we also + * need to return the error, but only if no bytes can be written + * successfully. */ + if (((unsigned) pos + count) > 0x7FFFFFFFUL) { + count = 0x7FFFFFFFL - pos; + if (((signed) count) < 0) return -EFBIG; } #else { off_t max = ext2_max_sizes[EXT2_BLOCK_SIZE_BITS(sb)]; + if (pos >= max) + return -EFBIG; + if (pos + count > max) { count = max - pos; if (!count) return -EFBIG; } - if (((pos + count) >> 32) && + if (((pos + count) >> 33) && !(sb->u.ext2_sb.s_es->s_feature_ro_compat & cpu_to_le32(EXT2_FEATURE_RO_COMPAT_LARGE_FILE))) { /* If this is the first large file created, add a flag @@ -225,6 +240,20 @@ } } #endif + + /* From SUS: We must generate a SIGXFSZ for file size overflow + * only if no bytes were actually written to the file. --sct */ + + limit = current->rlim[RLIMIT_FSIZE].rlim_cur; + if (limit < RLIM_INFINITY) { + if (((unsigned) pos+count) >= limit) { + count = limit - pos; + if (((signed) count) <= 0) { + send_sig(SIGXFSZ, current, 0); + return -EFBIG; + } + } + } /* * If a file has been opened in synchronous mode, we have to ensure --- fs/ext2/inode.c.~1~ Wed May 12 15:54:22 1999 +++ fs/ext2/inode.c Fri Jul 9 11:10:39 1999 @@ -220,21 +220,7 @@ } *err = -EFBIG; if (!create) - goto dont_create; - - /* Check file limits.. */ - { - unsigned long limit = current->rlim[RLIMIT_FSIZE].rlim_cur; - if (limit < RLIM_INFINITY) { - limit >>= EXT2_BLOCK_SIZE_BITS(inode->i_sb); - if (new_block >= limit) { - send_sig(SIGXFSZ, current, 0); -dont_create: - *err = -EFBIG; - return NULL; - } - } - } + return NULL; if (inode->u.ext2_i.i_next_alloc_block == new_block) goal = inode->u.ext2_i.i_next_alloc_goal; @@ -286,7 +272,6 @@ u32 * p; struct buffer_head * result; int blocks = inode->i_sb->s_blocksize / 512; - unsigned long limit; if (!bh) return NULL; @@ -316,16 +301,6 @@ return NULL; } - limit = current->rlim[RLIMIT_FSIZE].rlim_cur; - if (limit < RLIM_INFINITY) { - limit >>= EXT2_BLOCK_SIZE_BITS(inode->i_sb); - if (new_block >= limit) { - brelse (bh); - send_sig(SIGXFSZ, current, 0); - return NULL; - } - } - if (inode->u.ext2_i.i_next_alloc_block == new_block) goal = inode->u.ext2_i.i_next_alloc_goal; if (!goal) { @@ -732,7 +707,8 @@ unsigned int flags; retval = -EPERM; - if ((iattr->ia_attr_flags & + if ((iattr->ia_valid & ATTR_ATTR_FLAG) && + (iattr->ia_attr_flags & (ATTR_FLAG_APPEND | ATTR_FLAG_IMMUTABLE)) ^ (inode->u.ext2_i.i_flags & (EXT2_APPEND_FL | EXT2_IMMUTABLE_FL))) { @@ -741,40 +717,73 @@ } else if ((current->fsuid != inode->i_uid) && !capable(CAP_FOWNER)) goto out; + if (iattr->ia_valid & ATTR_SIZE) { + off_t size = iattr->ia_size; + unsigned long limit = current->rlim[RLIMIT_FSIZE].rlim_cur; + + if (size < 0) + return -EINVAL; +#if BITS_PER_LONG == 64 + if (size > ext2_max_sizes[EXT2_BLOCK_SIZE_BITS(inode->i_sb)]) + return -EFBIG; +#endif + if (limit < RLIM_INFINITY && size > limit) { + send_sig(SIGXFSZ, current, 0); + return -EFBIG; + } + +#if BITS_PER_LONG == 64 + if (size >> 33) { + struct super_block *sb = inode->i_sb; + struct ext2_super_block *es = sb->u.ext2_sb.s_es; + if (!(es->s_feature_ro_compat & + cpu_to_le32(EXT2_FEATURE_RO_COMPAT_LARGE_FILE))){ + /* If this is the first large file + * created, add a flag to the superblock */ + es->s_feature_ro_compat |= + cpu_to_le32(EXT2_FEATURE_RO_COMPAT_LARGE_FILE); + mark_buffer_dirty(sb->u.ext2_sb.s_sbh, 1); + } + } +#endif + } + retval = inode_change_ok(inode, iattr); if (retval != 0) goto out; inode_setattr(inode, iattr); - flags = iattr->ia_attr_flags; - if (flags & ATTR_FLAG_SYNCRONOUS) { - inode->i_flags |= MS_SYNCHRONOUS; - inode->u.ext2_i.i_flags = EXT2_SYNC_FL; - } else { - inode->i_flags &= ~MS_SYNCHRONOUS; - inode->u.ext2_i.i_flags &= ~EXT2_SYNC_FL; - } - if (flags & ATTR_FLAG_NOATIME) { - inode->i_flags |= MS_NOATIME; - inode->u.ext2_i.i_flags = EXT2_NOATIME_FL; - } else { - inode->i_flags &= ~MS_NOATIME; - inode->u.ext2_i.i_flags &= ~EXT2_NOATIME_FL; - } - if (flags & ATTR_FLAG_APPEND) { - inode->i_flags |= S_APPEND; - inode->u.ext2_i.i_flags = EXT2_APPEND_FL; - } else { - inode->i_flags &= ~S_APPEND; - inode->u.ext2_i.i_flags &= ~EXT2_APPEND_FL; - } - if (flags & ATTR_FLAG_IMMUTABLE) { - inode->i_flags |= S_IMMUTABLE; - inode->u.ext2_i.i_flags = EXT2_IMMUTABLE_FL; - } else { - inode->i_flags &= ~S_IMMUTABLE; - inode->u.ext2_i.i_flags &= ~EXT2_IMMUTABLE_FL; + if (iattr->ia_valid & ATTR_ATTR_FLAG) { + flags = iattr->ia_attr_flags; + if (flags & ATTR_FLAG_SYNCRONOUS) { + inode->i_flags |= MS_SYNCHRONOUS; + inode->u.ext2_i.i_flags = EXT2_SYNC_FL; + } else { + inode->i_flags &= ~MS_SYNCHRONOUS; + inode->u.ext2_i.i_flags &= ~EXT2_SYNC_FL; + } + if (flags & ATTR_FLAG_NOATIME) { + inode->i_flags |= MS_NOATIME; + inode->u.ext2_i.i_flags = EXT2_NOATIME_FL; + } else { + inode->i_flags &= ~MS_NOATIME; + inode->u.ext2_i.i_flags &= ~EXT2_NOATIME_FL; + } + if (flags & ATTR_FLAG_APPEND) { + inode->i_flags |= S_APPEND; + inode->u.ext2_i.i_flags = EXT2_APPEND_FL; + } else { + inode->i_flags &= ~S_APPEND; + inode->u.ext2_i.i_flags &= ~EXT2_APPEND_FL; + } + if (flags & ATTR_FLAG_IMMUTABLE) { + inode->i_flags |= S_IMMUTABLE; + inode->u.ext2_i.i_flags = EXT2_IMMUTABLE_FL; + } else { + inode->i_flags &= ~S_IMMUTABLE; + inode->u.ext2_i.i_flags &= ~EXT2_IMMUTABLE_FL; + } } mark_inode_dirty(inode); out: --- fs/ext2/super.c.~1~ Tue Apr 20 22:33:36 1999 +++ fs/ext2/super.c Fri Jul 9 11:10:00 1999 @@ -134,7 +134,7 @@ ext2_write_inode, ext2_put_inode, ext2_delete_inode, - NULL, + ext2_notify_change, ext2_put_super, ext2_write_super, ext2_statfs, --- include/linux/ext2_fs.h.~1~ Wed May 12 15:54:02 1999 +++ include/linux/ext2_fs.h Fri Jul 9 11:12:00 1999 @@ -504,6 +504,10 @@ ~EXT2_DIR_ROUND) #ifdef __KERNEL__ + +/* Filesize hard limits for 64-bit file offsets */ +extern long long ext2_max_sizes[]; + /* * Function prototypes */ @@ -564,6 +568,7 @@ extern void ext2_put_inode (struct inode *); extern void ext2_delete_inode (struct inode *); extern int ext2_sync_inode (struct inode *); +extern int ext2_notify_change(struct dentry *, struct iattr *); extern void ext2_discard_prealloc (struct inode *); /* ioctl.c */
