Messages in this thread | | | Date | Tue, 17 Oct 2023 17:35:30 +0530 | Subject | Re: [PATCH] jfs: fix array-index-out-of-bounds in dbAdjTree | From | Manas Ghandat <> |
| |
Just a friendly remainder
On 17/10/23 17:33, Manas Ghandat wrote: > Currently there is a bound check missing in the dbAdjTree while > accessing the dmt_stree. To add the required check added the bool is_ctl > which is required to determine the size as suggest in the following > commit. > https://lore.kernel.org/linux-kernel-mentees/f9475918-2186-49b8-b801-6f0f9e75f4fa@oracle.com/ > > Reported-by: syzbot+39ba34a099ac2e9bd3cb@syzkaller.appspotmail.com > Fixes: https://syzkaller.appspot.com/bug?extid=39ba34a099ac2e9bd3cb > Signed-off-by: Manas Ghandat <ghandatmanas@gmail.com> > --- > fs/jfs/jfs_dmap.c | 57 ++++++++++++++++++++++++++--------------------- > 1 file changed, 31 insertions(+), 26 deletions(-) > > diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c > index 88afd108c2dd..57fba21994c3 100644 > --- a/fs/jfs/jfs_dmap.c > +++ b/fs/jfs/jfs_dmap.c > @@ -63,10 +63,10 @@ > */ > static void dbAllocBits(struct bmap * bmp, struct dmap * dp, s64 blkno, > int nblocks); > -static void dbSplit(dmtree_t * tp, int leafno, int splitsz, int newval); > -static int dbBackSplit(dmtree_t * tp, int leafno); > -static int dbJoin(dmtree_t * tp, int leafno, int newval); > -static void dbAdjTree(dmtree_t * tp, int leafno, int newval); > +static void dbSplit(dmtree_t * tp, int leafno, int splitsz, int newval, bool is_ctl); > +static int dbBackSplit(dmtree_t * tp, int leafno, bool is_ctl); > +static int dbJoin(dmtree_t * tp, int leafno, int newval, bool is_ctl); > +static void dbAdjTree(dmtree_t * tp, int leafno, int newval, bool is_ctl); > static int dbAdjCtl(struct bmap * bmp, s64 blkno, int newval, int alloc, > int level); > static int dbAllocAny(struct bmap * bmp, s64 nblocks, int l2nb, s64 * results); > @@ -2096,7 +2096,7 @@ static int dbFreeDmap(struct bmap * bmp, struct dmap * dp, s64 blkno, > * system. > */ > if (dp->tree.stree[word] == NOFREE) > - dbBackSplit((dmtree_t *) & dp->tree, word); > + dbBackSplit((dmtree_t *) & dp->tree, word, false); > > dbAllocBits(bmp, dp, blkno, nblocks); > } > @@ -2182,7 +2182,7 @@ static void dbAllocBits(struct bmap * bmp, struct dmap * dp, s64 blkno, > * the binary system of the leaves if need be. > */ > dbSplit(tp, word, BUDMIN, > - dbMaxBud((u8 *) & dp->wmap[word])); > + dbMaxBud((u8 *) & dp->wmap[word]),false); > > word += 1; > } else { > @@ -2222,7 +2222,7 @@ static void dbAllocBits(struct bmap * bmp, struct dmap * dp, s64 blkno, > * system of the leaves to reflect the current > * allocation (size). > */ > - dbSplit(tp, word, size, NOFREE); > + dbSplit(tp, word, size, NOFREE, false); > > /* get the number of dmap words handled */ > nw = BUDSIZE(size, BUDMIN); > @@ -2329,7 +2329,7 @@ static int dbFreeBits(struct bmap * bmp, struct dmap * dp, s64 blkno, > /* update the leaf for this dmap word. > */ > rc = dbJoin(tp, word, > - dbMaxBud((u8 *) & dp->wmap[word])); > + dbMaxBud((u8 *) & dp->wmap[word]),false); > if (rc) > return rc; > > @@ -2362,7 +2362,7 @@ static int dbFreeBits(struct bmap * bmp, struct dmap * dp, s64 blkno, > > /* update the leaf. > */ > - rc = dbJoin(tp, word, size); > + rc = dbJoin(tp, word, size, false); > if (rc) > return rc; > > @@ -2514,16 +2514,16 @@ dbAdjCtl(struct bmap * bmp, s64 blkno, int newval, int alloc, int level) > * that it is at the front of a binary buddy system. > */ > if (oldval == NOFREE) { > - rc = dbBackSplit((dmtree_t *) dcp, leafno); > + rc = dbBackSplit((dmtree_t *) dcp, leafno, true); > if (rc) { > release_metapage(mp); > return rc; > } > oldval = dcp->stree[ti]; > } > - dbSplit((dmtree_t *) dcp, leafno, dcp->budmin, newval); > + dbSplit((dmtree_t *) dcp, leafno, dcp->budmin, newval, true); > } else { > - rc = dbJoin((dmtree_t *) dcp, leafno, newval); > + rc = dbJoin((dmtree_t *) dcp, leafno, newval, true); > if (rc) { > release_metapage(mp); > return rc; > @@ -2554,7 +2554,7 @@ dbAdjCtl(struct bmap * bmp, s64 blkno, int newval, int alloc, int level) > */ > if (alloc) { > dbJoin((dmtree_t *) dcp, leafno, > - oldval); > + oldval, true); > } else { > /* the dbJoin() above might have > * caused a larger binary buddy system > @@ -2564,9 +2564,9 @@ dbAdjCtl(struct bmap * bmp, s64 blkno, int newval, int alloc, int level) > */ > if (dcp->stree[ti] == NOFREE) > dbBackSplit((dmtree_t *) > - dcp, leafno); > + dcp, leafno, true); > dbSplit((dmtree_t *) dcp, leafno, > - dcp->budmin, oldval); > + dcp->budmin, oldval, true); > } > > /* release the buffer and return the error. > @@ -2614,7 +2614,7 @@ dbAdjCtl(struct bmap * bmp, s64 blkno, int newval, int alloc, int level) > * > * serialization: IREAD_LOCK(ipbmap) or IWRITE_LOCK(ipbmap) held on entry/exit; > */ > -static void dbSplit(dmtree_t * tp, int leafno, int splitsz, int newval) > +static void dbSplit(dmtree_t * tp, int leafno, int splitsz, int newval, bool is_ctl) > { > int budsz; > int cursz; > @@ -2636,7 +2636,7 @@ static void dbSplit(dmtree_t * tp, int leafno, int splitsz, int newval) > while (cursz >= splitsz) { > /* update the buddy's leaf with its new value. > */ > - dbAdjTree(tp, leafno ^ budsz, cursz); > + dbAdjTree(tp, leafno ^ budsz, cursz, is_ctl); > > /* on to the next size and buddy. > */ > @@ -2648,7 +2648,7 @@ static void dbSplit(dmtree_t * tp, int leafno, int splitsz, int newval) > /* adjust the dmap tree to reflect the specified leaf's new > * value. > */ > - dbAdjTree(tp, leafno, newval); > + dbAdjTree(tp, leafno, newval, is_ctl); > } > > > @@ -2679,7 +2679,7 @@ static void dbSplit(dmtree_t * tp, int leafno, int splitsz, int newval) > * > * serialization: IREAD_LOCK(ipbmap) or IWRITE_LOCK(ipbmap) held on entry/exit; > */ > -static int dbBackSplit(dmtree_t * tp, int leafno) > +static int dbBackSplit(dmtree_t * tp, int leafno, bool is_ctl) > { > int budsz, bud, w, bsz, size; > int cursz; > @@ -2730,7 +2730,7 @@ static int dbBackSplit(dmtree_t * tp, int leafno) > * system in two. > */ > cursz = leaf[bud] - 1; > - dbSplit(tp, bud, cursz, cursz); > + dbSplit(tp, bud, cursz, cursz, is_ctl); > break; > } > } > @@ -2758,7 +2758,7 @@ static int dbBackSplit(dmtree_t * tp, int leafno) > * > * RETURN VALUES: none > */ > -static int dbJoin(dmtree_t * tp, int leafno, int newval) > +static int dbJoin(dmtree_t * tp, int leafno, int newval, bool is_ctl) > { > int budsz, buddy; > s8 *leaf; > @@ -2813,12 +2813,12 @@ static int dbJoin(dmtree_t * tp, int leafno, int newval) > if (leafno < buddy) { > /* leafno is the left buddy. > */ > - dbAdjTree(tp, buddy, NOFREE); > + dbAdjTree(tp, buddy, NOFREE, is_ctl); > } else { > /* buddy is the left buddy and becomes > * leafno. > */ > - dbAdjTree(tp, leafno, NOFREE); > + dbAdjTree(tp, leafno, NOFREE, is_ctl); > leafno = buddy; > } > > @@ -2831,7 +2831,7 @@ static int dbJoin(dmtree_t * tp, int leafno, int newval) > > /* update the leaf value. > */ > - dbAdjTree(tp, leafno, newval); > + dbAdjTree(tp, leafno, newval, is_ctl); > > return 0; > } > @@ -2852,15 +2852,20 @@ static int dbJoin(dmtree_t * tp, int leafno, int newval) > * > * RETURN VALUES: none > */ > -static void dbAdjTree(dmtree_t * tp, int leafno, int newval) > +static void dbAdjTree(dmtree_t * tp, int leafno, int newval, bool is_ctl) > { > int lp, pp, k; > - int max; > + int max, size; > + > + size = is_ctl ? CTLTREESIZE : TREESIZE; > > /* pick up the index of the leaf for this leafno. > */ > lp = leafno + le32_to_cpu(tp->dmt_leafidx); > > + if (lp > size || lp < 0) > + return; > + > /* is the current value the same as the old value ? if so, > * there is nothing to do. > */
| |