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 linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-Id: <6D4783E5-BFDB-42E0-800A-DFD18B3E7A64@dilger.ca> Date: Fri, 14 Aug 2015 11:03:46 -0600 From: Andreas Dilger <adilger@...ger.ca> To: Dan Carpenter <dan.carpenter@...cle.com> Cc: Theodore Ts'o <tytso@....edu>, Ext4 Developers List <linux-ext4@...r.kernel.org>, kernel-janitors@...r.kernel.org Subject: Re: [patch 1/2 v2] ext4: simplify some code in read_mmp_block() > On Aug 14, 2015, at 3:47 AM, Dan Carpenter <dan.carpenter@...cle.com> wrote: > > My static check complains because we have: > > if (!*bh) > return -ENOMEM; > if (*bh) { > > The second check is unnecessary. > > I've simplified this code by moving the "if (!*bh)" checks around. Also > Andreas Dilger says we should probably print a warning if sb_getblk() > fails. > > Signed-off-by: Dan Carpenter <dan.carpenter@...cle.com> Reviewed-by: Andreas Dilger <adilger@...ger.ca> > --- > v2: move the code around even more, add a warning message > > diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c > index 8313ca3..0880ec9 100644 > --- a/fs/ext4/mmp.c > +++ b/fs/ext4/mmp.c > @@ -69,6 +69,7 @@ static int read_mmp_block(struct super_block *sb, struct buffer_head **bh, > ext4_fsblk_t mmp_block) > { > struct mmp_struct *mmp; > + int ret; > > if (*bh) > clear_buffer_uptodate(*bh); > @@ -76,25 +77,24 @@ static int read_mmp_block(struct super_block *sb, struct buffer_head **bh, > /* This would be sb_bread(sb, mmp_block), except we need to be sure > * that the MD RAID device cache has been bypassed, and that the read > * is not blocked in the elevator. */ > - if (!*bh) > + if (!*bh) { > *bh = sb_getblk(sb, mmp_block); > - if (!*bh) > - return -ENOMEM; > - if (*bh) { > - get_bh(*bh); > - lock_buffer(*bh); > - (*bh)->b_end_io = end_buffer_read_sync; > - submit_bh(READ_SYNC | REQ_META | REQ_PRIO, *bh); > - wait_on_buffer(*bh); > - if (!buffer_uptodate(*bh)) { > - brelse(*bh); > - *bh = NULL; > + if (!*bh) { > + ret = -ENOMEM; > + goto warn_exit; > } > } > - if (unlikely(!*bh)) { > - ext4_warning(sb, "Error while reading MMP block %llu", > - mmp_block); > - return -EIO; > + > + get_bh(*bh); > + lock_buffer(*bh); > + (*bh)->b_end_io = end_buffer_read_sync; > + submit_bh(READ_SYNC | REQ_META | REQ_PRIO, *bh); > + wait_on_buffer(*bh); > + if (!buffer_uptodate(*bh)) { > + brelse(*bh); > + *bh = NULL; > + ret = -EIO; > + goto warn_exit; > } > > mmp = (struct mmp_struct *)((*bh)->b_data); > @@ -103,6 +103,10 @@ static int read_mmp_block(struct super_block *sb, struct buffer_head **bh, > return -EINVAL; > > return 0; > + > +warn_exit: > + ext4_warning(sb, "Error while reading MMP block %llu", mmp_block); It wouldn't be terrible to include the error in this message, but since it will also be returned to userspace it isn't critical. > + return ret; > } > > /* Cheers, Andreas -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists