| 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: <8084185d-b4cf-4c30-a67e-28b9b590306d@studenti.polito.it>
Date: Mon, 23 Sep 2024 11:23:48 +0200
From: Gianfranco Trad <s323713@...denti.polito.it>
To: kernel test robot <lkp@...el.com>, oe-kbuild@...ts.linux.dev
Cc: Dan Carpenter <error27@...il.com>, jack@...e.com,
linux-kernel@...r.kernel.org,
syzbot+8901c4560b7ab5c2f9df@...kaller.appspotmail.com
Subject: Re: [PATCH] udf: fix uninit-value use in udf_get_fileshortad
Hello,
On 23/09/24 02:26, kernel test robot wrote:
> BCC: lkp@...el.com
> CC: oe-kbuild-all@...ts.linux.dev
> In-Reply-To: <20240919195227.412583-1-gianf.trad@...il.com>
> References: <20240919195227.412583-1-gianf.trad@...il.com>
> TO: Gianfranco Trad <gianf.trad@...il.com>
> TO: jack@...e.com
> CC: linux-kernel@...r.kernel.org
> CC: skhan@...uxfoundation.org
> CC: Gianfranco Trad <gianf.trad@...il.com>
> CC: syzbot+8901c4560b7ab5c2f9df@...kaller.appspotmail.com
>
> Hi Gianfranco,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on brauner-vfs/vfs.all]
> [also build test WARNING on linus/master v6.11 next-20240920]
> [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/Gianfranco-Trad/udf-fix-uninit-value-use-in-udf_get_fileshortad/20240920-035519
> base: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all
> patch link: https://lore.kernel.org/r/20240919195227.412583-1-gianf.trad%40gmail.com
> patch subject: [PATCH] udf: fix uninit-value use in udf_get_fileshortad
> :::::: branch date: 3 days ago
> :::::: commit date: 3 days ago
> config: mips-randconfig-r073-20240922 (https://download.01.org/0day-ci/archive/20240923/202409230809.5ozgjLm9-lkp@intel.com/config)
> compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 8663a75fa2f31299ab8d1d90288d9df92aadee88)
>
> 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 <error27@...il.com>
> | Closes: https://lore.kernel.org/r/202409230809.5ozgjLm9-lkp@intel.com/
>
> smatch warnings:
> fs/udf/inode.c:2223 udf_current_aext() error: we previously assumed 'epos->bh' could be null (see line 2204)
Given this error my initial guess is to update the patch to check the
assumption of epos->bh not null.
> vim +2223 fs/udf/inode.c
>
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2193
> ff116fc8d1d439 Jan Kara 2007-05-08 2194 int8_t udf_current_aext(struct inode *inode, struct extent_position *epos,
> 5ca4e4be841e38 Pekka Enberg 2008-10-15 2195 struct kernel_lb_addr *eloc, uint32_t *elen, int inc)
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2196 {
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2197 int alen;
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2198 int8_t etype;
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2199 uint8_t *ptr;
> 5ca4e4be841e38 Pekka Enberg 2008-10-15 2200 struct short_ad *sad;
> 5ca4e4be841e38 Pekka Enberg 2008-10-15 2201 struct long_ad *lad;
> 48d6d8ff7dca80 Marcin Slusarz 2008-02-08 2202 struct udf_inode_info *iinfo = UDF_I(inode);
> 28de7948a89676 Cyrill Gorcunov 2007-07-21 2203
> cb00ea3528eb3c Cyrill Gorcunov 2007-07-19 @2204 if (!epos->bh) {
> ff116fc8d1d439 Jan Kara 2007-05-08 2205 if (!epos->offset)
> ff116fc8d1d439 Jan Kara 2007-05-08 2206 epos->offset = udf_file_entry_alloc_offset(inode);
> 382a2287bf9cd2 Jan Kara 2020-09-25 2207 ptr = iinfo->i_data + epos->offset -
> 4b11111aba6c80 Marcin Slusarz 2008-02-08 2208 udf_file_entry_alloc_offset(inode) +
> 48d6d8ff7dca80 Marcin Slusarz 2008-02-08 2209 iinfo->i_lenEAttr;
> 4b11111aba6c80 Marcin Slusarz 2008-02-08 2210 alen = udf_file_entry_alloc_offset(inode) +
> 48d6d8ff7dca80 Marcin Slusarz 2008-02-08 2211 iinfo->i_lenAlloc;
> cb00ea3528eb3c Cyrill Gorcunov 2007-07-19 2212 } else {
> ff116fc8d1d439 Jan Kara 2007-05-08 2213 if (!epos->offset)
> ff116fc8d1d439 Jan Kara 2007-05-08 2214 epos->offset = sizeof(struct allocExtDesc);
> ff116fc8d1d439 Jan Kara 2007-05-08 2215 ptr = epos->bh->b_data + epos->offset;
> 28de7948a89676 Cyrill Gorcunov 2007-07-21 2216 alen = sizeof(struct allocExtDesc) +
> 4b11111aba6c80 Marcin Slusarz 2008-02-08 2217 le32_to_cpu(((struct allocExtDesc *)epos->bh->b_data)->
> 4b11111aba6c80 Marcin Slusarz 2008-02-08 2218 lengthAllocDescs);
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2219 }
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2220
> 48d6d8ff7dca80 Marcin Slusarz 2008-02-08 2221 switch (iinfo->i_alloc_type) {
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2222 case ICBTAG_FLAG_AD_SHORT:
> 2aa242080b8dda Gianfranco Trad 2024-09-19 @2223 if (unlikely(alen < 0 && epos->offset == epos->bh->b_size))
> 2aa242080b8dda Gianfranco Trad 2024-09-19 2224 return -1;
The check would be inserted up here in the if clause. But if the patch
doesn't look good, I can redesign it in a better way. If so, I'm more
than glad to follow your advice.
> 4b11111aba6c80 Marcin Slusarz 2008-02-08 2225 sad = udf_get_fileshortad(ptr, alen, &epos->offset, inc);
> 4b11111aba6c80 Marcin Slusarz 2008-02-08 2226 if (!sad)
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2227 return -1;
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2228 etype = le32_to_cpu(sad->extLength) >> 30;
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2229 eloc->logicalBlockNum = le32_to_cpu(sad->extPosition);
> 4b11111aba6c80 Marcin Slusarz 2008-02-08 2230 eloc->partitionReferenceNum =
> 48d6d8ff7dca80 Marcin Slusarz 2008-02-08 2231 iinfo->i_location.partitionReferenceNum;
> 28de7948a89676 Cyrill Gorcunov 2007-07-21 2232 *elen = le32_to_cpu(sad->extLength) & UDF_EXTENT_LENGTH_MASK;
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2233 break;
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2234 case ICBTAG_FLAG_AD_LONG:
> 4b11111aba6c80 Marcin Slusarz 2008-02-08 2235 lad = udf_get_filelongad(ptr, alen, &epos->offset, inc);
> 4b11111aba6c80 Marcin Slusarz 2008-02-08 2236 if (!lad)
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2237 return -1;
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2238 etype = le32_to_cpu(lad->extLength) >> 30;
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2239 *eloc = lelb_to_cpu(lad->extLocation);
> 28de7948a89676 Cyrill Gorcunov 2007-07-21 2240 *elen = le32_to_cpu(lad->extLength) & UDF_EXTENT_LENGTH_MASK;
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2241 break;
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2242 default:
> fcbf7637e6647e Steve Magnani 2017-10-12 2243 udf_debug("alloc_type = %u unsupported\n", iinfo->i_alloc_type);
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2244 return -1;
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2245 }
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2246
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2247 return etype;
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2248 }
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 2249
>
Thanks for your time,
--Gian
Powered by blists - more mailing lists