| Message ID | 1463883560-2442-1-git-send-email-tytso@mit.edu |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Sun, May 22, 2016 at 10:19 AM, Theodore Ts'o <tytso@mit.edu> wrote: > Creating a file system with project quotas can fail if mke2fs is built > using hardening options. This is because quota_compute_usage() used > ext2fs_get_next_inode() instead of ext2fs_get_inode_full(), and a > small inode was passed into quota_data_add, when a large inode needs > to be used. As a result get_dq() would end up dereferencing undefined > space in the stack. Without the hardening options, this would be > zero, so "mke2fs -t ext4 -O project.quota -I 256 test.img" would work > essentially by accident. > > Fix this by using ext2fs_get_inode_full() so that a large inode is > available to quota_data_inodes(). > > Signed-off-by: Theodore Ts'o <tytso@mit.edu> I thought Li Xi sent updated e2fsprogs including this fixing parts. maybe because you merged early version patches. Reviewed-by: Wang Shilong <wshilong@ddn.com> > --- > lib/support/mkquota.c | 21 +++++++++++++-------- > 1 file changed, 13 insertions(+), 8 deletions(-) > > diff --git a/lib/support/mkquota.c b/lib/support/mkquota.c > index c5dd140..a432d4e 100644 > --- a/lib/support/mkquota.c > +++ b/lib/support/mkquota.c > @@ -448,7 +448,8 @@ errcode_t quota_compute_usage(quota_ctx_t qctx) > ext2_filsys fs; > ext2_ino_t ino; > errcode_t ret; > - struct ext2_inode inode; > + struct ext2_inode *inode; > + int inode_size; > qsize_t space; > ext2_inode_scan scan; > > @@ -461,27 +462,31 @@ errcode_t quota_compute_usage(quota_ctx_t qctx) > log_err("while opening inode scan. ret=%ld", ret); > return ret; > } > - > + inode_size = fs->super->s_inode_size; > + inode = malloc(inode_size); > + if (!inode) > + return ENOMEM; > while (1) { > - ret = ext2fs_get_next_inode(scan, &ino, &inode); > + ret = ext2fs_get_next_inode_full(scan, &ino, inode, inode_size); > if (ret) { > log_err("while getting next inode. ret=%ld", ret); > ext2fs_close_inode_scan(scan); > + free(inode); > return ret; > } > if (ino == 0) > break; > - if (inode.i_links_count && > + if (inode->i_links_count && > (ino == EXT2_ROOT_INO || > ino >= EXT2_FIRST_INODE(fs->super))) { > - space = ext2fs_inode_i_blocks(fs, &inode) << 9; > - quota_data_add(qctx, &inode, ino, space); > - quota_data_inodes(qctx, &inode, ino, +1); > + space = ext2fs_inode_i_blocks(fs, inode) << 9; > + quota_data_add(qctx, inode, ino, space); > + quota_data_inodes(qctx, inode, ino, +1); > } > } > > ext2fs_close_inode_scan(scan); > - > + free(inode); > return 0; > } > > -- > 2.5.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, May 22, 2016 at 01:34:59PM +0800, Wang Shilong wrote: > On Sun, May 22, 2016 at 10:19 AM, Theodore Ts'o <tytso@mit.edu> wrote: > > Creating a file system with project quotas can fail if mke2fs is built > > using hardening options. This is because quota_compute_usage() used > > ext2fs_get_next_inode() instead of ext2fs_get_inode_full(), and a > > small inode was passed into quota_data_add, when a large inode needs > > to be used. As a result get_dq() would end up dereferencing undefined > > space in the stack. Without the hardening options, this would be > > zero, so "mke2fs -t ext4 -O project.quota -I 256 test.img" would work > > essentially by accident. > > > > Fix this by using ext2fs_get_inode_full() so that a large inode is > > available to quota_data_inodes(). > > > > Signed-off-by: Theodore Ts'o <tytso@mit.edu> > > I thought Li Xi sent updated e2fsprogs including this fixing parts. > maybe because you merged early version patches. There was a separate patch that broke ABI backwards compatibility of e2fsprogs' shared libraries, which I rejected on those grounds, perhaps that's what you thinking of? It wasn't clear that the patch was in fact fixing a problem, as opposed to just being a clean up. So I didn't realize there was a problem that needed fixing. - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/lib/support/mkquota.c b/lib/support/mkquota.c index c5dd140..a432d4e 100644 --- a/lib/support/mkquota.c +++ b/lib/support/mkquota.c @@ -448,7 +448,8 @@ errcode_t quota_compute_usage(quota_ctx_t qctx) ext2_filsys fs; ext2_ino_t ino; errcode_t ret; - struct ext2_inode inode; + struct ext2_inode *inode; + int inode_size; qsize_t space; ext2_inode_scan scan; @@ -461,27 +462,31 @@ errcode_t quota_compute_usage(quota_ctx_t qctx) log_err("while opening inode scan. ret=%ld", ret); return ret; } - + inode_size = fs->super->s_inode_size; + inode = malloc(inode_size); + if (!inode) + return ENOMEM; while (1) { - ret = ext2fs_get_next_inode(scan, &ino, &inode); + ret = ext2fs_get_next_inode_full(scan, &ino, inode, inode_size); if (ret) { log_err("while getting next inode. ret=%ld", ret); ext2fs_close_inode_scan(scan); + free(inode); return ret; } if (ino == 0) break; - if (inode.i_links_count && + if (inode->i_links_count && (ino == EXT2_ROOT_INO || ino >= EXT2_FIRST_INODE(fs->super))) { - space = ext2fs_inode_i_blocks(fs, &inode) << 9; - quota_data_add(qctx, &inode, ino, space); - quota_data_inodes(qctx, &inode, ino, +1); + space = ext2fs_inode_i_blocks(fs, inode) << 9; + quota_data_add(qctx, inode, ino, space); + quota_data_inodes(qctx, inode, ino, +1); } } ext2fs_close_inode_scan(scan); - + free(inode); return 0; }
Creating a file system with project quotas can fail if mke2fs is built using hardening options. This is because quota_compute_usage() used ext2fs_get_next_inode() instead of ext2fs_get_inode_full(), and a small inode was passed into quota_data_add, when a large inode needs to be used. As a result get_dq() would end up dereferencing undefined space in the stack. Without the hardening options, this would be zero, so "mke2fs -t ext4 -O project.quota -I 256 test.img" would work essentially by accident. Fix this by using ext2fs_get_inode_full() so that a large inode is available to quota_data_inodes(). Signed-off-by: Theodore Ts'o <tytso@mit.edu> --- lib/support/mkquota.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)