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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <32f85613-725e-4306-8be1-54a526436a46@paragon-software.com>
Date: Mon, 9 Feb 2026 11:20:06 +0100
From: Konstantin Komarov <almaz.alexandrovich@...agon-software.com>
To: Dan Carpenter <dan.carpenter@...aro.org>
CC: <ntfs3@...ts.linux.dev>, linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [bug report] fs/ntfs3: Add initialization of super block

On 2/6/26 14:41, Dan Carpenter wrote:

> [ Smatch checking is paused while we raise funding.  #SadFace
>    https://lore.kernel.org/all/aTaiGSbWZ9DJaGo7@stanley.mountain/ -dan ]
>
> Hello Konstantin Komarov,
>
> Commit 82cae269cfa9 ("fs/ntfs3: Add initialization of super block")
> from Aug 13, 2021 (linux-next), leads to the following Smatch static
> checker warning:
>
> fs/ntfs3/fsntfs.c:1260 ntfs_read_run_nb_ra() error: we previously assumed 'run' could be null (see line 1178)
> fs/ntfs3/fsntfs.c:1259 ntfs_read_run_nb_ra() error: uninitialized symbol 'clen'.
> fs/ntfs3/fsntfs.c:1260 ntfs_read_run_nb_ra() error: uninitialized symbol 'idx'.
>
> fs/ntfs3/fsntfs.c
>      1161 int ntfs_read_run_nb_ra(struct ntfs_sb_info *sbi, const struct runs_tree *run,
>      1162                         u64 vbo, void *buf, u32 bytes, struct ntfs_buffers *nb,
>      1163                         struct file_ra_state *ra)
>      1164 {
>      1165         int err;
>      1166         struct super_block *sb = sbi->sb;
>      1167         struct address_space *mapping = sb->s_bdev->bd_mapping;
>      1168         u32 blocksize = sb->s_blocksize;
>      1169         u8 cluster_bits = sbi->cluster_bits;
>      1170         u32 off = vbo & sbi->cluster_mask;
>      1171         u32 nbh = 0;
>      1172         CLST vcn_next, vcn = vbo >> cluster_bits;
>      1173         CLST lcn, clen;
>      1174         u64 lbo, len;
>      1175         size_t idx;
>      1176         struct buffer_head *bh;
>      1177
>      1178         if (!run) {
>      1179                 /* First reading of $Volume + $MFTMirr + $LogFile goes here. */
>      1180                 if (vbo > MFT_REC_VOL * sbi->record_size) {
>      1181                         err = -ENOENT;
>      1182                         goto out;
>      1183                 }
>      1184
>      1185                 /* Use absolute boot's 'MFTCluster' to read record. */
>      1186                 lbo = vbo + sbi->mft.lbo;
>      1187                 len = sbi->record_size;
>
> If run is NULL then "clen" is uninitialized.
>
>      1188         } else if (!run_lookup_entry(run, vcn, &lcn, &clen, &idx)) {
>      1189                 err = -ENOENT;
>      1190                 goto out;
>      1191         } else {
>      1192                 if (lcn == SPARSE_LCN) {
>      1193                         err = -EINVAL;
>      1194                         goto out;
>      1195                 }
>      1196
>      1197                 lbo = ((u64)lcn << cluster_bits) + off;
>      1198                 len = ((u64)clen << cluster_bits) - off;
>      1199         }
>      1200
>      1201         off = lbo & (blocksize - 1);
>      1202         if (nb) {
>      1203                 nb->off = off;
>      1204                 nb->bytes = bytes;
>      1205         }
>      1206
>      1207         if (ra && !ra->ra_pages)
>      1208                 file_ra_state_init(ra, mapping);
>      1209
>      1210         for (;;) {
>      1211                 u32 len32 = len >= bytes ? bytes : len;
>      1212                 sector_t block = lbo >> sb->s_blocksize_bits;
>      1213
>      1214                 if (ra) {
>      1215                         pgoff_t index = lbo >> PAGE_SHIFT;
>      1216                         if (!ra_has_index(ra, index)) {
>      1217                                 page_cache_sync_readahead(mapping, ra, NULL,
>      1218                                                           index, 1);
>      1219                                 ra->prev_pos = (loff_t)index << PAGE_SHIFT;
>      1220                         }
>      1221                 }
>      1222
>      1223                 do {
>      1224                         u32 op = blocksize - off;
>      1225
>      1226                         if (op > len32)
>      1227                                 op = len32;
>      1228
>      1229                         bh = ntfs_bread(sb, block);
>      1230                         if (!bh) {
>      1231                                 err = -EIO;
>      1232                                 goto out;
>      1233                         }
>      1234
>      1235                         if (buf) {
>      1236                                 memcpy(buf, bh->b_data + off, op);
>      1237                                 buf = Add2Ptr(buf, op);
>      1238                         }
>      1239
>      1240                         if (!nb) {
>      1241                                 put_bh(bh);
>      1242                         } else if (nbh >= ARRAY_SIZE(nb->bh)) {
>      1243                                 err = -EINVAL;
>      1244                                 goto out;
>      1245                         } else {
>      1246                                 nb->bh[nbh++] = bh;
>      1247                                 nb->nbufs = nbh;
>      1248                         }
>      1249
>      1250                         bytes -= op;
>      1251                         if (!bytes)
>      1252                                 return 0;
>      1253                         len32 -= op;
>      1254                         block += 1;
>      1255                         off = 0;
>      1256
>      1257                 } while (len32);
>      1258
> --> 1259                 vcn_next = vcn + clen;
>                                            ^^^^
> Used uninitalized here.
>
>      1260                 if (!run_get_entry(run, ++idx, &vcn, &lcn, &clen) ||
>
> But also if we pass a NULL run to run_get_entry() it will crash.  I'm
> a bit confused by this code.
>
>      1261                     vcn != vcn_next) {
>      1262                         err = -ENOENT;
>      1263                         goto out;
>      1264                 }
>      1265
>      1266                 if (lcn == SPARSE_LCN) {
>      1267                         err = -EINVAL;
>      1268                         goto out;
>      1269                 }
>      1270
>      1271                 lbo = ((u64)lcn << cluster_bits);
>      1272                 len = ((u64)clen << cluster_bits);
>      1273         }
>      1274
>      1275 out:
>      1276         if (!nbh)
>      1277                 return err;
>      1278
>      1279         while (nbh) {
>      1280                 put_bh(nb->bh[--nbh]);
>      1281                 nb->bh[nbh] = NULL;
>      1282         }
>      1283
>      1284         nb->nbufs = 0;
>      1285         return err;
>      1286 }
>
> regards,
> dan carpenter

Hello,

Thanks for the Smatch report. I’ll examine the warnings, prepare a fix,
and post a patch.

Regards,
Konstantin


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ