Re: [PATCH] jfs: fix slab-out-of-bounds Read in dtSearch

From: kernel test robot
Date: Tue Oct 24 2023 - 08:16:03 EST


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/202310241933.iekefgtT-lkp@xxxxxxxxx/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/202310241933.iekefgtT-lkp@xxxxxxxxx/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 <lkp@xxxxxxxxx>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310241933.iekefgtT-lkp@xxxxxxxxx/

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