Currently while searching for current page in the sorted entry table
of the page there is a out of bound access. Added a bound check to fix
the error.
Reported-by: [email protected]
Fixes: https://syzkaller.appspot.com/bug?extid=9924e2a08d9ba0fd4ce2
Signed-off-by: Manas Ghandat <[email protected]>
---
fs/jfs/jfs_dtree.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/jfs/jfs_dtree.c b/fs/jfs/jfs_dtree.c
index 92b7c533407c..cf67d32d5b7f 100644
--- a/fs/jfs/jfs_dtree.c
+++ b/fs/jfs/jfs_dtree.c
@@ -633,6 +633,9 @@ int dtSearch(struct inode *ip, struct component_name * key, ino_t * data,
for (base = 0, lim = p->header.nextindex; lim; lim >>= 1) {
index = base + (lim >> 1);
+ if (stbl[index] > 128 || stbl[index] < 0)
+ goto out;
+
if (p->header.flag & BT_LEAF) {
/* uppercase leaf name to compare */
cmp =
--
2.37.2
Hi Manas,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Manas-Ghandat/jfs-fix-slab-out-of-bounds-Read-in-dtSearch/20231017-152500
base: https://github.com/kleikamp/linux-shaggy jfs-next
patch link: https://lore.kernel.org/r/20231016171130.15952-1-ghandatmanas%40gmail.com
patch subject: [PATCH] jfs: fix slab-out-of-bounds Read in dtSearch
config: i386-randconfig-141-20231022 (https://download.01.org/0day-ci/archive/20231024/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20231024/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Closes: https://lore.kernel.org/r/[email protected]/
smatch warnings:
fs/jfs/jfs_dtree.c:636 dtSearch() warn: impossible condition '(stbl[index] > 128) => ((-128)-127 > 128)'
vim +636 fs/jfs/jfs_dtree.c
^1da177e4c3f41 Linus Torvalds 2005-04-16 567 int dtSearch(struct inode *ip, struct component_name * key, ino_t * data,
^1da177e4c3f41 Linus Torvalds 2005-04-16 568 struct btstack * btstack, int flag)
^1da177e4c3f41 Linus Torvalds 2005-04-16 569 {
^1da177e4c3f41 Linus Torvalds 2005-04-16 570 int rc = 0;
^1da177e4c3f41 Linus Torvalds 2005-04-16 571 int cmp = 1; /* init for empty page */
^1da177e4c3f41 Linus Torvalds 2005-04-16 572 s64 bn;
^1da177e4c3f41 Linus Torvalds 2005-04-16 573 struct metapage *mp;
^1da177e4c3f41 Linus Torvalds 2005-04-16 574 dtpage_t *p;
^1da177e4c3f41 Linus Torvalds 2005-04-16 575 s8 *stbl;
^^^^^^^^
^1da177e4c3f41 Linus Torvalds 2005-04-16 576 int base, index, lim;
^1da177e4c3f41 Linus Torvalds 2005-04-16 577 struct btframe *btsp;
^1da177e4c3f41 Linus Torvalds 2005-04-16 578 pxd_t *pxd;
^1da177e4c3f41 Linus Torvalds 2005-04-16 579 int psize = 288; /* initial in-line directory */
^1da177e4c3f41 Linus Torvalds 2005-04-16 580 ino_t inumber;
^1da177e4c3f41 Linus Torvalds 2005-04-16 581 struct component_name ciKey;
^1da177e4c3f41 Linus Torvalds 2005-04-16 582 struct super_block *sb = ip->i_sb;
^1da177e4c3f41 Linus Torvalds 2005-04-16 583
6da2ec56059c3c Kees Cook 2018-06-12 584 ciKey.name = kmalloc_array(JFS_NAME_MAX + 1, sizeof(wchar_t),
6da2ec56059c3c Kees Cook 2018-06-12 585 GFP_NOFS);
09aaa749f637b1 Joe Perches 2007-11-13 586 if (!ciKey.name) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 587 rc = -ENOMEM;
^1da177e4c3f41 Linus Torvalds 2005-04-16 588 goto dtSearch_Exit2;
^1da177e4c3f41 Linus Torvalds 2005-04-16 589 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 590
^1da177e4c3f41 Linus Torvalds 2005-04-16 591
^1da177e4c3f41 Linus Torvalds 2005-04-16 592 /* uppercase search key for c-i directory */
^1da177e4c3f41 Linus Torvalds 2005-04-16 593 UniStrcpy(ciKey.name, key->name);
^1da177e4c3f41 Linus Torvalds 2005-04-16 594 ciKey.namlen = key->namlen;
^1da177e4c3f41 Linus Torvalds 2005-04-16 595
^1da177e4c3f41 Linus Torvalds 2005-04-16 596 /* only uppercase if case-insensitive support is on */
^1da177e4c3f41 Linus Torvalds 2005-04-16 597 if ((JFS_SBI(sb)->mntflag & JFS_OS2) == JFS_OS2) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 598 ciToUpper(&ciKey);
^1da177e4c3f41 Linus Torvalds 2005-04-16 599 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 600 BT_CLR(btstack); /* reset stack */
^1da177e4c3f41 Linus Torvalds 2005-04-16 601
^1da177e4c3f41 Linus Torvalds 2005-04-16 602 /* init level count for max pages to split */
^1da177e4c3f41 Linus Torvalds 2005-04-16 603 btstack->nsplit = 1;
^1da177e4c3f41 Linus Torvalds 2005-04-16 604
^1da177e4c3f41 Linus Torvalds 2005-04-16 605 /*
^1da177e4c3f41 Linus Torvalds 2005-04-16 606 * search down tree from root:
^1da177e4c3f41 Linus Torvalds 2005-04-16 607 *
^1da177e4c3f41 Linus Torvalds 2005-04-16 608 * between two consecutive entries of <Ki, Pi> and <Kj, Pj> of
^1da177e4c3f41 Linus Torvalds 2005-04-16 609 * internal page, child page Pi contains entry with k, Ki <= K < Kj.
^1da177e4c3f41 Linus Torvalds 2005-04-16 610 *
^1da177e4c3f41 Linus Torvalds 2005-04-16 611 * if entry with search key K is not found
^1da177e4c3f41 Linus Torvalds 2005-04-16 612 * internal page search find the entry with largest key Ki
^1da177e4c3f41 Linus Torvalds 2005-04-16 613 * less than K which point to the child page to search;
^1da177e4c3f41 Linus Torvalds 2005-04-16 614 * leaf page search find the entry with smallest key Kj
^1da177e4c3f41 Linus Torvalds 2005-04-16 615 * greater than K so that the returned index is the position of
^1da177e4c3f41 Linus Torvalds 2005-04-16 616 * the entry to be shifted right for insertion of new entry.
^1da177e4c3f41 Linus Torvalds 2005-04-16 617 * for empty tree, search key is greater than any key of the tree.
^1da177e4c3f41 Linus Torvalds 2005-04-16 618 *
^1da177e4c3f41 Linus Torvalds 2005-04-16 619 * by convention, root bn = 0.
^1da177e4c3f41 Linus Torvalds 2005-04-16 620 */
^1da177e4c3f41 Linus Torvalds 2005-04-16 621 for (bn = 0;;) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 622 /* get/pin the page to search */
^1da177e4c3f41 Linus Torvalds 2005-04-16 623 DT_GETPAGE(ip, bn, mp, psize, p, rc);
^1da177e4c3f41 Linus Torvalds 2005-04-16 624 if (rc)
^1da177e4c3f41 Linus Torvalds 2005-04-16 625 goto dtSearch_Exit1;
^1da177e4c3f41 Linus Torvalds 2005-04-16 626
^1da177e4c3f41 Linus Torvalds 2005-04-16 627 /* get sorted entry table of the page */
^1da177e4c3f41 Linus Torvalds 2005-04-16 628 stbl = DT_GETSTBL(p);
^1da177e4c3f41 Linus Torvalds 2005-04-16 629
^1da177e4c3f41 Linus Torvalds 2005-04-16 630 /*
^1da177e4c3f41 Linus Torvalds 2005-04-16 631 * binary search with search key K on the current page.
^1da177e4c3f41 Linus Torvalds 2005-04-16 632 */
^1da177e4c3f41 Linus Torvalds 2005-04-16 633 for (base = 0, lim = p->header.nextindex; lim; lim >>= 1) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 634 index = base + (lim >> 1);
^1da177e4c3f41 Linus Torvalds 2005-04-16 635
7812e358b5edde Manas Ghandat 2023-10-16 @636 if (stbl[index] > 128 || stbl[index] < 0)
If it's stbl is an s8 so it can't be > 128.
7812e358b5edde Manas Ghandat 2023-10-16 637 goto out;
7812e358b5edde Manas Ghandat 2023-10-16 638
^1da177e4c3f41 Linus Torvalds 2005-04-16 639 if (p->header.flag & BT_LEAF) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 640 /* uppercase leaf name to compare */
^1da177e4c3f41 Linus Torvalds 2005-04-16 641 cmp =
^1da177e4c3f41 Linus Torvalds 2005-04-16 642 ciCompare(&ciKey, p, stbl[index],
^1da177e4c3f41 Linus Torvalds 2005-04-16 643 JFS_SBI(sb)->mntflag);
^1da177e4c3f41 Linus Torvalds 2005-04-16 644 } else {
^1da177e4c3f41 Linus Torvalds 2005-04-16 645 /* router key is in uppercase */
^1da177e4c3f41 Linus Torvalds 2005-04-16 646
^1da177e4c3f41 Linus Torvalds 2005-04-16 647 cmp = dtCompare(&ciKey, p, stbl[index]);
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Manas,
kernel test robot noticed the following build warnings:
[auto build test WARNING on kleikamp-shaggy/jfs-next]
[also build test WARNING on linus/master v6.6-rc7 next-20231023]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Manas-Ghandat/jfs-fix-slab-out-of-bounds-Read-in-dtSearch/20231017-152500
base: https://github.com/kleikamp/linux-shaggy jfs-next
patch link: https://lore.kernel.org/r/20231016171130.15952-1-ghandatmanas%40gmail.com
patch subject: [PATCH] jfs: fix slab-out-of-bounds Read in dtSearch
config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20231024/[email protected]/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231024/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
>> fs/jfs/jfs_dtree.c:636:20: warning: result of comparison of constant 128 with expression of type 's8' (aka 'signed char') is always false [-Wtautological-constant-out-of-range-compare]
if (stbl[index] > 128 || stbl[index] < 0)
~~~~~~~~~~~ ^ ~~~
1 warning generated.
vim +636 fs/jfs/jfs_dtree.c
555
556 /*
557 * dtSearch()
558 *
559 * function:
560 * Search for the entry with specified key
561 *
562 * parameter:
563 *
564 * return: 0 - search result on stack, leaf page pinned;
565 * errno - I/O error
566 */
567 int dtSearch(struct inode *ip, struct component_name * key, ino_t * data,
568 struct btstack * btstack, int flag)
569 {
570 int rc = 0;
571 int cmp = 1; /* init for empty page */
572 s64 bn;
573 struct metapage *mp;
574 dtpage_t *p;
575 s8 *stbl;
576 int base, index, lim;
577 struct btframe *btsp;
578 pxd_t *pxd;
579 int psize = 288; /* initial in-line directory */
580 ino_t inumber;
581 struct component_name ciKey;
582 struct super_block *sb = ip->i_sb;
583
584 ciKey.name = kmalloc_array(JFS_NAME_MAX + 1, sizeof(wchar_t),
585 GFP_NOFS);
586 if (!ciKey.name) {
587 rc = -ENOMEM;
588 goto dtSearch_Exit2;
589 }
590
591
592 /* uppercase search key for c-i directory */
593 UniStrcpy(ciKey.name, key->name);
594 ciKey.namlen = key->namlen;
595
596 /* only uppercase if case-insensitive support is on */
597 if ((JFS_SBI(sb)->mntflag & JFS_OS2) == JFS_OS2) {
598 ciToUpper(&ciKey);
599 }
600 BT_CLR(btstack); /* reset stack */
601
602 /* init level count for max pages to split */
603 btstack->nsplit = 1;
604
605 /*
606 * search down tree from root:
607 *
608 * between two consecutive entries of <Ki, Pi> and <Kj, Pj> of
609 * internal page, child page Pi contains entry with k, Ki <= K < Kj.
610 *
611 * if entry with search key K is not found
612 * internal page search find the entry with largest key Ki
613 * less than K which point to the child page to search;
614 * leaf page search find the entry with smallest key Kj
615 * greater than K so that the returned index is the position of
616 * the entry to be shifted right for insertion of new entry.
617 * for empty tree, search key is greater than any key of the tree.
618 *
619 * by convention, root bn = 0.
620 */
621 for (bn = 0;;) {
622 /* get/pin the page to search */
623 DT_GETPAGE(ip, bn, mp, psize, p, rc);
624 if (rc)
625 goto dtSearch_Exit1;
626
627 /* get sorted entry table of the page */
628 stbl = DT_GETSTBL(p);
629
630 /*
631 * binary search with search key K on the current page.
632 */
633 for (base = 0, lim = p->header.nextindex; lim; lim >>= 1) {
634 index = base + (lim >> 1);
635
> 636 if (stbl[index] > 128 || stbl[index] < 0)
637 goto out;
638
639 if (p->header.flag & BT_LEAF) {
640 /* uppercase leaf name to compare */
641 cmp =
642 ciCompare(&ciKey, p, stbl[index],
643 JFS_SBI(sb)->mntflag);
644 } else {
645 /* router key is in uppercase */
646
647 cmp = dtCompare(&ciKey, p, stbl[index]);
648
649
650 }
651 if (cmp == 0) {
652 /*
653 * search hit
654 */
655 /* search hit - leaf page:
656 * return the entry found
657 */
658 if (p->header.flag & BT_LEAF) {
659 inumber = le32_to_cpu(
660 ((struct ldtentry *) & p->slot[stbl[index]])->inumber);
661
662 /*
663 * search for JFS_LOOKUP
664 */
665 if (flag == JFS_LOOKUP) {
666 *data = inumber;
667 rc = 0;
668 goto out;
669 }
670
671 /*
672 * search for JFS_CREATE
673 */
674 if (flag == JFS_CREATE) {
675 *data = inumber;
676 rc = -EEXIST;
677 goto out;
678 }
679
680 /*
681 * search for JFS_REMOVE or JFS_RENAME
682 */
683 if ((flag == JFS_REMOVE ||
684 flag == JFS_RENAME) &&
685 *data != inumber) {
686 rc = -ESTALE;
687 goto out;
688 }
689
690 /*
691 * JFS_REMOVE|JFS_FINDDIR|JFS_RENAME
692 */
693 /* save search result */
694 *data = inumber;
695 btsp = btstack->top;
696 btsp->bn = bn;
697 btsp->index = index;
698 btsp->mp = mp;
699
700 rc = 0;
701 goto dtSearch_Exit1;
702 }
703
704 /* search hit - internal page:
705 * descend/search its child page
706 */
707 goto getChild;
708 }
709
710 if (cmp > 0) {
711 base = index + 1;
712 --lim;
713 }
714 }
715
716 /*
717 * search miss
718 *
719 * base is the smallest index with key (Kj) greater than
720 * search key (K) and may be zero or (maxindex + 1) index.
721 */
722 /*
723 * search miss - leaf page
724 *
725 * return location of entry (base) where new entry with
726 * search key K is to be inserted.
727 */
728 if (p->header.flag & BT_LEAF) {
729 /*
730 * search for JFS_LOOKUP, JFS_REMOVE, or JFS_RENAME
731 */
732 if (flag == JFS_LOOKUP || flag == JFS_REMOVE ||
733 flag == JFS_RENAME) {
734 rc = -ENOENT;
735 goto out;
736 }
737
738 /*
739 * search for JFS_CREATE|JFS_FINDDIR:
740 *
741 * save search result
742 */
743 *data = 0;
744 btsp = btstack->top;
745 btsp->bn = bn;
746 btsp->index = base;
747 btsp->mp = mp;
748
749 rc = 0;
750 goto dtSearch_Exit1;
751 }
752
753 /*
754 * search miss - internal page
755 *
756 * if base is non-zero, decrement base by one to get the parent
757 * entry of the child page to search.
758 */
759 index = base ? base - 1 : base;
760
761 /*
762 * go down to child page
763 */
764 getChild:
765 /* update max. number of pages to split */
766 if (BT_STACK_FULL(btstack)) {
767 /* Something's corrupted, mark filesystem dirty so
768 * chkdsk will fix it.
769 */
770 jfs_error(sb, "stack overrun!\n");
771 BT_STACK_DUMP(btstack);
772 rc = -EIO;
773 goto out;
774 }
775 btstack->nsplit++;
776
777 /* push (bn, index) of the parent page/entry */
778 BT_PUSH(btstack, bn, index);
779
780 /* get the child page block number */
781 pxd = (pxd_t *) & p->slot[stbl[index]];
782 bn = addressPXD(pxd);
783 psize = lengthPXD(pxd) << JFS_SBI(ip->i_sb)->l2bsize;
784
785 /* unpin the parent page */
786 DT_PUTPAGE(mp);
787 }
788
789 out:
790 DT_PUTPAGE(mp);
791
792 dtSearch_Exit1:
793
794 kfree(ciKey.name);
795
796 dtSearch_Exit2:
797
798 return rc;
799 }
800
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki