* [PATCH 1/4] befs: check allocation_group number before use @ 2016-08-10 22:12 Salah Triki 2016-08-10 22:12 ` [PATCH 2/4] befs: check return value of iaddr2blockno Salah Triki ` (3 more replies) 0 siblings, 4 replies; 9+ messages in thread From: Salah Triki @ 2016-08-10 22:12 UTC (permalink / raw) To: akpm, viro, luisbg; +Cc: linux-kernel, salah.triki Check that the allocation group number is not greater or equal to the number of allocations group in the file system and return BEF_ERR in the case of error. Signed-off-by: Salah Triki <salah.triki@gmail.com> --- fs/befs/befs.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/fs/befs/befs.h b/fs/befs/befs.h index 55f3ea2..6daf4c4 100644 --- a/fs/befs/befs.h +++ b/fs/befs/befs.h @@ -121,6 +121,11 @@ BEFS_I(const struct inode *inode) static inline befs_blocknr_t iaddr2blockno(struct super_block *sb, const befs_inode_addr *iaddr) { + if (iaddr->allocation_group >= BEFS_SB(sb)->num_ags) { + befs_error(sb, "BEFS: Invalid allocation group %u, max is %u", + iaddr->allocation_group, BEFS_SB(sb)->num_ags); + return BEFS_ERR; + } return ((iaddr->allocation_group << BEFS_SB(sb)->ag_shift) + iaddr->start); } -- 1.9.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/4] befs: check return value of iaddr2blockno 2016-08-10 22:12 [PATCH 1/4] befs: check allocation_group number before use Salah Triki @ 2016-08-10 22:12 ` Salah Triki 2016-08-10 22:12 ` [PATCH 3/4] befs: remove the validation of allocation group number Salah Triki ` (2 subsequent siblings) 3 siblings, 0 replies; 9+ messages in thread From: Salah Triki @ 2016-08-10 22:12 UTC (permalink / raw) To: akpm, viro, luisbg; +Cc: linux-kernel, salah.triki The call of iaddr2blockno may fail, so check its return value and propagate error, if any. Signed-off-by: Salah Triki <salah.triki@gmail.com> --- fs/befs/datastream.c | 30 ++++++++++++++++++++---------- fs/befs/inode.c | 10 ++++++++-- fs/befs/io.c | 3 +++ fs/befs/linuxvfs.c | 12 +++++++++++- 4 files changed, 42 insertions(+), 13 deletions(-) diff --git a/fs/befs/datastream.c b/fs/befs/datastream.c index 343123c..1747f64 100644 --- a/fs/befs/datastream.c +++ b/fs/befs/datastream.c @@ -308,8 +308,11 @@ befs_find_brun_indirect(struct super_block *sb, befs_disk_block_run *array; befs_block_run indirect = data->indirect; - befs_blocknr_t indirblockno = iaddr2blockno(sb, &indirect); int arraylen = befs_iaddrs_per_block(sb); + befs_blocknr_t indirblockno = iaddr2blockno(sb, &indirect); + + if (indirblockno == BEFS_ERR) + return BEFS_ERR; befs_debug(sb, "---> %s, find %lu", __func__, (unsigned long)blockno); @@ -422,6 +425,7 @@ befs_find_brun_dblindirect(struct super_block *sb, struct buffer_head *indir_block; befs_block_run indir_run; befs_disk_inode_addr *iaddr_array; + befs_blocknr_t block; befs_blocknr_t indir_start_blk = data->max_indirect_range >> BEFS_SB(sb)->block_shift; @@ -461,15 +465,17 @@ befs_find_brun_dblindirect(struct super_block *sb, return BEFS_ERR; } - dbl_indir_block = - sb_bread(sb, iaddr2blockno(sb, &data->double_indirect) + - dbl_which_block); + block = iaddr2blockno(sb, &data->double_indirect); + + if (block == BEFS_ERR) + return BEFS_ERR; + + dbl_indir_block = sb_bread(sb, blockno + dbl_which_block); + if (dbl_indir_block == NULL) { befs_error(sb, "%s couldn't read the " "double-indirect block at blockno %lu", __func__, - (unsigned long) - iaddr2blockno(sb, &data->double_indirect) + - dbl_which_block); + (unsigned long)blockno + dbl_which_block); return BEFS_ERR; } @@ -488,12 +494,16 @@ befs_find_brun_dblindirect(struct super_block *sb, return BEFS_ERR; } - indir_block = - sb_bread(sb, iaddr2blockno(sb, &indir_run) + which_block); + blockno = iaddr2blockno(sb, &indir_run); + + if (blockno == BEFS_ERR) + return BEFS_ERR; + + indir_block = sb_bread(sb, blockno + which_block); if (indir_block == NULL) { befs_error(sb, "%s couldn't read the indirect block " "at blockno %lu", __func__, (unsigned long) - iaddr2blockno(sb, &indir_run) + which_block); + blockno + which_block); return BEFS_ERR; } diff --git a/fs/befs/inode.c b/fs/befs/inode.c index fa4b718..495d270 100644 --- a/fs/befs/inode.c +++ b/fs/befs/inode.c @@ -21,6 +21,7 @@ befs_check_inode(struct super_block *sb, befs_inode * raw_inode, u32 magic1 = fs32_to_cpu(sb, raw_inode->magic1); befs_inode_addr ino_num = fsrun_to_cpu(sb, raw_inode->inode_num); u32 flags = fs32_to_cpu(sb, raw_inode->flags); + befs_blocknr_t block; /* check magic header. */ if (magic1 != BEFS_INODE_MAGIC1) { @@ -33,10 +34,15 @@ befs_check_inode(struct super_block *sb, befs_inode * raw_inode, /* * Sanity check2: inodes store their own block address. Check it. */ - if (inode != iaddr2blockno(sb, &ino_num)) { + block = iaddr2blockno(sb, &ino_num); + + if (block == BEFS_ERR) + return BEFS_ERR; + + if (inode != block) { befs_error(sb, "inode blocknr field disagrees with vfs " "VFS: %lu, Inode %lu", (unsigned long) - inode, (unsigned long)iaddr2blockno(sb, &ino_num)); + inode, (unsigned long)block); return BEFS_BAD_INODE; } diff --git a/fs/befs/io.c b/fs/befs/io.c index 4223b77..14eef7d 100644 --- a/fs/befs/io.c +++ b/fs/befs/io.c @@ -42,6 +42,9 @@ befs_bread_iaddr(struct super_block *sb, befs_inode_addr iaddr) block = iaddr2blockno(sb, &iaddr); + if (block == BEFS_ERR) + goto error; + befs_debug(sb, "%s: offset = %lu", __func__, (unsigned long)block); bh = sb_bread(sb, block); diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c index c57f831..6635515 100644 --- a/fs/befs/linuxvfs.c +++ b/fs/befs/linuxvfs.c @@ -143,6 +143,9 @@ befs_get_block(struct inode *inode, sector_t block, disk_off = (ulong) iaddr2blockno(sb, &run); + if (disk_off == BEFS_ERR) + return -EINVAL; + map_bh(bh_result, inode->i_sb, disk_off); befs_debug(sb, "<--- %s for inode %lu, block %ld, disk address %lu", @@ -756,6 +759,7 @@ befs_fill_super(struct super_block *sb, void *data, int silent) long ret = -EINVAL; const unsigned long sb_block = 0; const off_t x86_sb_off = 512; + befs_blocknr_t block; save_mount_options(sb, data); @@ -830,7 +834,13 @@ befs_fill_super(struct super_block *sb, void *data, int silent) /* Set real blocksize of fs */ sb_set_blocksize(sb, (ulong) befs_sb->block_size); sb->s_op = &befs_sops; - root = befs_iget(sb, iaddr2blockno(sb, &(befs_sb->root_dir))); + + block = iaddr2blockno(sb, &(befs_sb->root_dir)); + + if (block == BEFS_ERR) + goto unacquire_priv_sbp; + + root = befs_iget(sb, block); if (IS_ERR(root)) { ret = PTR_ERR(root); goto unacquire_priv_sbp; -- 1.9.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/4] befs: remove the validation of allocation group number 2016-08-10 22:12 [PATCH 1/4] befs: check allocation_group number before use Salah Triki 2016-08-10 22:12 ` [PATCH 2/4] befs: check return value of iaddr2blockno Salah Triki @ 2016-08-10 22:12 ` Salah Triki 2016-08-10 22:12 ` [PATCH 4/4] befs: remove unnecessary initialization Salah Triki 2016-08-11 10:56 ` [PATCH 1/4] befs: check allocation_group number before use Luis de Bethencourt 3 siblings, 0 replies; 9+ messages in thread From: Salah Triki @ 2016-08-10 22:12 UTC (permalink / raw) To: akpm, viro, luisbg; +Cc: linux-kernel, salah.triki Validating the allocation group number is unnecessary since it will be done by iaddr2blockno(). Signed-off-by: Salah Triki <salah.triki@gmail.com> --- fs/befs/io.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/fs/befs/io.c b/fs/befs/io.c index 14eef7d..f9790f5 100644 --- a/fs/befs/io.c +++ b/fs/befs/io.c @@ -28,18 +28,11 @@ befs_bread_iaddr(struct super_block *sb, befs_inode_addr iaddr) { struct buffer_head *bh; befs_blocknr_t block = 0; - struct befs_sb_info *befs_sb = BEFS_SB(sb); befs_debug(sb, "---> Enter %s " "[%u, %hu, %hu]", __func__, iaddr.allocation_group, iaddr.start, iaddr.len); - if (iaddr.allocation_group > befs_sb->num_ags) { - befs_error(sb, "BEFS: Invalid allocation group %u, max is %u", - iaddr.allocation_group, befs_sb->num_ags); - goto error; - } - block = iaddr2blockno(sb, &iaddr); if (block == BEFS_ERR) -- 1.9.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/4] befs: remove unnecessary initialization 2016-08-10 22:12 [PATCH 1/4] befs: check allocation_group number before use Salah Triki 2016-08-10 22:12 ` [PATCH 2/4] befs: check return value of iaddr2blockno Salah Triki 2016-08-10 22:12 ` [PATCH 3/4] befs: remove the validation of allocation group number Salah Triki @ 2016-08-10 22:12 ` Salah Triki 2016-08-11 11:07 ` Luis de Bethencourt 2016-08-11 10:56 ` [PATCH 1/4] befs: check allocation_group number before use Luis de Bethencourt 3 siblings, 1 reply; 9+ messages in thread From: Salah Triki @ 2016-08-10 22:12 UTC (permalink / raw) To: akpm, viro, luisbg; +Cc: linux-kernel, salah.triki There is no need to init block, since it will be overwitten later by iaddr2blockno(). Signed-off-by: Salah Triki <salah.triki@gmail.com> --- fs/befs/io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/befs/io.c b/fs/befs/io.c index f9790f5..39d0dce 100644 --- a/fs/befs/io.c +++ b/fs/befs/io.c @@ -27,7 +27,7 @@ struct buffer_head * befs_bread_iaddr(struct super_block *sb, befs_inode_addr iaddr) { struct buffer_head *bh; - befs_blocknr_t block = 0; + befs_blocknr_t block; befs_debug(sb, "---> Enter %s " "[%u, %hu, %hu]", __func__, iaddr.allocation_group, -- 1.9.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 4/4] befs: remove unnecessary initialization 2016-08-10 22:12 ` [PATCH 4/4] befs: remove unnecessary initialization Salah Triki @ 2016-08-11 11:07 ` Luis de Bethencourt 0 siblings, 0 replies; 9+ messages in thread From: Luis de Bethencourt @ 2016-08-11 11:07 UTC (permalink / raw) To: Salah Triki, akpm, viro; +Cc: linux-kernel On 10/08/16 23:12, Salah Triki wrote: > There is no need to init block, since it will be overwitten later by > iaddr2blockno(). > > Signed-off-by: Salah Triki <salah.triki@gmail.com> > --- > fs/befs/io.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/befs/io.c b/fs/befs/io.c > index f9790f5..39d0dce 100644 > --- a/fs/befs/io.c > +++ b/fs/befs/io.c > @@ -27,7 +27,7 @@ struct buffer_head * > befs_bread_iaddr(struct super_block *sb, befs_inode_addr iaddr) > { > struct buffer_head *bh; > - befs_blocknr_t block = 0; > + befs_blocknr_t block; > > befs_debug(sb, "---> Enter %s " > "[%u, %hu, %hu]", __func__, iaddr.allocation_group, > Applied and merged to: https://github.com/luisbg/linux-befs/commits/for-next Thanks Salah :) Luis ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] befs: check allocation_group number before use 2016-08-10 22:12 [PATCH 1/4] befs: check allocation_group number before use Salah Triki ` (2 preceding siblings ...) 2016-08-10 22:12 ` [PATCH 4/4] befs: remove unnecessary initialization Salah Triki @ 2016-08-11 10:56 ` Luis de Bethencourt 2016-08-12 7:26 ` Salah Triki 3 siblings, 1 reply; 9+ messages in thread From: Luis de Bethencourt @ 2016-08-11 10:56 UTC (permalink / raw) To: Salah Triki, akpm, viro; +Cc: linux-kernel On 10/08/16 23:12, Salah Triki wrote: > Check that the allocation group number is not greater or equal to the > number of allocations group in the file system and return BEF_ERR in the > case of error. > > Signed-off-by: Salah Triki <salah.triki@gmail.com> > --- > fs/befs/befs.h | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/fs/befs/befs.h b/fs/befs/befs.h > index 55f3ea2..6daf4c4 100644 > --- a/fs/befs/befs.h > +++ b/fs/befs/befs.h > @@ -121,6 +121,11 @@ BEFS_I(const struct inode *inode) > static inline befs_blocknr_t > iaddr2blockno(struct super_block *sb, const befs_inode_addr *iaddr) > { > + if (iaddr->allocation_group >= BEFS_SB(sb)->num_ags) { > + befs_error(sb, "BEFS: Invalid allocation group %u, max is %u", > + iaddr->allocation_group, BEFS_SB(sb)->num_ags); > + return BEFS_ERR; > + } > return ((iaddr->allocation_group << BEFS_SB(sb)->ag_shift) + > iaddr->start); > } > Hi Salah, To understand why would we check for this. When can this error happen? I mean, when can iaddr2blockno() be called with an out of bounds allocation group? Thanks, Luis ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] befs: check allocation_group number before use 2016-08-11 10:56 ` [PATCH 1/4] befs: check allocation_group number before use Luis de Bethencourt @ 2016-08-12 7:26 ` Salah Triki 2016-08-12 9:38 ` Luis de Bethencourt 0 siblings, 1 reply; 9+ messages in thread From: Salah Triki @ 2016-08-12 7:26 UTC (permalink / raw) To: Luis de Bethencourt; +Cc: Salah Triki, akpm, viro, linux-kernel On Thu, Aug 11, 2016 at 11:56:16AM +0100, Luis de Bethencourt wrote: > On 10/08/16 23:12, Salah Triki wrote: > > Check that the allocation group number is not greater or equal to the > > number of allocations group in the file system and return BEF_ERR in the > > case of error. > > > > Signed-off-by: Salah Triki <salah.triki@gmail.com> > > --- > > fs/befs/befs.h | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/fs/befs/befs.h b/fs/befs/befs.h > > index 55f3ea2..6daf4c4 100644 > > --- a/fs/befs/befs.h > > +++ b/fs/befs/befs.h > > @@ -121,6 +121,11 @@ BEFS_I(const struct inode *inode) > > static inline befs_blocknr_t > > iaddr2blockno(struct super_block *sb, const befs_inode_addr *iaddr) > > { > > + if (iaddr->allocation_group >= BEFS_SB(sb)->num_ags) { > > + befs_error(sb, "BEFS: Invalid allocation group %u, max is %u", > > + iaddr->allocation_group, BEFS_SB(sb)->num_ags); > > + return BEFS_ERR; > > + } > > return ((iaddr->allocation_group << BEFS_SB(sb)->ag_shift) + > > iaddr->start); > > } > > > > Hi Salah, > > To understand why would we check for this. When can this error happen? I mean, > when can iaddr2blockno() be called with an out of bounds allocation group? > > Thanks, > Luis Hi Luis, allocation group number is set to blockno >> BEFS_SB(sb)->ag_shift by blockno2iaddr(), so if ag_shift is not valid, an out of bound may occur. By now, thanx to your question ;), I think it's better to check the validity of ag_shift when the superblock is loaded. Nack. Thanx for the question :) Salah ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] befs: check allocation_group number before use 2016-08-12 7:26 ` Salah Triki @ 2016-08-12 9:38 ` Luis de Bethencourt 2016-08-12 10:17 ` Salah Triki 0 siblings, 1 reply; 9+ messages in thread From: Luis de Bethencourt @ 2016-08-12 9:38 UTC (permalink / raw) To: Salah Triki; +Cc: akpm, viro, linux-kernel On 12/08/16 08:26, Salah Triki wrote: > On Thu, Aug 11, 2016 at 11:56:16AM +0100, Luis de Bethencourt wrote: >> On 10/08/16 23:12, Salah Triki wrote: >>> Check that the allocation group number is not greater or equal to the >>> number of allocations group in the file system and return BEF_ERR in the >>> case of error. >>> >>> Signed-off-by: Salah Triki <salah.triki@gmail.com> >>> --- >>> fs/befs/befs.h | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/fs/befs/befs.h b/fs/befs/befs.h >>> index 55f3ea2..6daf4c4 100644 >>> --- a/fs/befs/befs.h >>> +++ b/fs/befs/befs.h >>> @@ -121,6 +121,11 @@ BEFS_I(const struct inode *inode) >>> static inline befs_blocknr_t >>> iaddr2blockno(struct super_block *sb, const befs_inode_addr *iaddr) >>> { >>> + if (iaddr->allocation_group >= BEFS_SB(sb)->num_ags) { >>> + befs_error(sb, "BEFS: Invalid allocation group %u, max is %u", >>> + iaddr->allocation_group, BEFS_SB(sb)->num_ags); >>> + return BEFS_ERR; >>> + } >>> return ((iaddr->allocation_group << BEFS_SB(sb)->ag_shift) + >>> iaddr->start); >>> } >>> >> >> Hi Salah, >> >> To understand why would we check for this. When can this error happen? I mean, >> when can iaddr2blockno() be called with an out of bounds allocation group? >> >> Thanks, >> Luis > > Hi Luis, > > allocation group number is set to blockno >> BEFS_SB(sb)->ag_shift by > blockno2iaddr(), so if ag_shift is not valid, an out of bound may occur. > By now, thanx to your question ;), I think it's better to check the validity > of ag_shift when the superblock is loaded. > > Nack. > > Thanx for the question :) > Salah > No problem :) I will assume the Nack covers patch 1 to 3. Since they are all related. Enjoy the weekend, Luis ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] befs: check allocation_group number before use 2016-08-12 9:38 ` Luis de Bethencourt @ 2016-08-12 10:17 ` Salah Triki 0 siblings, 0 replies; 9+ messages in thread From: Salah Triki @ 2016-08-12 10:17 UTC (permalink / raw) To: Luis de Bethencourt; +Cc: akpm, viro, linux-kernel On Fri, Aug 12, 2016 at 10:38:58AM +0100, Luis de Bethencourt wrote: > On 12/08/16 08:26, Salah Triki wrote: > > On Thu, Aug 11, 2016 at 11:56:16AM +0100, Luis de Bethencourt wrote: > >> On 10/08/16 23:12, Salah Triki wrote: > >>> Check that the allocation group number is not greater or equal to the > >>> number of allocations group in the file system and return BEF_ERR in the > >>> case of error. > >>> > >>> Signed-off-by: Salah Triki <salah.triki@gmail.com> > >>> --- > >>> fs/befs/befs.h | 5 +++++ > >>> 1 file changed, 5 insertions(+) > >>> > >>> diff --git a/fs/befs/befs.h b/fs/befs/befs.h > >>> index 55f3ea2..6daf4c4 100644 > >>> --- a/fs/befs/befs.h > >>> +++ b/fs/befs/befs.h > >>> @@ -121,6 +121,11 @@ BEFS_I(const struct inode *inode) > >>> static inline befs_blocknr_t > >>> iaddr2blockno(struct super_block *sb, const befs_inode_addr *iaddr) > >>> { > >>> + if (iaddr->allocation_group >= BEFS_SB(sb)->num_ags) { > >>> + befs_error(sb, "BEFS: Invalid allocation group %u, max is %u", > >>> + iaddr->allocation_group, BEFS_SB(sb)->num_ags); > >>> + return BEFS_ERR; > >>> + } > >>> return ((iaddr->allocation_group << BEFS_SB(sb)->ag_shift) + > >>> iaddr->start); > >>> } > >>> > >> > >> Hi Salah, > >> > >> To understand why would we check for this. When can this error happen? I mean, > >> when can iaddr2blockno() be called with an out of bounds allocation group? > >> > >> Thanks, > >> Luis > > > > Hi Luis, > > > > allocation group number is set to blockno >> BEFS_SB(sb)->ag_shift by > > blockno2iaddr(), so if ag_shift is not valid, an out of bound may occur. > > By now, thanx to your question ;), I think it's better to check the validity > > of ag_shift when the superblock is loaded. > > > > Nack. > > > > Thanx for the question :) > > Salah > > > > No problem :) > > I will assume the Nack covers patch 1 to 3. Since they are all related. > > Enjoy the weekend, > Luis You too. Salah ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-08-12 10:17 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-08-10 22:12 [PATCH 1/4] befs: check allocation_group number before use Salah Triki 2016-08-10 22:12 ` [PATCH 2/4] befs: check return value of iaddr2blockno Salah Triki 2016-08-10 22:12 ` [PATCH 3/4] befs: remove the validation of allocation group number Salah Triki 2016-08-10 22:12 ` [PATCH 4/4] befs: remove unnecessary initialization Salah Triki 2016-08-11 11:07 ` Luis de Bethencourt 2016-08-11 10:56 ` [PATCH 1/4] befs: check allocation_group number before use Luis de Bethencourt 2016-08-12 7:26 ` Salah Triki 2016-08-12 9:38 ` Luis de Bethencourt 2016-08-12 10:17 ` Salah Triki
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.