From: Toshiyuki Okajima <[email protected]>
By running the following reproducer, we can confirm that the write
system call returns with 0.
-------------------------------------------------------------------------------
[reproducer] repro.sh
#!/bin/sh
/bin/dd if=/dev/zero of=./img bs=1k count=1 seek=1024k > /dev/null 2>&1
/sbin/mkfs.ext3 -Fq ./img
/bin/mount -o loop -t ext4 ./img /mnt
/bin/touch /mnt/file
strace /bin/dd if=/dev/zero of=/mnt/file conv=notrunc bs=1k count=1 seek=$((2194719883264/1024)) 2>&1 | /bin/egrep "write.* 1024\) = "
/bin/umount /mnt
exit
# sh ./repro.sh
write(1, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 1024) = 0
-------------------------------------------------------------------------------
2194719883264 bytes is the maximum file size of ext4 with noextent (nohugefile).
Therefore a "write system call" from 2194719883264th offset of a file should
be failed.
This problem can occur because the following check is insufficient.
58 static ssize_t
59 ext4_file_write(struct kiocb *iocb, const struct iovec *iov,
60 unsigned long nr_segs, loff_t pos)
61 {
62 struct inode *inode = iocb->ki_filp->f_path.dentry->d_inode;
63
64 /*
65 * If we have encountered a bitmap-format file, the size limit
66 * is smaller than s_maxbytes, which is for extent-mapped files.
67 */
68
69 if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))) {
70 struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
71 size_t length = iov_length(iov, nr_segs);
72
73 if (pos > sbi->s_bitmap_maxbytes)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
74 return -EFBIG;
75
76 if (pos + length > sbi->s_bitmap_maxbytes) {
77 nr_segs = iov_shorten((struct iovec *)iov,
nr_segs,
78 sbi->s_bitmap_maxbytes -
pos);
79 }
80 }
So, we need consider the case: "the file position is the max file size and
we write something to the file".
If the consideration is included, this problem can fix.
[applying the fix patch]
# sh ./repro.sh
write(1, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 1024) = -1 EFBIG (File too large)
Signed-off-by: Toshiyuki Okajima <[email protected]>
Cc: Eric Sandeen <[email protected]>
---
fs/ext4/file.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 5313ae4..3834ead 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -70,7 +70,8 @@ ext4_file_write(struct kiocb *iocb, const struct iovec *iov,
struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
size_t length = iov_length(iov, nr_segs);
- if (pos > sbi->s_bitmap_maxbytes)
+ if (pos > sbi->s_bitmap_maxbytes
+ || (pos == sbi->s_bitmap_maxbytes && length > 0))
return -EFBIG;
if (pos + length > sbi->s_bitmap_maxbytes) {
--
1.5.5.6
On 2010-07-25, at 20:16, Toshiyuki Okajima <[email protected]> wrote:
>
> - if (pos > sbi->s_bitmap_maxbytes)
> + if (pos > sbi->s_bitmap_maxbytes
> + || (pos == sbi->s_bitmap_maxbytes && length > 0))
> return -EFBIG;
Minor note - the kernel coding style is to put the '||' at the end of the first line instead of the start of the second line.
I thunk it would also be sufficient to do:
if (pos > sbi->s_bitmap_maxbytes + !!length)
return -EFBIG;
Thanks, I've applied this patch, although I edited patch description
to make it be a bit more succint.
- Ted