* [PATCH 1/2] ext4: cleanup GFP flags inside resize path
@ 2014-11-09 16:37 Dmitry Monakhov
2014-11-09 16:37 ` [PATCH 2/2] ext4: fix potential use after free issue while fsresize Dmitry Monakhov
2014-11-14 9:37 ` [PATCH 1/2] ext4: cleanup GFP flags inside resize path Dmitry Monakhov
0 siblings, 2 replies; 3+ messages in thread
From: Dmitry Monakhov @ 2014-11-09 16:37 UTC (permalink / raw)
To: linux-ext4; +Cc: tytso, cmm, Dmitry Monakhov
We must use GFP_NOFS instead GFP_KERNEL inside ext4_mb_add_groupinfo because resizing call
it inside journal transaction. Call trace:
ioctl
->ext4_group_add
->journal_start
->ext4_setup_new_descs
->ext4_mb_add_groupinfo
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
fs/ext4/mballoc.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index dbfe15c..654e70d 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2385,7 +2385,7 @@ int ext4_mb_add_groupinfo(struct super_block *sb, ext4_group_t group,
if (group % EXT4_DESC_PER_BLOCK(sb) == 0) {
metalen = sizeof(*meta_group_info) <<
EXT4_DESC_PER_BLOCK_BITS(sb);
- meta_group_info = kmalloc(metalen, GFP_KERNEL);
+ meta_group_info = kmalloc(metalen, GFP_NOFS);
if (meta_group_info == NULL) {
ext4_msg(sb, KERN_ERR, "can't allocate mem "
"for a buddy group");
@@ -2399,7 +2399,7 @@ int ext4_mb_add_groupinfo(struct super_block *sb, ext4_group_t group,
sbi->s_group_info[group >> EXT4_DESC_PER_BLOCK_BITS(sb)];
i = group & (EXT4_DESC_PER_BLOCK(sb) - 1);
- meta_group_info[i] = kmem_cache_zalloc(cachep, GFP_KERNEL);
+ meta_group_info[i] = kmem_cache_zalloc(cachep, GFP_NOFS);
if (meta_group_info[i] == NULL) {
ext4_msg(sb, KERN_ERR, "can't allocate buddy mem");
goto exit_group_info;
@@ -2428,7 +2428,7 @@ int ext4_mb_add_groupinfo(struct super_block *sb, ext4_group_t group,
{
struct buffer_head *bh;
meta_group_info[i]->bb_bitmap =
- kmalloc(sb->s_blocksize, GFP_KERNEL);
+ kmalloc(sb->s_blocksize, GFP_NOFS);
BUG_ON(meta_group_info[i]->bb_bitmap == NULL);
bh = ext4_read_block_bitmap(sb, group);
BUG_ON(bh == NULL);
--
1.7.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 2/2] ext4: fix potential use after free issue while fsresize
2014-11-09 16:37 [PATCH 1/2] ext4: cleanup GFP flags inside resize path Dmitry Monakhov
@ 2014-11-09 16:37 ` Dmitry Monakhov
2014-11-14 9:37 ` [PATCH 1/2] ext4: cleanup GFP flags inside resize path Dmitry Monakhov
1 sibling, 0 replies; 3+ messages in thread
From: Dmitry Monakhov @ 2014-11-09 16:37 UTC (permalink / raw)
To: linux-ext4; +Cc: tytso, cmm, Dmitry Monakhov
We need some sort of synchronization while updating ->s_group_desc because there
are a lot of users which can access old ->s_group_desc array after it was released.
It is reasonable to use lightweight seqcount_t here instead of RCU.
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
fs/ext4/balloc.c | 14 ++++++++++----
fs/ext4/ext4.h | 1 +
fs/ext4/resize.c | 5 ++++-
fs/ext4/super.c | 1 +
4 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index 83a6f49..4746907 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -280,8 +280,10 @@ struct ext4_group_desc * ext4_get_group_desc(struct super_block *sb,
{
unsigned int group_desc;
unsigned int offset;
+ unsigned int seq;
ext4_group_t ngroups = ext4_get_groups_count(sb);
struct ext4_group_desc *desc;
+ struct buffer_head *gd_bh;
struct ext4_sb_info *sbi = EXT4_SB(sb);
if (block_group >= ngroups) {
@@ -293,7 +295,11 @@ struct ext4_group_desc * ext4_get_group_desc(struct super_block *sb,
group_desc = block_group >> EXT4_DESC_PER_BLOCK_BITS(sb);
offset = block_group & (EXT4_DESC_PER_BLOCK(sb) - 1);
- if (!sbi->s_group_desc[group_desc]) {
+ do {
+ seq = read_seqcount_begin(&sbi->s_group_desc_seq);
+ gd_bh = sbi->s_group_desc[group_desc];
+ } while (unlikely(read_seqcount_retry(&sbi->s_group_desc_seq, seq)));
+ if (!gd_bh) {
ext4_error(sb, "Group descriptor not loaded - "
"block_group = %u, group_desc = %u, desc = %u",
block_group, group_desc, offset);
@@ -301,10 +307,10 @@ struct ext4_group_desc * ext4_get_group_desc(struct super_block *sb,
}
desc = (struct ext4_group_desc *)(
- (__u8 *)sbi->s_group_desc[group_desc]->b_data +
- offset * EXT4_DESC_SIZE(sb));
+ (__u8 *)gd_bh->b_data + offset * EXT4_DESC_SIZE(sb));
if (bh)
- *bh = sbi->s_group_desc[group_desc];
+ *bh = gd_bh;
+
return desc;
}
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index c55a1fa..2c039d2 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1199,6 +1199,7 @@ struct ext4_sb_info {
struct buffer_head * s_sbh; /* Buffer containing the super block */
struct ext4_super_block *s_es; /* Pointer to the super block in the buffer */
struct buffer_head **s_group_desc;
+ seqcount_t s_group_desc_seq; /* protects s_group_desc updates */
unsigned int s_mount_opt;
unsigned int s_mount_opt2;
unsigned int s_mount_flags;
diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index ca45883..f9ec935 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -854,8 +854,10 @@ static int add_new_gdb(handle_t *handle, struct inode *inode,
memcpy(n_group_desc, o_group_desc,
EXT4_SB(sb)->s_gdb_count * sizeof(struct buffer_head *));
n_group_desc[gdb_num] = gdb_bh;
+ write_seqcount_begin(&EXT4_SB(sb)->s_group_desc_seq);
EXT4_SB(sb)->s_group_desc = n_group_desc;
EXT4_SB(sb)->s_gdb_count++;
+ write_seqcount_end(&EXT4_SB(sb)->s_group_desc_seq);
ext4_kvfree(o_group_desc);
le16_add_cpu(&es->s_reserved_gdt_blocks, -1);
@@ -902,13 +904,14 @@ static int add_new_gdb_meta_bg(struct super_block *sb,
gdb_num + 1);
return err;
}
-
o_group_desc = EXT4_SB(sb)->s_group_desc;
memcpy(n_group_desc, o_group_desc,
EXT4_SB(sb)->s_gdb_count * sizeof(struct buffer_head *));
n_group_desc[gdb_num] = gdb_bh;
+ write_seqcount_begin(&EXT4_SB(sb)->s_group_desc_seq);
EXT4_SB(sb)->s_group_desc = n_group_desc;
EXT4_SB(sb)->s_gdb_count++;
+ write_seqcount_end(&EXT4_SB(sb)->s_group_desc_seq);
ext4_kvfree(o_group_desc);
BUFFER_TRACE(gdb_bh, "get_write_access");
err = ext4_journal_get_write_access(handle, gdb_bh);
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 2c9e686..08d50a1 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3872,6 +3872,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
(EXT4_MAX_BLOCK_FILE_PHYS / EXT4_BLOCKS_PER_GROUP(sb)));
db_count = (sbi->s_groups_count + EXT4_DESC_PER_BLOCK(sb) - 1) /
EXT4_DESC_PER_BLOCK(sb);
+ seqcount_init(&sbi->s_group_desc_seq);
sbi->s_group_desc = ext4_kvmalloc(db_count *
sizeof(struct buffer_head *),
GFP_KERNEL);
--
1.7.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/2] ext4: cleanup GFP flags inside resize path
2014-11-09 16:37 [PATCH 1/2] ext4: cleanup GFP flags inside resize path Dmitry Monakhov
2014-11-09 16:37 ` [PATCH 2/2] ext4: fix potential use after free issue while fsresize Dmitry Monakhov
@ 2014-11-14 9:37 ` Dmitry Monakhov
1 sibling, 0 replies; 3+ messages in thread
From: Dmitry Monakhov @ 2014-11-14 9:37 UTC (permalink / raw)
To: linux-ext4; +Cc: tytso, cmm
Dmitry Monakhov <dmonakhov@openvz.org> writes:
> We must use GFP_NOFS instead GFP_KERNEL inside ext4_mb_add_groupinfo because resizing call
> it inside journal transaction. Call trace:
> ioctl
> ->ext4_group_add
> ->journal_start
> ->ext4_setup_new_descs
> ->ext4_mb_add_groupinfo
It is appeared that there is one more case missed.
ext4_group_add
->journal_start
->ext4_flex_group_add
->ext4_update_super
->ext4_calculate_overhead -> GFP_KERNEL
I'll send updated version soon.
>
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> ---
> fs/ext4/mballoc.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index dbfe15c..654e70d 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -2385,7 +2385,7 @@ int ext4_mb_add_groupinfo(struct super_block *sb, ext4_group_t group,
> if (group % EXT4_DESC_PER_BLOCK(sb) == 0) {
> metalen = sizeof(*meta_group_info) <<
> EXT4_DESC_PER_BLOCK_BITS(sb);
> - meta_group_info = kmalloc(metalen, GFP_KERNEL);
> + meta_group_info = kmalloc(metalen, GFP_NOFS);
> if (meta_group_info == NULL) {
> ext4_msg(sb, KERN_ERR, "can't allocate mem "
> "for a buddy group");
> @@ -2399,7 +2399,7 @@ int ext4_mb_add_groupinfo(struct super_block *sb, ext4_group_t group,
> sbi->s_group_info[group >> EXT4_DESC_PER_BLOCK_BITS(sb)];
> i = group & (EXT4_DESC_PER_BLOCK(sb) - 1);
>
> - meta_group_info[i] = kmem_cache_zalloc(cachep, GFP_KERNEL);
> + meta_group_info[i] = kmem_cache_zalloc(cachep, GFP_NOFS);
> if (meta_group_info[i] == NULL) {
> ext4_msg(sb, KERN_ERR, "can't allocate buddy mem");
> goto exit_group_info;
> @@ -2428,7 +2428,7 @@ int ext4_mb_add_groupinfo(struct super_block *sb, ext4_group_t group,
> {
> struct buffer_head *bh;
> meta_group_info[i]->bb_bitmap =
> - kmalloc(sb->s_blocksize, GFP_KERNEL);
> + kmalloc(sb->s_blocksize, GFP_NOFS);
> BUG_ON(meta_group_info[i]->bb_bitmap == NULL);
> bh = ext4_read_block_bitmap(sb, group);
> BUG_ON(bh == NULL);
> --
> 1.7.1
>
> --
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-11-14 9:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-09 16:37 [PATCH 1/2] ext4: cleanup GFP flags inside resize path Dmitry Monakhov
2014-11-09 16:37 ` [PATCH 2/2] ext4: fix potential use after free issue while fsresize Dmitry Monakhov
2014-11-14 9:37 ` [PATCH 1/2] ext4: cleanup GFP flags inside resize path Dmitry Monakhov
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.