* [PATCH 0/2] quota: Handle corrupted quota file better @ 2020-11-02 17:27 Jan Kara 2020-11-02 17:27 ` [PATCH 1/2] quota: Don't overflow quota file offsets Jan Kara 2020-11-02 17:27 ` [PATCH 2/2] quota: Sanity-check quota file headers on load Jan Kara 0 siblings, 2 replies; 6+ messages in thread From: Jan Kara @ 2020-11-02 17:27 UTC (permalink / raw) To: linux-fsdevel; +Cc: Jan Kara Hello, syzbot has spotted that quota code does not handle corrupted quota files quite well. Let's add some sanity checks to avoid kernel crashes when accessing corrupted quota files. Honza ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] quota: Don't overflow quota file offsets 2020-11-02 17:27 [PATCH 0/2] quota: Handle corrupted quota file better Jan Kara @ 2020-11-02 17:27 ` Jan Kara 2020-11-02 21:30 ` Andreas Dilger 2020-11-02 17:27 ` [PATCH 2/2] quota: Sanity-check quota file headers on load Jan Kara 1 sibling, 1 reply; 6+ messages in thread From: Jan Kara @ 2020-11-02 17:27 UTC (permalink / raw) To: linux-fsdevel; +Cc: Jan Kara 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(-) 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] quota: Don't overflow quota file offsets 2020-11-02 17:27 ` [PATCH 1/2] quota: Don't overflow quota file offsets Jan Kara @ 2020-11-02 21:30 ` Andreas Dilger 2020-11-03 8:57 ` Jan Kara 0 siblings, 1 reply; 6+ messages in thread From: Andreas Dilger @ 2020-11-02 21:30 UTC (permalink / raw) To: Jan Kara; +Cc: linux-fsdevel [-- Attachment #1: Type: text/plain, Size: 2822 bytes --] 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 [-- Attachment #2: Message signed with OpenPGP --] [-- Type: application/pgp-signature, Size: 873 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] quota: Don't overflow quota file offsets 2020-11-02 21:30 ` Andreas Dilger @ 2020-11-03 8:57 ` Jan Kara 0 siblings, 0 replies; 6+ messages in thread From: Jan Kara @ 2020-11-03 8:57 UTC (permalink / raw) To: Andreas Dilger; +Cc: Jan Kara, linux-fsdevel 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 > > > > > -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] quota: Sanity-check quota file headers on load 2020-11-02 17:27 [PATCH 0/2] quota: Handle corrupted quota file better Jan Kara 2020-11-02 17:27 ` [PATCH 1/2] quota: Don't overflow quota file offsets Jan Kara @ 2020-11-02 17:27 ` Jan Kara 2020-11-02 21:32 ` Andreas Dilger 1 sibling, 1 reply; 6+ messages in thread From: Jan Kara @ 2020-11-02 17:27 UTC (permalink / raw) To: linux-fsdevel; +Cc: Jan Kara, stable Perform basic sanity checks of quota headers to avoid kernel crashes on corrupted quota files. CC: stable@vger.kernel.org Reported-by: syzbot+f816042a7ae2225f25ba@syzkaller.appspotmail.com Signed-off-by: Jan Kara <jack@suse.cz> --- fs/quota/quota_v2.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/fs/quota/quota_v2.c b/fs/quota/quota_v2.c index e69a2bfdd81c..c21106557a37 100644 --- a/fs/quota/quota_v2.c +++ b/fs/quota/quota_v2.c @@ -157,6 +157,25 @@ static int v2_read_file_info(struct super_block *sb, int type) qinfo->dqi_entry_size = sizeof(struct v2r1_disk_dqblk); qinfo->dqi_ops = &v2r1_qtree_ops; } + ret = -EUCLEAN; + /* Some sanity checks of the read headers... */ + if ((loff_t)qinfo->dqi_blocks << qinfo->dqi_blocksize_bits > + i_size_read(sb_dqopt(sb)->files[type])) { + quota_error(sb, "Number of blocks too big for quota file size (%llu > %llu).", + (loff_t)qinfo->dqi_blocks << qinfo->dqi_blocksize_bits, + i_size_read(sb_dqopt(sb)->files[type])); + goto out; + } + if (qinfo->dqi_free_blk >= qinfo->dqi_blocks) { + quota_error(sb, "Free block number too big (%u >= %u).", + qinfo->dqi_free_blk, qinfo->dqi_blocks); + goto out; + } + if (qinfo->dqi_free_entry >= qinfo->dqi_blocks) { + quota_error(sb, "Block with free entry too big (%u >= %u).", + qinfo->dqi_free_entry, qinfo->dqi_blocks); + goto out; + } ret = 0; out: up_read(&dqopt->dqio_sem); -- 2.16.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] quota: Sanity-check quota file headers on load 2020-11-02 17:27 ` [PATCH 2/2] quota: Sanity-check quota file headers on load Jan Kara @ 2020-11-02 21:32 ` Andreas Dilger 0 siblings, 0 replies; 6+ messages in thread From: Andreas Dilger @ 2020-11-02 21:32 UTC (permalink / raw) To: Jan Kara; +Cc: linux-fsdevel, stable [-- Attachment #1: Type: text/plain, Size: 1742 bytes --] On Nov 2, 2020, at 10:27 AM, Jan Kara <jack@suse.cz> wrote: > > Perform basic sanity checks of quota headers to avoid kernel crashes on > corrupted quota files. > > CC: stable@vger.kernel.org > Reported-by: syzbot+f816042a7ae2225f25ba@syzkaller.appspotmail.com > Signed-off-by: Jan Kara <jack@suse.cz> Looks reasonable. Reviewed-by: Andreas Dilger <adilger@dilger.ca> > --- > fs/quota/quota_v2.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/fs/quota/quota_v2.c b/fs/quota/quota_v2.c > index e69a2bfdd81c..c21106557a37 100644 > --- a/fs/quota/quota_v2.c > +++ b/fs/quota/quota_v2.c > @@ -157,6 +157,25 @@ static int v2_read_file_info(struct super_block *sb, int type) > qinfo->dqi_entry_size = sizeof(struct v2r1_disk_dqblk); > qinfo->dqi_ops = &v2r1_qtree_ops; > } > + ret = -EUCLEAN; > + /* Some sanity checks of the read headers... */ > + if ((loff_t)qinfo->dqi_blocks << qinfo->dqi_blocksize_bits > > + i_size_read(sb_dqopt(sb)->files[type])) { > + quota_error(sb, "Number of blocks too big for quota file size (%llu > %llu).", > + (loff_t)qinfo->dqi_blocks << qinfo->dqi_blocksize_bits, > + i_size_read(sb_dqopt(sb)->files[type])); > + goto out; > + } > + if (qinfo->dqi_free_blk >= qinfo->dqi_blocks) { > + quota_error(sb, "Free block number too big (%u >= %u).", > + qinfo->dqi_free_blk, qinfo->dqi_blocks); > + goto out; > + } > + if (qinfo->dqi_free_entry >= qinfo->dqi_blocks) { > + quota_error(sb, "Block with free entry too big (%u >= %u).", > + qinfo->dqi_free_entry, qinfo->dqi_blocks); > + goto out; > + } > ret = 0; > out: > up_read(&dqopt->dqio_sem); > -- > 2.16.4 > Cheers, Andreas [-- Attachment #2: Message signed with OpenPGP --] [-- Type: application/pgp-signature, Size: 873 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-03 8:57 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-02 17:27 [PATCH 0/2] quota: Handle corrupted quota file better Jan Kara 2020-11-02 17:27 ` [PATCH 1/2] quota: Don't overflow quota file offsets Jan Kara 2020-11-02 21:30 ` Andreas Dilger 2020-11-03 8:57 ` Jan Kara 2020-11-02 17:27 ` [PATCH 2/2] quota: Sanity-check quota file headers on load Jan Kara 2020-11-02 21:32 ` Andreas Dilger
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.