* [PATCH 1/4] e2fsck: don't ignore return values in e2fsck_rewrite_extent_tree
@ 2021-02-19 21:03 Harshad Shirwadkar
2021-02-19 21:03 ` [PATCH 2/4] ext2fs: don't ignore return value in ext2fs_count_blocks Harshad Shirwadkar
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Harshad Shirwadkar @ 2021-02-19 21:03 UTC (permalink / raw)
To: linux-ext4; +Cc: tytso, Harshad Shirwadkar
From: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
Don't ignore return values of ext2fs_read/write_inode_full() in
e2fsck_rewrite_extent_tree.
Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
---
e2fsck/extents.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/e2fsck/extents.c b/e2fsck/extents.c
index 600dbc97..f48f14ff 100644
--- a/e2fsck/extents.c
+++ b/e2fsck/extents.c
@@ -290,8 +290,10 @@ errcode_t e2fsck_rewrite_extent_tree(e2fsck_t ctx, struct extent_list *list)
errcode_t err;
memset(&inode, 0, sizeof(inode));
- ext2fs_read_inode_full(ctx->fs, list->ino, EXT2_INODE(&inode),
- sizeof(inode));
+ err = ext2fs_read_inode_full(ctx->fs, list->ino, EXT2_INODE(&inode),
+ sizeof(inode));
+ if (err)
+ return err;
/* Skip deleted inodes and inline data files */
if (inode.i_flags & EXT4_INLINE_DATA_FL)
@@ -306,10 +308,8 @@ errcode_t e2fsck_rewrite_extent_tree(e2fsck_t ctx, struct extent_list *list)
if (err)
return err;
ext2fs_iblk_set(ctx->fs, EXT2_INODE(&inode), blk_count);
- ext2fs_write_inode_full(ctx->fs, list->ino, EXT2_INODE(&inode),
- sizeof(inode));
-
- return 0;
+ return ext2fs_write_inode_full(ctx->fs, list->ino, EXT2_INODE(&inode),
+ sizeof(inode));
}
errcode_t e2fsck_read_extents(e2fsck_t ctx, struct extent_list *extents)
--
2.30.0.617.g56c4b15f3c-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/4] ext2fs: don't ignore return value in ext2fs_count_blocks
2021-02-19 21:03 [PATCH 1/4] e2fsck: don't ignore return values in e2fsck_rewrite_extent_tree Harshad Shirwadkar
@ 2021-02-19 21:03 ` Harshad Shirwadkar
2021-02-19 21:03 ` [PATCH 3/4] e2fsck: add fallthrough comment in fc replay switch case Harshad Shirwadkar
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Harshad Shirwadkar @ 2021-02-19 21:03 UTC (permalink / raw)
To: linux-ext4; +Cc: tytso, Harshad Shirwadkar
From: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
Don't ignore return value of ext2fs_get_array() in
ext2fs_count_blocks().
Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
---
lib/ext2fs/extent.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/ext2fs/extent.c b/lib/ext2fs/extent.c
index 1a87e68b..9e611038 100644
--- a/lib/ext2fs/extent.c
+++ b/lib/ext2fs/extent.c
@@ -1824,8 +1824,11 @@ errcode_t ext2fs_count_blocks(ext2_filsys fs, ext2_ino_t ino,
if (errcode)
goto out;
- ext2fs_get_array(handle->max_depth, sizeof(blk64_t),
- &intermediate_nodes);
+ errcode = ext2fs_get_array(handle->max_depth, sizeof(blk64_t),
+ &intermediate_nodes);
+ if (errcode)
+ goto out;
+
blkcount = handle->level;
while (!errcode) {
if (extent.e_flags & EXT2_EXTENT_FLAGS_LEAF) {
--
2.30.0.617.g56c4b15f3c-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/4] e2fsck: add fallthrough comment in fc replay switch case
2021-02-19 21:03 [PATCH 1/4] e2fsck: don't ignore return values in e2fsck_rewrite_extent_tree Harshad Shirwadkar
2021-02-19 21:03 ` [PATCH 2/4] ext2fs: don't ignore return value in ext2fs_count_blocks Harshad Shirwadkar
@ 2021-02-19 21:03 ` Harshad Shirwadkar
2021-02-19 21:03 ` [PATCH 4/4] e2fsck: initialize variable before first use in fast commit replay Harshad Shirwadkar
2021-02-20 8:58 ` [PATCH 1/4] e2fsck: don't ignore return values in e2fsck_rewrite_extent_tree Благодаренко Артём
3 siblings, 0 replies; 8+ messages in thread
From: Harshad Shirwadkar @ 2021-02-19 21:03 UTC (permalink / raw)
To: linux-ext4; +Cc: tytso, Harshad Shirwadkar
From: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
During fast commit replay scan phase, in ext4_fc_replay_scan(), we
want to fallthrough in switch case for EXT4_FC_TAG_ADD_RANGE case. Add
a comment for that.
Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
---
e2fsck/journal.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/e2fsck/journal.c b/e2fsck/journal.c
index 2708942a..a67ef745 100644
--- a/e2fsck/journal.c
+++ b/e2fsck/journal.c
@@ -325,6 +325,7 @@ static int ext4_fc_replay_scan(journal_t *j, struct buffer_head *bh,
ret = JBD2_FC_REPLAY_STOP;
else
ret = JBD2_FC_REPLAY_CONTINUE;
+ /* fallthrough */
case EXT4_FC_TAG_DEL_RANGE:
case EXT4_FC_TAG_LINK:
case EXT4_FC_TAG_UNLINK:
--
2.30.0.617.g56c4b15f3c-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/4] e2fsck: initialize variable before first use in fast commit replay
2021-02-19 21:03 [PATCH 1/4] e2fsck: don't ignore return values in e2fsck_rewrite_extent_tree Harshad Shirwadkar
2021-02-19 21:03 ` [PATCH 2/4] ext2fs: don't ignore return value in ext2fs_count_blocks Harshad Shirwadkar
2021-02-19 21:03 ` [PATCH 3/4] e2fsck: add fallthrough comment in fc replay switch case Harshad Shirwadkar
@ 2021-02-19 21:03 ` Harshad Shirwadkar
2021-02-21 23:14 ` Theodore Ts'o
2021-02-20 8:58 ` [PATCH 1/4] e2fsck: don't ignore return values in e2fsck_rewrite_extent_tree Благодаренко Артём
3 siblings, 1 reply; 8+ messages in thread
From: Harshad Shirwadkar @ 2021-02-19 21:03 UTC (permalink / raw)
To: linux-ext4; +Cc: tytso, Harshad Shirwadkar
From: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
Initialize ext2fs_ex variable in ext4_fc_replay_scan() before first
use.
Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
---
e2fsck/journal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/e2fsck/journal.c b/e2fsck/journal.c
index a67ef745..8e7ba819 100644
--- a/e2fsck/journal.c
+++ b/e2fsck/journal.c
@@ -289,7 +289,7 @@ static int ext4_fc_replay_scan(journal_t *j, struct buffer_head *bh,
struct ext4_fc_tail *tail;
__u8 *start, *end;
struct ext4_fc_head *head;
- struct ext2fs_extent ext2fs_ex;
+ struct ext2fs_extent ext2fs_ex = {0};
state = &ctx->fc_replay_state;
--
2.30.0.617.g56c4b15f3c-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] e2fsck: don't ignore return values in e2fsck_rewrite_extent_tree
2021-02-19 21:03 [PATCH 1/4] e2fsck: don't ignore return values in e2fsck_rewrite_extent_tree Harshad Shirwadkar
` (2 preceding siblings ...)
2021-02-19 21:03 ` [PATCH 4/4] e2fsck: initialize variable before first use in fast commit replay Harshad Shirwadkar
@ 2021-02-20 8:58 ` Благодаренко Артём
2021-02-23 17:40 ` harshad shirwadkar
3 siblings, 1 reply; 8+ messages in thread
From: Благодаренко Артём @ 2021-02-20 8:58 UTC (permalink / raw)
To: Harshad Shirwadkar; +Cc: linux-ext4, tytso
Hello Harshad,
ext2fs_iblk_set in the same e2fsck_rewrite_extent_tee returns a return code, but code is ignored.
Could you also add check there?
Best regards,
Artem Blagodarenko
> On 20 Feb 2021, at 00:03, Harshad Shirwadkar <harshadshirwadkar@gmail.com> wrote:
>
> From: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
>
> Don't ignore return values of ext2fs_read/write_inode_full() in
> e2fsck_rewrite_extent_tree.
>
> Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
> ---
> e2fsck/extents.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/e2fsck/extents.c b/e2fsck/extents.c
> index 600dbc97..f48f14ff 100644
> --- a/e2fsck/extents.c
> +++ b/e2fsck/extents.c
> @@ -290,8 +290,10 @@ errcode_t e2fsck_rewrite_extent_tree(e2fsck_t ctx, struct extent_list *list)
> errcode_t err;
>
> memset(&inode, 0, sizeof(inode));
> - ext2fs_read_inode_full(ctx->fs, list->ino, EXT2_INODE(&inode),
> - sizeof(inode));
> + err = ext2fs_read_inode_full(ctx->fs, list->ino, EXT2_INODE(&inode),
> + sizeof(inode));
> + if (err)
> + return err;
>
> /* Skip deleted inodes and inline data files */
> if (inode.i_flags & EXT4_INLINE_DATA_FL)
> @@ -306,10 +308,8 @@ errcode_t e2fsck_rewrite_extent_tree(e2fsck_t ctx, struct extent_list *list)
> if (err)
> return err;
> ext2fs_iblk_set(ctx->fs, EXT2_INODE(&inode), blk_count);
> - ext2fs_write_inode_full(ctx->fs, list->ino, EXT2_INODE(&inode),
> - sizeof(inode));
> -
> - return 0;
> + return ext2fs_write_inode_full(ctx->fs, list->ino, EXT2_INODE(&inode),
> + sizeof(inode));
> }
>
> errcode_t e2fsck_read_extents(e2fsck_t ctx, struct extent_list *extents)
> --
> 2.30.0.617.g56c4b15f3c-goog
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 4/4] e2fsck: initialize variable before first use in fast commit replay
2021-02-19 21:03 ` [PATCH 4/4] e2fsck: initialize variable before first use in fast commit replay Harshad Shirwadkar
@ 2021-02-21 23:14 ` Theodore Ts'o
2021-02-23 17:41 ` harshad shirwadkar
0 siblings, 1 reply; 8+ messages in thread
From: Theodore Ts'o @ 2021-02-21 23:14 UTC (permalink / raw)
To: Harshad Shirwadkar; +Cc: linux-ext4
On Fri, Feb 19, 2021 at 01:03:33PM -0800, Harshad Shirwadkar wrote:
> From: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
>
> Initialize ext2fs_ex variable in ext4_fc_replay_scan() before first
> use.
>
> Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
I wonder if we should make the following change to
ext2fs_decode_extent(), which will prevent other future bugs to
potential users of the function:
to->e_pblk = ext2fs_le32_to_cpu(from->ee_start) +
((__u64) ext2fs_le16_to_cpu(from->ee_start_hi)
<< 32);
to->e_lblk = ext2fs_le32_to_cpu(from->ee_block);
to->e_len = ext2fs_le16_to_cpu(from->ee_len);
- to->e_flags |= EXT2_EXTENT_FLAGS_LEAF;
+ to->e_flags = EXT2_EXTENT_FLAGS_LEAF;
ext2fs_decode_extent() overwrites all other members of the structure,
so we might as well just initialize e_flags as opposed to depending
the caller to initiaize *to just for the sake of to->e_flags.
Cheers,
- Ted
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] e2fsck: don't ignore return values in e2fsck_rewrite_extent_tree
2021-02-20 8:58 ` [PATCH 1/4] e2fsck: don't ignore return values in e2fsck_rewrite_extent_tree Благодаренко Артём
@ 2021-02-23 17:40 ` harshad shirwadkar
0 siblings, 0 replies; 8+ messages in thread
From: harshad shirwadkar @ 2021-02-23 17:40 UTC (permalink / raw)
To: Благодаренко
Артём
Cc: Ext4 Developers List, Theodore Y. Ts'o
Thanks Artem, will fix this in V2.
- Harshad
On Sat, Feb 20, 2021 at 12:58 AM Благодаренко Артём
<artem.blagodarenko@gmail.com> wrote:
>
> Hello Harshad,
>
> ext2fs_iblk_set in the same e2fsck_rewrite_extent_tee returns a return code, but code is ignored.
> Could you also add check there?
>
> Best regards,
> Artem Blagodarenko
>
> > On 20 Feb 2021, at 00:03, Harshad Shirwadkar <harshadshirwadkar@gmail.com> wrote:
> >
> > From: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
> >
> > Don't ignore return values of ext2fs_read/write_inode_full() in
> > e2fsck_rewrite_extent_tree.
> >
> > Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
> > ---
> > e2fsck/extents.c | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/e2fsck/extents.c b/e2fsck/extents.c
> > index 600dbc97..f48f14ff 100644
> > --- a/e2fsck/extents.c
> > +++ b/e2fsck/extents.c
> > @@ -290,8 +290,10 @@ errcode_t e2fsck_rewrite_extent_tree(e2fsck_t ctx, struct extent_list *list)
> > errcode_t err;
> >
> > memset(&inode, 0, sizeof(inode));
> > - ext2fs_read_inode_full(ctx->fs, list->ino, EXT2_INODE(&inode),
> > - sizeof(inode));
> > + err = ext2fs_read_inode_full(ctx->fs, list->ino, EXT2_INODE(&inode),
> > + sizeof(inode));
> > + if (err)
> > + return err;
> >
> > /* Skip deleted inodes and inline data files */
> > if (inode.i_flags & EXT4_INLINE_DATA_FL)
> > @@ -306,10 +308,8 @@ errcode_t e2fsck_rewrite_extent_tree(e2fsck_t ctx, struct extent_list *list)
> > if (err)
> > return err;
> > ext2fs_iblk_set(ctx->fs, EXT2_INODE(&inode), blk_count);
> > - ext2fs_write_inode_full(ctx->fs, list->ino, EXT2_INODE(&inode),
> > - sizeof(inode));
> > -
> > - return 0;
> > + return ext2fs_write_inode_full(ctx->fs, list->ino, EXT2_INODE(&inode),
> > + sizeof(inode));
> > }
> >
> > errcode_t e2fsck_read_extents(e2fsck_t ctx, struct extent_list *extents)
> > --
> > 2.30.0.617.g56c4b15f3c-goog
> >
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 4/4] e2fsck: initialize variable before first use in fast commit replay
2021-02-21 23:14 ` Theodore Ts'o
@ 2021-02-23 17:41 ` harshad shirwadkar
0 siblings, 0 replies; 8+ messages in thread
From: harshad shirwadkar @ 2021-02-23 17:41 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Ext4 Developers List
Thanks Ted, it makes sense, will fix this in V2.
- Harshad
On Sun, Feb 21, 2021 at 3:14 PM Theodore Ts'o <tytso@mit.edu> wrote:
>
> On Fri, Feb 19, 2021 at 01:03:33PM -0800, Harshad Shirwadkar wrote:
> > From: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
> >
> > Initialize ext2fs_ex variable in ext4_fc_replay_scan() before first
> > use.
> >
> > Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
>
> I wonder if we should make the following change to
> ext2fs_decode_extent(), which will prevent other future bugs to
> potential users of the function:
>
> to->e_pblk = ext2fs_le32_to_cpu(from->ee_start) +
> ((__u64) ext2fs_le16_to_cpu(from->ee_start_hi)
> << 32);
> to->e_lblk = ext2fs_le32_to_cpu(from->ee_block);
> to->e_len = ext2fs_le16_to_cpu(from->ee_len);
> - to->e_flags |= EXT2_EXTENT_FLAGS_LEAF;
> + to->e_flags = EXT2_EXTENT_FLAGS_LEAF;
>
> ext2fs_decode_extent() overwrites all other members of the structure,
> so we might as well just initialize e_flags as opposed to depending
> the caller to initiaize *to just for the sake of to->e_flags.
>
> Cheers,
>
> - Ted
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-02-23 17:41 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-19 21:03 [PATCH 1/4] e2fsck: don't ignore return values in e2fsck_rewrite_extent_tree Harshad Shirwadkar
2021-02-19 21:03 ` [PATCH 2/4] ext2fs: don't ignore return value in ext2fs_count_blocks Harshad Shirwadkar
2021-02-19 21:03 ` [PATCH 3/4] e2fsck: add fallthrough comment in fc replay switch case Harshad Shirwadkar
2021-02-19 21:03 ` [PATCH 4/4] e2fsck: initialize variable before first use in fast commit replay Harshad Shirwadkar
2021-02-21 23:14 ` Theodore Ts'o
2021-02-23 17:41 ` harshad shirwadkar
2021-02-20 8:58 ` [PATCH 1/4] e2fsck: don't ignore return values in e2fsck_rewrite_extent_tree Благодаренко Артём
2021-02-23 17:40 ` harshad shirwadkar
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.