[PATCH v2 01/11] ext4 resise: extra brelse in setup_new_flex_group_blocks()

[PATCH v2 01/11] ext4 resise: extra brelse in setup_new_flex_group_blocks()

[Vasily Averin]

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

Re: [PATCH v2 01/11] ext4 resise: extra brelse in setup_new_flex_group_blocks()

[Theodore Y. Ts'o]

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

[PATCH v2 02/11] ext4 resize: missing brelse() after errors in set_flexbg_block_bitmap()

[Vasily Averin]

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

Re: [PATCH v2 02/11] ext4 resize: missing brelse() after errors in set_flexbg_block_bitmap()

[Theodore Y. Ts'o]

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

[PATCH v2 03/11] ext4 resize: brelse() cleanup in add_new_gdb_meta_bg()

[Vasily Averin]

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

Re: [PATCH v2 03/11] ext4 resize: brelse() cleanup in add_new_gdb_meta_bg()

[Theodore Y. Ts'o]

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
								

[PATCH v2 04/11] ext4 resize: lost brelse() in update_backups()

[Vasily Averin]

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

Re: [PATCH v2 04/11] ext4 resize: lost brelse() in update_backups()

[Theodore Y. Ts'o]

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

[PATCH v2 05/11] ext4 resize: lost rollback in ext4_resize_fs()

[Vasily Averin]

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

Re: [PATCH v2 05/11] ext4 resize: lost rollback in ext4_resize_fs()

[Theodore Y. Ts'o]

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

[PATCH v2 06/11] ext4 resize: lost resize_inode cleanup before retry in ext4_resize_fs()

[Vasily Averin]

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

Re: [PATCH v2 06/11] ext4 resize: lost resize_inode cleanup before retry in ext4_resize_fs()

[Theodore Y. Ts'o]

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

[PATCH v2 07/11] ext4: lost put_bh in ext4_mark_iloc_dirty()

[Vasily Averin]

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

Re: [PATCH v2 07/11] ext4: lost put_bh in ext4_mark_iloc_dirty()

[Theodore Y. Ts'o]

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

[PATCH v2 08/11] ext4: lost brelse in ext4_orphan_add()

[Vasily Averin]

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

Re: [PATCH v2 08/11] ext4: lost brelse in ext4_orphan_add()

[Theodore Y. Ts'o]

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

[PATCH v2 09/11] ext4: iloc.bh cleanup in add_new_gdb()

[Vasily Averin]

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

Re: [PATCH v2 09/11] ext4: iloc.bh cleanup in add_new_gdb()

[Theodore Y. Ts'o]

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

[PATCH v2 10/11] ext4: remove useless brelse call in ext4_xattr_inode_update_ref()

[Vasily Averin]

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

Re: [PATCH v2 10/11] ext4: remove useless brelse call in ext4_xattr_inode_update_ref()

[Theodore Y. Ts'o]

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

[PATCH v2 11/11] ext4: access to uninitialized bh fields in ext4_xattr_set_handle()

[Vasily Averin]

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

Re: [PATCH v2 11/11] ext4: access to uninitialized bh fields in ext4_xattr_set_handle()

[Vasily Averin]

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

Re: [PATCH v2 11/11] ext4: access to uninitialized bh fields in ext4_xattr_set_handle()

[Andreas Dilger]

[-- 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 --]