| Message ID | 20201102172733.23444-2-jack@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | quota: Handle corrupted quota file better | expand |
On Nov 2, 2020, at 10:27 AM, Jan Kara <jack@suse.cz> wrote: > > The on-disk quota format supports quota files with upto 2^32 blocks. Be > careful when computing quota file offsets in the quota files from block > numbers as they can overflow 32-bit types. Since quota files larger than > 4GB would require ~26 millions of quota users, this is mostly a > theoretical concern now but better be careful, fuzzers would find the > problem sooner or later anyway... > > Signed-off-by: Jan Kara <jack@suse.cz> Out of curiosity, is this 26 million *quota entries*, or is it just a UID larger than 26M? At one point the quota files were sparse and indexed by the UID, but I guess very file name "quota tree" means this is not correct. Is there some document/comment that describes the on-disk quota file format? In any case, the change makes sense regardless, since ->quota_read() takes loff_t for the offset. Reviewed-by: Andreas Dilger <adilger@dilger.ca> > --- > fs/quota/quota_tree.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/fs/quota/quota_tree.c b/fs/quota/quota_tree.c > index a6f856f341dc..c5562c871c8b 100644 > --- a/fs/quota/quota_tree.c > +++ b/fs/quota/quota_tree.c > @@ -62,7 +62,7 @@ static ssize_t read_blk(struct qtree_mem_dqinfo *info, uint blk, char *buf) > > memset(buf, 0, info->dqi_usable_bs); > return sb->s_op->quota_read(sb, info->dqi_type, buf, > - info->dqi_usable_bs, blk << info->dqi_blocksize_bits); > + info->dqi_usable_bs, (loff_t)blk << info->dqi_blocksize_bits); > } > > static ssize_t write_blk(struct qtree_mem_dqinfo *info, uint blk, char *buf) > @@ -71,7 +71,7 @@ static ssize_t write_blk(struct qtree_mem_dqinfo *info, uint blk, char *buf) > ssize_t ret; > > ret = sb->s_op->quota_write(sb, info->dqi_type, buf, > - info->dqi_usable_bs, blk << info->dqi_blocksize_bits); > + info->dqi_usable_bs, (loff_t)blk << info->dqi_blocksize_bits); > if (ret != info->dqi_usable_bs) { > quota_error(sb, "dquota write failed"); > if (ret >= 0) > @@ -284,7 +284,7 @@ static uint find_free_dqentry(struct qtree_mem_dqinfo *info, > blk); > goto out_buf; > } > - dquot->dq_off = (blk << info->dqi_blocksize_bits) + > + dquot->dq_off = ((loff_t)blk << info->dqi_blocksize_bits) + > sizeof(struct qt_disk_dqdbheader) + > i * info->dqi_entry_size; > kfree(buf); > @@ -559,7 +559,7 @@ static loff_t find_block_dqentry(struct qtree_mem_dqinfo *info, > ret = -EIO; > goto out_buf; > } else { > - ret = (blk << info->dqi_blocksize_bits) + sizeof(struct > + ret = ((loff_t)blk << info->dqi_blocksize_bits) + sizeof(struct > qt_disk_dqdbheader) + i * info->dqi_entry_size; > } > out_buf: > -- > 2.16.4 > Cheers, Andreas
On Mon 02-11-20 14:30:37, Andreas Dilger wrote: > On Nov 2, 2020, at 10:27 AM, Jan Kara <jack@suse.cz> wrote: > > > > The on-disk quota format supports quota files with upto 2^32 blocks. Be > > careful when computing quota file offsets in the quota files from block > > numbers as they can overflow 32-bit types. Since quota files larger than > > 4GB would require ~26 millions of quota users, this is mostly a > > theoretical concern now but better be careful, fuzzers would find the > > problem sooner or later anyway... > > > > Signed-off-by: Jan Kara <jack@suse.cz> > > Out of curiosity, is this 26 million *quota entries*, or is it just a UID > larger than 26M? At one point the quota files were sparse and indexed by > the UID, but I guess very file name "quota tree" means this is not correct. > Is there some document/comment that describes the on-disk quota file format? It is really 26M different UIDs/GIDs/ProjectIDs. The sparse file format you describe is the original quota format (implemented by fs/quota/quota_v1.c) that the current format superseeded sometime in 1999 or so ;). The current format uses radix tree for structure lookup and then structures are just stored in a linear array. There's documentation of the format in quota-tools in doc/quotadoc.sgml. > In any case, the change makes sense regardless, since ->quota_read() takes > loff_t for the offset. > > Reviewed-by: Andreas Dilger <adilger@dilger.ca> Thanks! Honza > > > --- > > fs/quota/quota_tree.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/fs/quota/quota_tree.c b/fs/quota/quota_tree.c > > index a6f856f341dc..c5562c871c8b 100644 > > --- a/fs/quota/quota_tree.c > > +++ b/fs/quota/quota_tree.c > > @@ -62,7 +62,7 @@ static ssize_t read_blk(struct qtree_mem_dqinfo *info, uint blk, char *buf) > > > > memset(buf, 0, info->dqi_usable_bs); > > return sb->s_op->quota_read(sb, info->dqi_type, buf, > > - info->dqi_usable_bs, blk << info->dqi_blocksize_bits); > > + info->dqi_usable_bs, (loff_t)blk << info->dqi_blocksize_bits); > > } > > > > static ssize_t write_blk(struct qtree_mem_dqinfo *info, uint blk, char *buf) > > @@ -71,7 +71,7 @@ static ssize_t write_blk(struct qtree_mem_dqinfo *info, uint blk, char *buf) > > ssize_t ret; > > > > ret = sb->s_op->quota_write(sb, info->dqi_type, buf, > > - info->dqi_usable_bs, blk << info->dqi_blocksize_bits); > > + info->dqi_usable_bs, (loff_t)blk << info->dqi_blocksize_bits); > > if (ret != info->dqi_usable_bs) { > > quota_error(sb, "dquota write failed"); > > if (ret >= 0) > > @@ -284,7 +284,7 @@ static uint find_free_dqentry(struct qtree_mem_dqinfo *info, > > blk); > > goto out_buf; > > } > > - dquot->dq_off = (blk << info->dqi_blocksize_bits) + > > + dquot->dq_off = ((loff_t)blk << info->dqi_blocksize_bits) + > > sizeof(struct qt_disk_dqdbheader) + > > i * info->dqi_entry_size; > > kfree(buf); > > @@ -559,7 +559,7 @@ static loff_t find_block_dqentry(struct qtree_mem_dqinfo *info, > > ret = -EIO; > > goto out_buf; > > } else { > > - ret = (blk << info->dqi_blocksize_bits) + sizeof(struct > > + ret = ((loff_t)blk << info->dqi_blocksize_bits) + sizeof(struct > > qt_disk_dqdbheader) + i * info->dqi_entry_size; > > } > > out_buf: > > -- > > 2.16.4 > > > > > Cheers, Andreas > > > > >
diff --git a/fs/quota/quota_tree.c b/fs/quota/quota_tree.c index a6f856f341dc..c5562c871c8b 100644 --- a/fs/quota/quota_tree.c +++ b/fs/quota/quota_tree.c @@ -62,7 +62,7 @@ static ssize_t read_blk(struct qtree_mem_dqinfo *info, uint blk, char *buf) memset(buf, 0, info->dqi_usable_bs); return sb->s_op->quota_read(sb, info->dqi_type, buf, - info->dqi_usable_bs, blk << info->dqi_blocksize_bits); + info->dqi_usable_bs, (loff_t)blk << info->dqi_blocksize_bits); } static ssize_t write_blk(struct qtree_mem_dqinfo *info, uint blk, char *buf) @@ -71,7 +71,7 @@ static ssize_t write_blk(struct qtree_mem_dqinfo *info, uint blk, char *buf) ssize_t ret; ret = sb->s_op->quota_write(sb, info->dqi_type, buf, - info->dqi_usable_bs, blk << info->dqi_blocksize_bits); + info->dqi_usable_bs, (loff_t)blk << info->dqi_blocksize_bits); if (ret != info->dqi_usable_bs) { quota_error(sb, "dquota write failed"); if (ret >= 0) @@ -284,7 +284,7 @@ static uint find_free_dqentry(struct qtree_mem_dqinfo *info, blk); goto out_buf; } - dquot->dq_off = (blk << info->dqi_blocksize_bits) + + dquot->dq_off = ((loff_t)blk << info->dqi_blocksize_bits) + sizeof(struct qt_disk_dqdbheader) + i * info->dqi_entry_size; kfree(buf); @@ -559,7 +559,7 @@ static loff_t find_block_dqentry(struct qtree_mem_dqinfo *info, ret = -EIO; goto out_buf; } else { - ret = (blk << info->dqi_blocksize_bits) + sizeof(struct + ret = ((loff_t)blk << info->dqi_blocksize_bits) + sizeof(struct qt_disk_dqdbheader) + i * info->dqi_entry_size; } out_buf:
The on-disk quota format supports quota files with upto 2^32 blocks. Be careful when computing quota file offsets in the quota files from block numbers as they can overflow 32-bit types. Since quota files larger than 4GB would require ~26 millions of quota users, this is mostly a theoretical concern now but better be careful, fuzzers would find the problem sooner or later anyway... Signed-off-by: Jan Kara <jack@suse.cz> --- fs/quota/quota_tree.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)