linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 01/11] ext4 resise: extra brelse in setup_new_flex_group_blocks()
       [not found] <cover.1540935522.git.vvs@virtuozzo.com>
@ 2018-10-30 21:57 ` Vasily Averin
  2018-11-03 20:20   ` Theodore Y. Ts'o
  2018-10-30 21:57 ` [PATCH v2 02/11] ext4 resize: missing brelse() after errors in set_flexbg_block_bitmap() Vasily Averin
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Vasily Averin @ 2018-10-30 21:57 UTC (permalink / raw)
  To: linux-ext4, Theodore Ts'o
  Cc: Andreas Dilger, linux-kernel, Yongqiang Yang, Yongqiang Yang

currently bh is set to NULL only during first iteration of for cycle,
then this pointer is not cleared after end of using.
Therefore rollback after errors can lead to extra brelse(bh) call,
decrements bh counter and later trigger an unexpected warning in __brelse()

Patch moves brelse() calls in body of cycle to exclude requirement of
brelse() call in rollback.

Fixes 33afdcc5402d ("ext4: add a function which sets up group blocks ...") # 3.3+

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 fs/ext4/resize.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index ebbc663d0798..c3fa30878ca8 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -605,7 +605,6 @@ static int setup_new_flex_group_blocks(struct super_block *sb,
 		bh = bclean(handle, sb, block);
 		if (IS_ERR(bh)) {
 			err = PTR_ERR(bh);
-			bh = NULL;
 			goto out;
 		}
 		overhead = ext4_group_overhead_blocks(sb, group);
@@ -618,9 +617,9 @@ static int setup_new_flex_group_blocks(struct super_block *sb,
 		ext4_mark_bitmap_end(EXT4_B2C(sbi, group_data[i].blocks_count),
 				     sb->s_blocksize * 8, bh->b_data);
 		err = ext4_handle_dirty_metadata(handle, NULL, bh);
+		brelse(bh);
 		if (err)
 			goto out;
-		brelse(bh);
 
 handle_ib:
 		if (bg_flags[i] & EXT4_BG_INODE_UNINIT)
@@ -635,18 +634,16 @@ static int setup_new_flex_group_blocks(struct super_block *sb,
 		bh = bclean(handle, sb, block);
 		if (IS_ERR(bh)) {
 			err = PTR_ERR(bh);
-			bh = NULL;
 			goto out;
 		}
 
 		ext4_mark_bitmap_end(EXT4_INODES_PER_GROUP(sb),
 				     sb->s_blocksize * 8, bh->b_data);
 		err = ext4_handle_dirty_metadata(handle, NULL, bh);
+		brelse(bh);
 		if (err)
 			goto out;
-		brelse(bh);
 	}
-	bh = NULL;
 
 	/* Mark group tables in block bitmap */
 	for (j = 0; j < GROUP_TABLE_COUNT; j++) {
@@ -685,7 +682,6 @@ static int setup_new_flex_group_blocks(struct super_block *sb,
 	}
 
 out:
-	brelse(bh);
 	err2 = ext4_journal_stop(handle);
 	if (err2 && !err)
 		err = err2;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 02/11] ext4 resize: missing brelse() after errors in set_flexbg_block_bitmap()
       [not found] <cover.1540935522.git.vvs@virtuozzo.com>
  2018-10-30 21:57 ` [PATCH v2 01/11] ext4 resise: extra brelse in setup_new_flex_group_blocks() Vasily Averin
@ 2018-10-30 21:57 ` Vasily Averin
  2018-11-03 20:28   ` Theodore Y. Ts'o
  2018-10-30 21:57 ` [PATCH v2 03/11] ext4 resize: brelse() cleanup in add_new_gdb_meta_bg() Vasily Averin
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Vasily Averin @ 2018-10-30 21:57 UTC (permalink / raw)
  To: linux-ext4, Theodore Ts'o
  Cc: Andreas Dilger, linux-kernel, Yongqiang Yang, Yongqiang Yang

Fixes 33afdcc5402d ("ext4: add a function which sets up group blocks ...") # 3.3

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 fs/ext4/resize.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index c3fa30878ca8..0a4dc6217e78 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -459,16 +459,18 @@ static int set_flexbg_block_bitmap(struct super_block *sb, handle_t *handle,
 
 		BUFFER_TRACE(bh, "get_write_access");
 		err = ext4_journal_get_write_access(handle, bh);
-		if (err)
+		if (err) {
+			brelse(bh);
 			return err;
+		}
 		ext4_debug("mark block bitmap %#04llx (+%llu/%u)\n",
 			   first_cluster, first_cluster - start, count2);
 		ext4_set_bits(bh->b_data, first_cluster - start, count2);
 
 		err = ext4_handle_dirty_metadata(handle, NULL, bh);
+		brelse(bh);
 		if (unlikely(err))
 			return err;
-		brelse(bh);
 	}
 
 	return 0;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 03/11] ext4 resize: brelse() cleanup in add_new_gdb_meta_bg()
       [not found] <cover.1540935522.git.vvs@virtuozzo.com>
  2018-10-30 21:57 ` [PATCH v2 01/11] ext4 resise: extra brelse in setup_new_flex_group_blocks() Vasily Averin
  2018-10-30 21:57 ` [PATCH v2 02/11] ext4 resize: missing brelse() after errors in set_flexbg_block_bitmap() Vasily Averin
@ 2018-10-30 21:57 ` Vasily Averin
  2018-11-03 20:55   ` Theodore Y. Ts'o
  2018-10-30 21:58 ` [PATCH v2 04/11] ext4 resize: lost brelse() in update_backups() Vasily Averin
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Vasily Averin @ 2018-10-30 21:57 UTC (permalink / raw)
  To: linux-ext4, Theodore Ts'o
  Cc: Andreas Dilger, linux-kernel, Yongqiang Yang, Yongqiang Yang

gdb_bh must be released in case of errors before update of s_group_desc
but it must not be released after update of group descriptors
because in this case bh can be used later.

Fixes 01f795f9e0d6 ("ext4: add online resizing support for meta_bg ...") # 3.7

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 fs/ext4/resize.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 0a4dc6217e78..7131f35b62d9 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -922,6 +922,7 @@ static int add_new_gdb_meta_bg(struct super_block *sb,
 				     sizeof(struct buffer_head *),
 				     GFP_NOFS);
 	if (!n_group_desc) {
+		brelse(gdb_bh);
 		err = -ENOMEM;
 		ext4_warning(sb, "not enough memory for %lu groups",
 			     gdb_num + 1);
@@ -937,8 +938,6 @@ static int add_new_gdb_meta_bg(struct super_block *sb,
 	kvfree(o_group_desc);
 	BUFFER_TRACE(gdb_bh, "get_write_access");
 	err = ext4_journal_get_write_access(handle, gdb_bh);
-	if (unlikely(err))
-		brelse(gdb_bh);
 	return err;
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 04/11] ext4 resize: lost brelse() in update_backups()
       [not found] <cover.1540935522.git.vvs@virtuozzo.com>
                   ` (2 preceding siblings ...)
  2018-10-30 21:57 ` [PATCH v2 03/11] ext4 resize: brelse() cleanup in add_new_gdb_meta_bg() Vasily Averin
@ 2018-10-30 21:58 ` Vasily Averin
  2018-11-03 21:14   ` Theodore Y. Ts'o
  2018-10-30 21:58 ` [PATCH v2 05/11] ext4 resize: lost rollback in ext4_resize_fs() Vasily Averin
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Vasily Averin @ 2018-10-30 21:58 UTC (permalink / raw)
  To: linux-ext4, Theodore Ts'o; +Cc: Andreas Dilger, linux-kernel

bh was not released after error in ext4_journal_get_write_access()

Fixes ac27a0ec112a ("ext4: initial copy of files from ext3") # 2.6.19

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 fs/ext4/resize.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 7131f35b62d9..3df326ee6d50 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -1121,8 +1121,10 @@ static void update_backups(struct super_block *sb, sector_t blk_off, char *data,
 			   backup_block, backup_block -
 			   ext4_group_first_block_no(sb, group));
 		BUFFER_TRACE(bh, "get_write_access");
-		if ((err = ext4_journal_get_write_access(handle, bh)))
+		if ((err = ext4_journal_get_write_access(handle, bh))) {
+			brelse(bh);
 			break;
+		}
 		lock_buffer(bh);
 		memcpy(bh->b_data, data, size);
 		if (rest)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 05/11] ext4 resize: lost rollback in ext4_resize_fs()
       [not found] <cover.1540935522.git.vvs@virtuozzo.com>
                   ` (3 preceding siblings ...)
  2018-10-30 21:58 ` [PATCH v2 04/11] ext4 resize: lost brelse() in update_backups() Vasily Averin
@ 2018-10-30 21:58 ` Vasily Averin
  2018-11-06 21:17   ` Theodore Y. Ts'o
  2018-10-30 21:58 ` [PATCH v2 06/11] ext4 resize: lost resize_inode cleanup before retry " Vasily Averin
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Vasily Averin @ 2018-10-30 21:58 UTC (permalink / raw)
  To: linux-ext4, Theodore Ts'o; +Cc: Andreas Dilger, linux-kernel

Fixes 117fff10d7f1 ("ext4: grow the s_flex_groups array as needed ...") # 3.7

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 fs/ext4/resize.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 3df326ee6d50..5fee65afd58b 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -2022,7 +2022,7 @@ int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count)
 
 	err = ext4_alloc_flex_bg_array(sb, n_group + 1);
 	if (err)
-		return err;
+		goto out;
 
 	err = ext4_mb_alloc_groupinfo(sb, n_group + 1);
 	if (err)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 06/11] ext4 resize: lost resize_inode cleanup before retry in ext4_resize_fs()
       [not found] <cover.1540935522.git.vvs@virtuozzo.com>
                   ` (4 preceding siblings ...)
  2018-10-30 21:58 ` [PATCH v2 05/11] ext4 resize: lost rollback in ext4_resize_fs() Vasily Averin
@ 2018-10-30 21:58 ` Vasily Averin
  2018-11-06 21:23   ` Theodore Y. Ts'o
  2018-10-30 21:58 ` [PATCH v2 07/11] ext4: lost put_bh in ext4_mark_iloc_dirty() Vasily Averin
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Vasily Averin @ 2018-10-30 21:58 UTC (permalink / raw)
  To: linux-ext4, Theodore Ts'o; +Cc: Andreas Dilger, linux-kernel

Fixes 1c6bd7173d66 ("ext4: convert file system to meta_bg if needed ...") # 3.7

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 fs/ext4/resize.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 5fee65afd58b..85158e9de7c2 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -2058,6 +2058,10 @@ int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count)
 		n_blocks_count_retry = 0;
 		free_flex_gd(flex_gd);
 		flex_gd = NULL;
+		if (resize_inode) {
+			iput(resize_inode);
+			resize_inode = NULL;
+		}
 		goto retry;
 	}
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 07/11] ext4: lost put_bh in ext4_mark_iloc_dirty()
       [not found] <cover.1540935522.git.vvs@virtuozzo.com>
                   ` (5 preceding siblings ...)
  2018-10-30 21:58 ` [PATCH v2 06/11] ext4 resize: lost resize_inode cleanup before retry " Vasily Averin
@ 2018-10-30 21:58 ` Vasily Averin
  2018-11-06 21:53   ` Theodore Y. Ts'o
  2018-10-30 21:58 ` [PATCH v2 08/11] ext4: lost brelse in ext4_orphan_add() Vasily Averin
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Vasily Averin @ 2018-10-30 21:58 UTC (permalink / raw)
  To: linux-ext4, Theodore Ts'o; +Cc: Andreas Dilger, linux-kernel

ext4_mark_iloc_dirty() callers expect that it releases iloc->bh
even if it returns an error.

Fixes 0db1ff222d40 ("ext4: add shutdown bit and check for it") # 4.11

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 fs/ext4/inode.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index c3d9a42c561e..55c8fca76daf 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5835,9 +5835,10 @@ int ext4_mark_iloc_dirty(handle_t *handle,
 {
 	int err = 0;
 
-	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
+	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb)))) {
+		put_bh(iloc->bh);
 		return -EIO;
-
+	}
 	if (IS_I_VERSION(inode))
 		inode_inc_iversion(inode);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 08/11] ext4: lost brelse in ext4_orphan_add()
       [not found] <cover.1540935522.git.vvs@virtuozzo.com>
                   ` (6 preceding siblings ...)
  2018-10-30 21:58 ` [PATCH v2 07/11] ext4: lost put_bh in ext4_mark_iloc_dirty() Vasily Averin
@ 2018-10-30 21:58 ` Vasily Averin
  2018-11-06 22:04   ` Theodore Y. Ts'o
  2018-10-30 21:58 ` [PATCH v2 09/11] ext4: iloc.bh cleanup in add_new_gdb() Vasily Averin
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Vasily Averin @ 2018-10-30 21:58 UTC (permalink / raw)
  To: linux-ext4, Theodore Ts'o
  Cc: Andreas Dilger, linux-kernel, Dmitry Monakhov

iloc.bh os taken in ext4_reserve_inode_write().
If (dirty == true) it is released in ext4_mark_iloc_dirty(),
however it is not released in (dirty == false) case.

Fixes d745a8c20c1f ("ext4: reduce contention on s_orphan_lock")
however iloc.bh count balance was broken earlier by
Fixes 6e3617e579e0 ("ext4: Handle non empty on-disk orphan link") #2.6.34

cc: Dmitry Monakhov <dmonakhov@gmail.com>
Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 fs/ext4/namei.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 67a38532032a..d388cce72db2 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -2811,7 +2811,9 @@ int ext4_orphan_add(handle_t *handle, struct inode *inode)
 			list_del_init(&EXT4_I(inode)->i_orphan);
 			mutex_unlock(&sbi->s_orphan_lock);
 		}
-	}
+	} else
+		brelse(iloc.bh);
+
 	jbd_debug(4, "superblock will point to %lu\n", inode->i_ino);
 	jbd_debug(4, "orphan inode %lu will point to %d\n",
 			inode->i_ino, NEXT_ORPHAN(inode));
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 09/11] ext4: iloc.bh cleanup in add_new_gdb()
       [not found] <cover.1540935522.git.vvs@virtuozzo.com>
                   ` (7 preceding siblings ...)
  2018-10-30 21:58 ` [PATCH v2 08/11] ext4: lost brelse in ext4_orphan_add() Vasily Averin
@ 2018-10-30 21:58 ` Vasily Averin
  2018-11-06 22:19   ` Theodore Y. Ts'o
  2018-10-30 21:58 ` [PATCH v2 10/11] ext4: remove useless brelse call in ext4_xattr_inode_update_ref() Vasily Averin
  2018-10-30 21:58 ` [PATCH v2 11/11] ext4: access to uninitialized bh fields in ext4_xattr_set_handle() Vasily Averin
  10 siblings, 1 reply; 23+ messages in thread
From: Vasily Averin @ 2018-10-30 21:58 UTC (permalink / raw)
  To: linux-ext4, Theodore Ts'o; +Cc: Andreas Dilger, linux-kernel

iloc.bh was taken in ext4_reserve_inode_write() and released
in ext4_mark_iloc_dirty(). It should not be released 2nd time
in rollback after failed ext4_handle_dirty_metadata(gdb_bh)

Fixes b40971426a83 ("ext4: add error checking to calls to ...") # 2.6.38

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 fs/ext4/resize.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 85158e9de7c2..aedfd6a6fcd1 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -871,7 +871,7 @@ static int add_new_gdb(handle_t *handle, struct inode *inode,
 	err = ext4_handle_dirty_metadata(handle, NULL, gdb_bh);
 	if (unlikely(err)) {
 		ext4_std_error(sb, err);
-		goto exit_inode;
+		goto exit_kfree;
 	}
 	brelse(dind);
 
@@ -891,8 +891,9 @@ static int add_new_gdb(handle_t *handle, struct inode *inode,
 	return err;
 
 exit_inode:
-	kvfree(n_group_desc);
 	brelse(iloc.bh);
+exit_kfree:
+	kvfree(n_group_desc);
 exit_dind:
 	brelse(dind);
 exit_bh:
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 10/11] ext4: remove useless brelse call in ext4_xattr_inode_update_ref()
       [not found] <cover.1540935522.git.vvs@virtuozzo.com>
                   ` (8 preceding siblings ...)
  2018-10-30 21:58 ` [PATCH v2 09/11] ext4: iloc.bh cleanup in add_new_gdb() Vasily Averin
@ 2018-10-30 21:58 ` Vasily Averin
  2018-11-06 22:49   ` Theodore Y. Ts'o
  2018-10-30 21:58 ` [PATCH v2 11/11] ext4: access to uninitialized bh fields in ext4_xattr_set_handle() Vasily Averin
  10 siblings, 1 reply; 23+ messages in thread
From: Vasily Averin @ 2018-10-30 21:58 UTC (permalink / raw)
  To: linux-ext4, Theodore Ts'o
  Cc: Andreas Dilger, linux-kernel, Tahsin Erdogan

brelse(iloc.bh) is useless here, it is always called with iloc.bh = NULL

Fixes dec214d00e0d ("ext4: xattr inode deduplication") # 4.13
cc: Tahsin Erdogan <tahsin@google.com>

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 fs/ext4/xattr.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index f36fc5d5b257..dc1aeab06dba 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1031,10 +1031,8 @@ static int ext4_xattr_inode_update_ref(handle_t *handle, struct inode *ea_inode,
 	inode_lock(ea_inode);
 
 	ret = ext4_reserve_inode_write(handle, ea_inode, &iloc);
-	if (ret) {
-		iloc.bh = NULL;
+	if (ret)
 		goto out;
-	}
 
 	ref_count = ext4_xattr_inode_get_ref(ea_inode);
 	ref_count += ref_change;
@@ -1080,12 +1078,10 @@ static int ext4_xattr_inode_update_ref(handle_t *handle, struct inode *ea_inode,
 	}
 
 	ret = ext4_mark_iloc_dirty(handle, ea_inode, &iloc);
-	iloc.bh = NULL;
 	if (ret)
 		ext4_warning_inode(ea_inode,
 				   "ext4_mark_iloc_dirty() failed ret=%d", ret);
 out:
-	brelse(iloc.bh);
 	inode_unlock(ea_inode);
 	return ret;
 }
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 11/11] ext4: access to uninitialized bh fields in ext4_xattr_set_handle()
       [not found] <cover.1540935522.git.vvs@virtuozzo.com>
                   ` (9 preceding siblings ...)
  2018-10-30 21:58 ` [PATCH v2 10/11] ext4: remove useless brelse call in ext4_xattr_inode_update_ref() Vasily Averin
@ 2018-10-30 21:58 ` Vasily Averin
       [not found]   ` <48BA929C-9BF7-4092-BDD3-BECB64AF0599@dilger.ca>
  10 siblings, 1 reply; 23+ messages in thread
From: Vasily Averin @ 2018-10-30 21:58 UTC (permalink / raw)
  To: linux-ext4, Theodore Ts'o; +Cc: Andreas Dilger, linux-kernel

On-stack initialization does not guarantee zeroying of unintialized
fields. So is.iloc.bh and bs.bh can be contain garbage of old stack
conent.

Errors in the beginning of ext4_xattr_set_handle() function
lead to jump to "cleanup:" label where brelse(is.iloc.bh)
and brelse(bs.bh) can access uninitialized bh fields of
on-stack located "is" and "bs" structures.

Issue was inherited from ext3 and was present in first ext4 commit.

Fixes ac27a0ec112a ("ext4: initial copy of files from ext3") # 2.6.19

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 fs/ext4/xattr.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index dc1aeab06dba..aae12425597e 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -2303,9 +2303,11 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
 	};
 	struct ext4_xattr_ibody_find is = {
 		.s = { .not_found = -ENODATA, },
+		.iloc = { .bh = NULL, },
 	};
 	struct ext4_xattr_block_find bs = {
 		.s = { .not_found = -ENODATA, },
+		.bh = NULL,
 	};
 	int no_expand;
 	int error;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 11/11] ext4: access to uninitialized bh fields in ext4_xattr_set_handle()
       [not found]   ` <48BA929C-9BF7-4092-BDD3-BECB64AF0599@dilger.ca>
@ 2018-10-31  3:39     ` Vasily Averin
  2018-11-01  0:42       ` Andreas Dilger
  0 siblings, 1 reply; 23+ messages in thread
From: Vasily Averin @ 2018-10-31  3:39 UTC (permalink / raw)
  To: Andreas Dilger
  Cc: linux-ext4, Theodore Ts'o, Andreas Dilger, linux-kernel

On 10/31/2018 04:30 AM, Andreas Dilger wrote:
> Could you please explain your statement below that on-stack
> initialization does not zero unspecified fields?  According 
> to documents I found, for example:
> 
> https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html
> 
> they *are* initialized to zero. 

I did not know it,
I re-checked it in generated assembler code and found that you are right and I was wrong.

Please drop this patch,
should I resend of rest of this patch set once again?

Thank you,
	Vasily Averin

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 11/11] ext4: access to uninitialized bh fields in ext4_xattr_set_handle()
  2018-10-31  3:39     ` Vasily Averin
@ 2018-11-01  0:42       ` Andreas Dilger
  0 siblings, 0 replies; 23+ messages in thread
From: Andreas Dilger @ 2018-11-01  0:42 UTC (permalink / raw)
  To: Vasily Averin; +Cc: Ext4 Developers List, Theodore Ts'o, LKML

[-- Attachment #1: Type: text/plain, Size: 741 bytes --]

On Oct 30, 2018, at 9:39 PM, Vasily Averin <vvs@virtuozzo.com> wrote:
> 
> On 10/31/2018 04:30 AM, Andreas Dilger wrote:
>> Could you please explain your statement below that on-stack
>> initialization does not zero unspecified fields?  According
>> to documents I found, for example:
>> 
>> https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html
>> 
>> they *are* initialized to zero.
> 
> I did not know it,
> I re-checked it in generated assembler code and found that you
> are right and I was wrong.
> 
> Please drop this patch,
> should I resend of rest of this patch set once again?

I don't think it is necessary to resend the whole patch series.
Ted should notice these emails on this patch and not apply it.

Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 01/11] ext4 resise: extra brelse in setup_new_flex_group_blocks()
  2018-10-30 21:57 ` [PATCH v2 01/11] ext4 resise: extra brelse in setup_new_flex_group_blocks() Vasily Averin
@ 2018-11-03 20:20   ` Theodore Y. Ts'o
  0 siblings, 0 replies; 23+ messages in thread
From: Theodore Y. Ts'o @ 2018-11-03 20:20 UTC (permalink / raw)
  To: Vasily Averin
  Cc: linux-ext4, Andreas Dilger, linux-kernel, Yongqiang Yang, Yongqiang Yang

On Wed, Oct 31, 2018 at 12:57:37AM +0300, Vasily Averin wrote:
> currently bh is set to NULL only during first iteration of for cycle,
> then this pointer is not cleared after end of using.
> Therefore rollback after errors can lead to extra brelse(bh) call,
> decrements bh counter and later trigger an unexpected warning in __brelse()
> 
> Patch moves brelse() calls in body of cycle to exclude requirement of
> brelse() call in rollback.
> 
> Fixes 33afdcc5402d ("ext4: add a function which sets up group blocks ...") # 3.3+
> 
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>

Thanks, applied.  I addjusted the patch summary to read:

    ext4: avoid potential extra brelse in setup_new_flex_group_blocks()

(Note that resise should have been spelled as "resize", by the way.)


						- Ted

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 02/11] ext4 resize: missing brelse() after errors in set_flexbg_block_bitmap()
  2018-10-30 21:57 ` [PATCH v2 02/11] ext4 resize: missing brelse() after errors in set_flexbg_block_bitmap() Vasily Averin
@ 2018-11-03 20:28   ` Theodore Y. Ts'o
  0 siblings, 0 replies; 23+ messages in thread
From: Theodore Y. Ts'o @ 2018-11-03 20:28 UTC (permalink / raw)
  To: Vasily Averin
  Cc: linux-ext4, Andreas Dilger, linux-kernel, Yongqiang Yang, Yongqiang Yang

On Wed, Oct 31, 2018 at 12:57:45AM +0300, Vasily Averin wrote:
> Fixes 33afdcc5402d ("ext4: add a function which sets up group blocks ...") # 3.3
> 
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>

Thanks, applied.  I adjusted the patch summary:

    ext4: add missing brelse() in set_flexbg_block_bitmap()'s error path

Also please note that properly Fixes should have a colon, and to add
Cc: stable@kernel.org to trailer to make sure they get backported:
    
    Fixes: 33afdcc5402d ("ext4: add a function which sets up group blocks ...")
    Cc: stable@kernel.org # 3.3

						- Ted

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 03/11] ext4 resize: brelse() cleanup in add_new_gdb_meta_bg()
  2018-10-30 21:57 ` [PATCH v2 03/11] ext4 resize: brelse() cleanup in add_new_gdb_meta_bg() Vasily Averin
@ 2018-11-03 20:55   ` Theodore Y. Ts'o
  0 siblings, 0 replies; 23+ messages in thread
From: Theodore Y. Ts'o @ 2018-11-03 20:55 UTC (permalink / raw)
  To: Vasily Averin
  Cc: linux-ext4, Andreas Dilger, linux-kernel, Yongqiang Yang, Yongqiang Yang

On Wed, Oct 31, 2018 at 12:57:55AM +0300, Vasily Averin wrote:
> gdb_bh must be released in case of errors before update of s_group_desc
> but it must not be released after update of group descriptors
> because in this case bh can be used later.
> 
> Fixes 01f795f9e0d6 ("ext4: add online resizing support for meta_bg ...") # 3.7
> 
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>

Thanks, applied.  The commit description is not accurate (I suspect it
was obsoleted when your patch for RHEL was forward ported) do I
dropped it.

I adjusted the patch summary to be:

    ext4: add missing brelse() add_new_gdb_meta_bg()'s error path

The commments for the Fixes and Cc: stable@kernel.org also apply.

							- Ted
								

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 04/11] ext4 resize: lost brelse() in update_backups()
  2018-10-30 21:58 ` [PATCH v2 04/11] ext4 resize: lost brelse() in update_backups() Vasily Averin
@ 2018-11-03 21:14   ` Theodore Y. Ts'o
  0 siblings, 0 replies; 23+ messages in thread
From: Theodore Y. Ts'o @ 2018-11-03 21:14 UTC (permalink / raw)
  To: Vasily Averin; +Cc: linux-ext4, Andreas Dilger, linux-kernel

On Wed, Oct 31, 2018 at 12:58:03AM +0300, Vasily Averin wrote:
> bh was not released after error in ext4_journal_get_write_access()
> 
> Fixes ac27a0ec112a ("ext4: initial copy of files from ext3") # 2.6.19
> 
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>

Thanks, applied.  I fixed up the commit description and Fixes/Cc
trailers.

						- Ted

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 05/11] ext4 resize: lost rollback in ext4_resize_fs()
  2018-10-30 21:58 ` [PATCH v2 05/11] ext4 resize: lost rollback in ext4_resize_fs() Vasily Averin
@ 2018-11-06 21:17   ` Theodore Y. Ts'o
  0 siblings, 0 replies; 23+ messages in thread
From: Theodore Y. Ts'o @ 2018-11-06 21:17 UTC (permalink / raw)
  To: Vasily Averin; +Cc: linux-ext4, Andreas Dilger, linux-kernel

On Wed, Oct 31, 2018 at 12:58:09AM +0300, Vasily Averin wrote:
> Fixes 117fff10d7f1 ("ext4: grow the s_flex_groups array as needed ...") # 3.7
> 
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>

Thanks, applied.  I fixed up the commit description and Fixes/Cc
trailers.

					- Ted

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 06/11] ext4 resize: lost resize_inode cleanup before retry in ext4_resize_fs()
  2018-10-30 21:58 ` [PATCH v2 06/11] ext4 resize: lost resize_inode cleanup before retry " Vasily Averin
@ 2018-11-06 21:23   ` Theodore Y. Ts'o
  0 siblings, 0 replies; 23+ messages in thread
From: Theodore Y. Ts'o @ 2018-11-06 21:23 UTC (permalink / raw)
  To: Vasily Averin; +Cc: linux-ext4, Andreas Dilger, linux-kernel

On Wed, Oct 31, 2018 at 12:58:16AM +0300, Vasily Averin wrote:
> Fixes 1c6bd7173d66 ("ext4: convert file system to meta_bg if needed ...") # 3.7
> 
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>

Thanks, applied.  I fixed up the commit description and Fixes/Cc
trailers.

						- Ted

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 07/11] ext4: lost put_bh in ext4_mark_iloc_dirty()
  2018-10-30 21:58 ` [PATCH v2 07/11] ext4: lost put_bh in ext4_mark_iloc_dirty() Vasily Averin
@ 2018-11-06 21:53   ` Theodore Y. Ts'o
  0 siblings, 0 replies; 23+ messages in thread
From: Theodore Y. Ts'o @ 2018-11-06 21:53 UTC (permalink / raw)
  To: Vasily Averin; +Cc: linux-ext4, Andreas Dilger, linux-kernel

On Wed, Oct 31, 2018 at 12:58:24AM +0300, Vasily Averin wrote:
> ext4_mark_iloc_dirty() callers expect that it releases iloc->bh
> even if it returns an error.
> 
> Fixes 0db1ff222d40 ("ext4: add shutdown bit and check for it") # 4.11
> 
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>

Thanks, applied.  I fixed up the commit description and Fixes/Cc
trailers.  I used the one-line description:

    ext4: avoid buffer leak on shutdown in ext4_mark_iloc_dirty()

    	  	       	       		- Ted

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 08/11] ext4: lost brelse in ext4_orphan_add()
  2018-10-30 21:58 ` [PATCH v2 08/11] ext4: lost brelse in ext4_orphan_add() Vasily Averin
@ 2018-11-06 22:04   ` Theodore Y. Ts'o
  0 siblings, 0 replies; 23+ messages in thread
From: Theodore Y. Ts'o @ 2018-11-06 22:04 UTC (permalink / raw)
  To: Vasily Averin; +Cc: linux-ext4, Andreas Dilger, linux-kernel, Dmitry Monakhov

On Wed, Oct 31, 2018 at 12:58:31AM +0300, Vasily Averin wrote:
> iloc.bh os taken in ext4_reserve_inode_write().
> If (dirty == true) it is released in ext4_mark_iloc_dirty(),
> however it is not released in (dirty == false) case.
> 
> Fixes d745a8c20c1f ("ext4: reduce contention on s_orphan_lock")
> however iloc.bh count balance was broken earlier by
> Fixes 6e3617e579e0 ("ext4: Handle non empty on-disk orphan link") #2.6.34
> 
> cc: Dmitry Monakhov <dmonakhov@gmail.com>
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>

Thanks, applied.  I fixed up the commit description and Fixes/Cc
trailers.  I used the one-line description:

    ext4: avoid buffer leak in ext4_orphan_add() after prior errors

  	       	       			       - Ted

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 09/11] ext4: iloc.bh cleanup in add_new_gdb()
  2018-10-30 21:58 ` [PATCH v2 09/11] ext4: iloc.bh cleanup in add_new_gdb() Vasily Averin
@ 2018-11-06 22:19   ` Theodore Y. Ts'o
  0 siblings, 0 replies; 23+ messages in thread
From: Theodore Y. Ts'o @ 2018-11-06 22:19 UTC (permalink / raw)
  To: Vasily Averin; +Cc: linux-ext4, Andreas Dilger, linux-kernel

On Wed, Oct 31, 2018 at 12:58:38AM +0300, Vasily Averin wrote:
> iloc.bh was taken in ext4_reserve_inode_write() and released
> in ext4_mark_iloc_dirty(). It should not be released 2nd time
> in rollback after failed ext4_handle_dirty_metadata(gdb_bh)
> 
> Fixes b40971426a83 ("ext4: add error checking to calls to ...") # 2.6.38
> 
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>

I fixed this in a simpler way:

From 6a91a2eb1c5af1381caebfb4c1c91dc175351f6f Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <tytso@mit.edu>
Date: Tue, 6 Nov 2018 17:18:17 -0500
Subject: [PATCH] ext4: avoid possible double brelse() in add_new_gdb() on
 error path

Fixes: b40971426a83 ("ext4: add error checking to calls to ...")
Reported-by: Vasily Averin <vvs@virtuozzo.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Cc: stable@vger.kernel.org # 2.6.38
---
 fs/ext4/resize.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 85158e9de7c2..a5efee34415f 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -871,6 +871,7 @@ static int add_new_gdb(handle_t *handle, struct inode *inode,
 	err = ext4_handle_dirty_metadata(handle, NULL, gdb_bh);
 	if (unlikely(err)) {
 		ext4_std_error(sb, err);
+		iloc.bh = NULL;
 		goto exit_inode;
 	}
 	brelse(dind);
-- 
2.18.0.rc0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 10/11] ext4: remove useless brelse call in ext4_xattr_inode_update_ref()
  2018-10-30 21:58 ` [PATCH v2 10/11] ext4: remove useless brelse call in ext4_xattr_inode_update_ref() Vasily Averin
@ 2018-11-06 22:49   ` Theodore Y. Ts'o
  0 siblings, 0 replies; 23+ messages in thread
From: Theodore Y. Ts'o @ 2018-11-06 22:49 UTC (permalink / raw)
  To: Vasily Averin; +Cc: linux-ext4, Andreas Dilger, linux-kernel

On Wed, Oct 31, 2018 at 12:58:46AM +0300, Vasily Averin wrote:
> brelse(iloc.bh) is useless here, it is always called with iloc.bh = NULL
> 
> Fixes dec214d00e0d ("ext4: xattr inode deduplication") # 4.13
> cc: Tahsin Erdogan <tahsin@google.com>
> 
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>

Applied.  I droppped the "Fixes" since it's really not needed for
stable kernel; this is really more of a cleanup.  I used the commit
description:

    ext4: remove unneeded brelse call in ext4_xattr_inode_update_ref()

    	  	 	  	      - Ted

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2018-11-06 22:49 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <cover.1540935522.git.vvs@virtuozzo.com>
2018-10-30 21:57 ` [PATCH v2 01/11] ext4 resise: extra brelse in setup_new_flex_group_blocks() Vasily Averin
2018-11-03 20:20   ` Theodore Y. Ts'o
2018-10-30 21:57 ` [PATCH v2 02/11] ext4 resize: missing brelse() after errors in set_flexbg_block_bitmap() Vasily Averin
2018-11-03 20:28   ` Theodore Y. Ts'o
2018-10-30 21:57 ` [PATCH v2 03/11] ext4 resize: brelse() cleanup in add_new_gdb_meta_bg() Vasily Averin
2018-11-03 20:55   ` Theodore Y. Ts'o
2018-10-30 21:58 ` [PATCH v2 04/11] ext4 resize: lost brelse() in update_backups() Vasily Averin
2018-11-03 21:14   ` Theodore Y. Ts'o
2018-10-30 21:58 ` [PATCH v2 05/11] ext4 resize: lost rollback in ext4_resize_fs() Vasily Averin
2018-11-06 21:17   ` Theodore Y. Ts'o
2018-10-30 21:58 ` [PATCH v2 06/11] ext4 resize: lost resize_inode cleanup before retry " Vasily Averin
2018-11-06 21:23   ` Theodore Y. Ts'o
2018-10-30 21:58 ` [PATCH v2 07/11] ext4: lost put_bh in ext4_mark_iloc_dirty() Vasily Averin
2018-11-06 21:53   ` Theodore Y. Ts'o
2018-10-30 21:58 ` [PATCH v2 08/11] ext4: lost brelse in ext4_orphan_add() Vasily Averin
2018-11-06 22:04   ` Theodore Y. Ts'o
2018-10-30 21:58 ` [PATCH v2 09/11] ext4: iloc.bh cleanup in add_new_gdb() Vasily Averin
2018-11-06 22:19   ` Theodore Y. Ts'o
2018-10-30 21:58 ` [PATCH v2 10/11] ext4: remove useless brelse call in ext4_xattr_inode_update_ref() Vasily Averin
2018-11-06 22:49   ` Theodore Y. Ts'o
2018-10-30 21:58 ` [PATCH v2 11/11] ext4: access to uninitialized bh fields in ext4_xattr_set_handle() Vasily Averin
     [not found]   ` <48BA929C-9BF7-4092-BDD3-BECB64AF0599@dilger.ca>
2018-10-31  3:39     ` Vasily Averin
2018-11-01  0:42       ` Andreas Dilger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).