lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 5 Jan 2022 18:24:36 -0500 From: "Theodore Ts'o" <tytso@....edu> To: cgel.zte@...il.com Cc: Andreas Dilger <adilger.kernel@...ger.ca>, linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org, luo penghao <luo.penghao@....com.cn>, Zeal Robot <zealci@....com.cn>, Harshad Shirwadkar <harshadshirwadkar@...il.com> Subject: Re: [PATCH v2] ext4: Remove unnecessary assignments On Thu, Nov 04, 2021 at 06:36:37AM +0000, cgel.zte@...il.com wrote: > From: luo penghao <luo.penghao@....com.cn> > > The assignment at the end of the function is not necessary > > diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c > index 089c958..f1258a7 100644 > --- a/fs/ext4/mballoc.c > +++ b/fs/ext4/mballoc.c > @@ -3886,7 +3886,7 @@ void ext4_mb_mark_bb(struct super_block *sb, ext4_fsblk_t block, > if (err) > goto out_err; > sync_dirty_buffer(bitmap_bh); > - err = ext4_handle_dirty_metadata(NULL, NULL, gdp_bh); > + ext4_handle_dirty_metadata(NULL, NULL, gdp_bh); > sync_dirty_buffer(gdp_bh); > > out_err: There's actually a bigger issue here than the Clang analyzer complaining about the unnecessasary assignment --- and that is we *should* be propagating the error up to ext4_mb_mark_bb's callers, and those callers should be logging errors and potentially aborting the fast_commit replay. There might be some errors that can be ignored, if an idempotent operation doesn't need to be redone. However, in cases like ext4_handle_dirty_metadata(), or ext4_read_block_bitmap(), any failures are probably due to something fatal happening --- either an ENOMEM, or an I/O error, etc., and simply silently aborting the current function without logging any kind of problem is going to make it much harder to root cause a potential fast commit replay failure. So what we should probably do is make ext4_mb_mark_bb return an error, and then we'll need to look at all of the callers of ext4_mb_mark_bb, and fix them up as necessary. - Ted
Powered by blists - more mailing lists