Re: [PATCH] jfs: fix array-index-out-of-bounds in diNewExt

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/2/24 7:29AM, Dan Carpenter wrote:
Hi Edward,

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/Edward-Adam-Davis/jfs-fix-array-index-out-of-bounds-in-diNewExt/20231212-095530
base:   https://github.com/kleikamp/linux-shaggy jfs-next
patch link:    https://lore.kernel.org/r/tencent_B86ECD2ECECC92A7ED86EF92D0064A499206%40qq.com
patch subject: [PATCH] jfs: fix array-index-out-of-bounds in diNewExt
config: i386-randconfig-141-20231212 (https://download.01.org/0day-ci/archive/20231214/202312142348.6HRZtXTB-lkp@xxxxxxxxx/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce: (https://download.01.org/0day-ci/archive/20231214/202312142348.6HRZtXTB-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>
| Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
| Closes: https://lore.kernel.org/r/202312142348.6HRZtXTB-lkp@xxxxxxxxx/

I modified Edward's patch in the jfs-next branch with the corrected test.

Thanks for catching this.

Shaggy


New smatch warnings:
fs/jfs/jfs_imap.c:2213 diNewExt() error: buffer overflow 'imap->im_imap.in_agctl' 128 <= 128

Old smatch warnings:
fs/jfs/jfs_imap.c:2229 diNewExt() error: buffer overflow 'imap->im_imap.in_agctl' 128 <= 128
fs/jfs/jfs_imap.c:2304 diNewExt() error: buffer overflow 'imap->im_imap.in_agctl' 128 <= 128
fs/jfs/jfs_imap.c:2318 diNewExt() error: buffer overflow 'imap->im_imap.in_agctl' 128 <= 128
fs/jfs/jfs_imap.c:2330 diNewExt() error: buffer overflow 'imap->im_imap.in_agctl' 128 <= 128
fs/jfs/jfs_imap.c:2332 diNewExt() error: buffer overflow 'imap->im_imap.in_agctl' 128 <= 128
fs/jfs/jfs_imap.c:2363 diNewExt() error: buffer overflow 'imap->im_imap.in_agctl' 128 <= 128
fs/jfs/jfs_imap.c:2364 diNewExt() error: buffer overflow 'imap->im_imap.in_agctl' 128 <= 128

vim +2213 fs/jfs/jfs_imap.c

^1da177e4c3f41 Linus Torvalds    2005-04-16  2152  static int diNewExt(struct inomap * imap, struct iag * iagp, int extno)
^1da177e4c3f41 Linus Torvalds    2005-04-16  2153  {
^1da177e4c3f41 Linus Torvalds    2005-04-16  2154  	int agno, iagno, fwd, back, freei = 0, sword, rc;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2155  	struct iag *aiagp = NULL, *biagp = NULL, *ciagp = NULL;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2156  	struct metapage *amp, *bmp, *cmp, *dmp;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2157  	struct inode *ipimap;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2158  	s64 blkno, hint;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2159  	int i, j;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2160  	u32 mask;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2161  	ino_t ino;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2162  	struct dinode *dp;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2163  	struct jfs_sb_info *sbi;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2164
^1da177e4c3f41 Linus Torvalds    2005-04-16  2165  	/* better have free extents.
^1da177e4c3f41 Linus Torvalds    2005-04-16  2166  	 */
^1da177e4c3f41 Linus Torvalds    2005-04-16  2167  	if (!iagp->nfreeexts) {
eb8630d7d2fd13 Joe Perches       2013-06-04  2168  		jfs_error(imap->im_ipimap->i_sb, "no free extents\n");
^1da177e4c3f41 Linus Torvalds    2005-04-16  2169  		return -EIO;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2170  	}
^1da177e4c3f41 Linus Torvalds    2005-04-16  2171
^1da177e4c3f41 Linus Torvalds    2005-04-16  2172  	/* get the inode map inode.
^1da177e4c3f41 Linus Torvalds    2005-04-16  2173  	 */
^1da177e4c3f41 Linus Torvalds    2005-04-16  2174  	ipimap = imap->im_ipimap;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2175  	sbi = JFS_SBI(ipimap->i_sb);
^1da177e4c3f41 Linus Torvalds    2005-04-16  2176
^1da177e4c3f41 Linus Torvalds    2005-04-16  2177  	amp = bmp = cmp = NULL;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2178
^1da177e4c3f41 Linus Torvalds    2005-04-16  2179  	/* get the ag and iag numbers for this iag.
^1da177e4c3f41 Linus Torvalds    2005-04-16  2180  	 */
^1da177e4c3f41 Linus Torvalds    2005-04-16  2181  	agno = BLKTOAG(le64_to_cpu(iagp->agstart), sbi);
f93b91b82fcf16 Edward Adam Davis 2023-12-12  2182  	if (agno > MAXAG || agno < 0)

The commit introduces this agno > MAXAG comparison.  But Smatch says
that it should be agno >= MAXAG.

f93b91b82fcf16 Edward Adam Davis 2023-12-12  2183  		return -EIO;
f93b91b82fcf16 Edward Adam Davis 2023-12-12  2184
^1da177e4c3f41 Linus Torvalds    2005-04-16  2185  	iagno = le32_to_cpu(iagp->iagnum);
^1da177e4c3f41 Linus Torvalds    2005-04-16  2186
^1da177e4c3f41 Linus Torvalds    2005-04-16  2187  	/* check if this is the last free extent within the
^1da177e4c3f41 Linus Torvalds    2005-04-16  2188  	 * iag.  if so, the iag must be removed from the ag
25985edcedea63 Lucas De Marchi   2011-03-30  2189  	 * free extent list, so get the iags preceding and
^1da177e4c3f41 Linus Torvalds    2005-04-16  2190  	 * following the iag on this list.
^1da177e4c3f41 Linus Torvalds    2005-04-16  2191  	 */
^1da177e4c3f41 Linus Torvalds    2005-04-16  2192  	if (iagp->nfreeexts == cpu_to_le32(1)) {
^1da177e4c3f41 Linus Torvalds    2005-04-16  2193  		if ((fwd = le32_to_cpu(iagp->extfreefwd)) >= 0) {
^1da177e4c3f41 Linus Torvalds    2005-04-16  2194  			if ((rc = diIAGRead(imap, fwd, &amp)))
^1da177e4c3f41 Linus Torvalds    2005-04-16  2195  				return (rc);
^1da177e4c3f41 Linus Torvalds    2005-04-16  2196  			aiagp = (struct iag *) amp->data;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2197  		}
^1da177e4c3f41 Linus Torvalds    2005-04-16  2198
^1da177e4c3f41 Linus Torvalds    2005-04-16  2199  		if ((back = le32_to_cpu(iagp->extfreeback)) >= 0) {
^1da177e4c3f41 Linus Torvalds    2005-04-16  2200  			if ((rc = diIAGRead(imap, back, &bmp)))
^1da177e4c3f41 Linus Torvalds    2005-04-16  2201  				goto error_out;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2202  			biagp = (struct iag *) bmp->data;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2203  		}
^1da177e4c3f41 Linus Torvalds    2005-04-16  2204  	} else {
^1da177e4c3f41 Linus Torvalds    2005-04-16  2205  		/* the iag has free extents.  if all extents are free
^1da177e4c3f41 Linus Torvalds    2005-04-16  2206  		 * (as is the case for a newly allocated iag), the iag
^1da177e4c3f41 Linus Torvalds    2005-04-16  2207  		 * must be added to the ag free extent list, so get
^1da177e4c3f41 Linus Torvalds    2005-04-16  2208  		 * the iag at the head of the list in preparation for
^1da177e4c3f41 Linus Torvalds    2005-04-16  2209  		 * adding this iag to this list.
^1da177e4c3f41 Linus Torvalds    2005-04-16  2210  		 */
^1da177e4c3f41 Linus Torvalds    2005-04-16  2211  		fwd = back = -1;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2212  		if (iagp->nfreeexts == cpu_to_le32(EXTSPERIAG)) {
^1da177e4c3f41 Linus Torvalds    2005-04-16 @2213  			if ((fwd = imap->im_agctl[agno].extfree) >= 0) {

If agno == MAXAG then we're out of bounds here.

^1da177e4c3f41 Linus Torvalds    2005-04-16  2214  				if ((rc = diIAGRead(imap, fwd, &amp)))
^1da177e4c3f41 Linus Torvalds    2005-04-16  2215  					goto error_out;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2216  				aiagp = (struct iag *) amp->data;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2217  			}
^1da177e4c3f41 Linus Torvalds    2005-04-16  2218  		}
^1da177e4c3f41 Linus Torvalds    2005-04-16  2219  	}
^1da177e4c3f41 Linus Torvalds    2005-04-16  2220
^1da177e4c3f41 Linus Torvalds    2005-04-16  2221  	/* check if the iag has no free inodes.  if so, the iag





[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux