| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <50be9574-b9b6-4752-861f-3e4b4e251207@oracle.com>
Date: Tue, 2 Jan 2024 11:10:15 -0600
From: Dave Kleikamp <dave.kleikamp@...cle.com>
To: Dan Carpenter <dan.carpenter@...aro.org>, oe-kbuild@...ts.linux.dev,
Edward Adam Davis <eadavis@...com>,
syzbot+553d90297e6d2f50dbc7@...kaller.appspotmail.com
Cc: lkp@...el.com, oe-kbuild-all@...ts.linux.dev,
jfs-discussion@...ts.sourceforge.net, linux-fsdevel@...r.kernel.org,
linux-kernel@...r.kernel.org, syzkaller-bugs@...glegroups.com
Subject: Re: [PATCH] jfs: fix array-index-out-of-bounds in diNewExt
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@intel.com/config)
> compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
> reproduce: (https://download.01.org/0day-ci/archive/20231214/202312142348.6HRZtXTB-lkp@intel.com/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@...el.com>
> | Reported-by: Dan Carpenter <dan.carpenter@...aro.org>
> | Closes: https://lore.kernel.org/r/202312142348.6HRZtXTB-lkp@intel.com/
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, &)))
> ^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, &)))
> ^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
>
Powered by blists - more mailing lists