minixfs file size check is buggy and it doesn't allow creating a block which
can't be fully filled
Signed-off-by: Vladimir Serbinenko <[email protected]>
diff --git a/fs/minix/itree_v1.c b/fs/minix/itree_v1.c
index 282e15a..4f8f8b2 100644
--- a/fs/minix/itree_v1.c
+++ b/fs/minix/itree_v1.c
@@ -29,7 +29,8 @@ static int block_to_path(struct inode * inode, long block, int offsets[DEPTH])
if (block < 0) {
printk("MINIX-fs: block_to_path: block %ld < 0 on dev %s\n",
block, bdevname(inode->i_sb->s_bdev, b));
- } else if (block >= (minix_sb(inode->i_sb)->s_max_size/BLOCK_SIZE)) {
+ } else if ((u64) block * (u64) BLOCK_SIZE
+ >= minix_sb(inode->i_sb)->s_max_size) {
if (printk_ratelimit())
printk("MINIX-fs: block_to_path: "
"block %ld too big on dev %s\n",
diff --git a/fs/minix/itree_v2.c b/fs/minix/itree_v2.c
index 13487ad..4a9a19d 100644
--- a/fs/minix/itree_v2.c
+++ b/fs/minix/itree_v2.c
@@ -32,7 +32,8 @@ static int block_to_path(struct inode * inode, long block, int offsets[DEPTH])
if (block < 0) {
printk("MINIX-fs: block_to_path: block %ld < 0 on dev %s\n",
block, bdevname(sb->s_bdev, b));
- } else if (block >= (minix_sb(inode->i_sb)->s_max_size/sb->s_blocksize)) {
+ } else if ((u64) block * (u64) sb->s_blocksize
+ >= minix_sb(inode->i_sb)->s_max_size) {
if (printk_ratelimit())
printk("MINIX-fs: block_to_path: "
"block %ld too big on dev %s\n",
On Sun 13-05-12 15:48:55, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> minixfs file size check is buggy and it doesn't allow creating a block which
> can't be fully filled
Umm, I'm not really minix expert but who'd set s_max_size to something
which is not a multiple of block size? This looks rather artifical problem
to me...
Honza
>
> Signed-off-by: Vladimir Serbinenko <[email protected]>
>
> diff --git a/fs/minix/itree_v1.c b/fs/minix/itree_v1.c
> index 282e15a..4f8f8b2 100644
> --- a/fs/minix/itree_v1.c
> +++ b/fs/minix/itree_v1.c
> @@ -29,7 +29,8 @@ static int block_to_path(struct inode * inode, long block, int offsets[DEPTH])
> if (block < 0) {
> printk("MINIX-fs: block_to_path: block %ld < 0 on dev %s\n",
> block, bdevname(inode->i_sb->s_bdev, b));
> - } else if (block >= (minix_sb(inode->i_sb)->s_max_size/BLOCK_SIZE)) {
> + } else if ((u64) block * (u64) BLOCK_SIZE
> + >= minix_sb(inode->i_sb)->s_max_size) {
> if (printk_ratelimit())
> printk("MINIX-fs: block_to_path: "
> "block %ld too big on dev %s\n",
> diff --git a/fs/minix/itree_v2.c b/fs/minix/itree_v2.c
> index 13487ad..4a9a19d 100644
> --- a/fs/minix/itree_v2.c
> +++ b/fs/minix/itree_v2.c
> @@ -32,7 +32,8 @@ static int block_to_path(struct inode * inode, long block, int offsets[DEPTH])
> if (block < 0) {
> printk("MINIX-fs: block_to_path: block %ld < 0 on dev %s\n",
> block, bdevname(sb->s_bdev, b));
> - } else if (block >= (minix_sb(inode->i_sb)->s_max_size/sb->s_blocksize)) {
> + } else if ((u64) block * (u64) sb->s_blocksize
> + >= minix_sb(inode->i_sb)->s_max_size) {
> if (printk_ratelimit())
> printk("MINIX-fs: block_to_path: "
> "block %ld too big on dev %s\n",
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On 15.05.2012 00:19, Jan Kara wrote:
> On Sun 13-05-12 15:48:55, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>> minixfs file size check is buggy and it doesn't allow creating a block which
>> can't be fully filled
> Umm, I'm not really minix expert but who'd set s_max_size to something
> which is not a multiple of block size? This looks rather artifical problem
> to me...
>
The usual and natural limit comes from interpreting 32-bit size field as
signed or unsigned. So it's either 2G - 1 or 4G - 1. Neither of which is
a multiple of block size.
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
On Tue 15-05-12 00:33:52, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 15.05.2012 00:19, Jan Kara wrote:
>
> > On Sun 13-05-12 15:48:55, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> >> minixfs file size check is buggy and it doesn't allow creating a block which
> >> can't be fully filled
> > Umm, I'm not really minix expert but who'd set s_max_size to something
> > which is not a multiple of block size? This looks rather artifical problem
> > to me...
> >
>
> The usual and natural limit comes from interpreting 32-bit size field as
> signed or unsigned. So it's either 2G - 1 or 4G - 1. Neither of which is
> a multiple of block size.
Oh, right. Then your patch should be OK, just it's enough to cast one of
the arguments to u64. And BTW looking at minix, it should also set
s_maxbytes to s_max_size. Otherwise it will be always limited by
MAX_NON_LFS which is 2^31-1.
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
> Oh, right. Then your patch should be OK, just it's enough to cast one of
> the arguments to u64.
I know. I just consider it clearer and less risk to lose it in the future.
Moreover it avoids thinking of how much is really needed
> And BTW looking at minix, it should also set
> s_maxbytes to s_max_size. Otherwise it will be always limited by
> MAX_NON_LFS which is 2^31-1.
Patch here:
minixfs file size check is buggy and it doesn't allow
creating a block which can't be fully filled
Signed-off-by: Vladimir Serbinenko <[email protected]>
---
fs/minix/inode.c | 1 +
fs/minix/itree_v1.c | 3 ++-
fs/minix/itree_v2.c | 3 ++-
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/fs/minix/inode.c b/fs/minix/inode.c
index fcb05d2..133bb02 100644
--- a/fs/minix/inode.c
+++ b/fs/minix/inode.c
@@ -227,6 +227,7 @@ static int minix_fill_super(struct super_block *s, void *data, int silent)
} else
goto out_no_fs;
+ s->s_maxbytes = sbi->s_max_size;
/*
* Allocate the buffer map to keep the superblock small.
*/
diff --git a/fs/minix/itree_v1.c b/fs/minix/itree_v1.c
index 282e15a..4f8f8b2 100644
--- a/fs/minix/itree_v1.c
+++ b/fs/minix/itree_v1.c
@@ -29,7 +29,8 @@ static int block_to_path(struct inode * inode, long block, int offsets[DEPTH])
if (block < 0) {
printk("MINIX-fs: block_to_path: block %ld < 0 on dev %s\n",
block, bdevname(inode->i_sb->s_bdev, b));
- } else if (block >= (minix_sb(inode->i_sb)->s_max_size/BLOCK_SIZE)) {
+ } else if ((u64) block * (u64) BLOCK_SIZE
+ >= minix_sb(inode->i_sb)->s_max_size) {
if (printk_ratelimit())
printk("MINIX-fs: block_to_path: "
"block %ld too big on dev %s\n",
diff --git a/fs/minix/itree_v2.c b/fs/minix/itree_v2.c
index 13487ad..4a9a19d 100644
--- a/fs/minix/itree_v2.c
+++ b/fs/minix/itree_v2.c
@@ -32,7 +32,8 @@ static int block_to_path(struct inode * inode, long block, int offsets[DEPTH])
if (block < 0) {
printk("MINIX-fs: block_to_path: block %ld < 0 on dev %s\n",
block, bdevname(sb->s_bdev, b));
- } else if (block >= (minix_sb(inode->i_sb)->s_max_size/sb->s_blocksize)) {
+ } else if ((u64) block * (u64) sb->s_blocksize
+ >= minix_sb(inode->i_sb)->s_max_size) {
if (printk_ratelimit())
printk("MINIX-fs: block_to_path: "
"block %ld too big on dev %s\n",
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko