* [bug report] ntfs: update attrib operations
@ 2026-02-27 7:58 Dan Carpenter
2026-02-27 9:46 ` Namjae Jeon
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2026-02-27 7:58 UTC (permalink / raw)
To: Namjae Jeon; +Cc: linux-fsdevel
[ Smatch checking is paused while we raise funding. #SadFace
https://lore.kernel.org/all/aTaiGSbWZ9DJaGo7@stanley.mountain/ -dan ]
Hello Namjae Jeon,
Commit 495e90fa3348 ("ntfs: update attrib operations") from Feb 13,
2026 (linux-next), leads to the following Smatch static checker
warning:
fs/ntfs/attrib.c:5197 ntfs_non_resident_attr_collapse_range()
warn: inconsistent returns '&ni->runlist.lock'.
fs/ntfs/attrib.c
5107 int ntfs_non_resident_attr_collapse_range(struct ntfs_inode *ni, s64 start_vcn, s64 len)
5108 {
5109 struct ntfs_volume *vol = ni->vol;
5110 struct runlist_element *punch_rl, *rl;
5111 struct ntfs_attr_search_ctx *ctx = NULL;
5112 s64 end_vcn;
5113 int dst_cnt;
5114 int ret;
5115 size_t new_rl_cnt;
5116
5117 if (NInoAttr(ni) || ni->type != AT_DATA)
5118 return -EOPNOTSUPP;
5119
5120 end_vcn = ntfs_bytes_to_cluster(vol, ni->allocated_size);
5121 if (start_vcn >= end_vcn)
5122 return -EINVAL;
5123
5124 down_write(&ni->runlist.lock);
5125 ret = ntfs_attr_map_whole_runlist(ni);
5126 if (ret)
5127 return ret;
up_write(&ni->runlist.lock) before returning.
5128
5129 len = min(len, end_vcn - start_vcn);
5130 for (rl = ni->runlist.rl, dst_cnt = 0; rl && rl->length; rl++)
5131 dst_cnt++;
5132 rl = ntfs_rl_find_vcn_nolock(ni->runlist.rl, start_vcn);
5133 if (!rl) {
5134 up_write(&ni->runlist.lock);
5135 return -EIO;
5136 }
5137
5138 rl = ntfs_rl_collapse_range(ni->runlist.rl, dst_cnt + 1,
5139 start_vcn, len, &punch_rl, &new_rl_cnt);
5140 if (IS_ERR(rl)) {
5141 up_write(&ni->runlist.lock);
5142 return PTR_ERR(rl);
5143 }
5144 ni->runlist.rl = rl;
5145 ni->runlist.count = new_rl_cnt;
5146
5147 ni->allocated_size -= ntfs_cluster_to_bytes(vol, len);
5148 if (ni->data_size > ntfs_cluster_to_bytes(vol, start_vcn)) {
5149 if (ni->data_size > ntfs_cluster_to_bytes(vol, (start_vcn + len)))
5150 ni->data_size -= ntfs_cluster_to_bytes(vol, len);
5151 else
5152 ni->data_size = ntfs_cluster_to_bytes(vol, start_vcn);
5153 }
5154 if (ni->initialized_size > ntfs_cluster_to_bytes(vol, start_vcn)) {
5155 if (ni->initialized_size >
5156 ntfs_cluster_to_bytes(vol, start_vcn + len))
5157 ni->initialized_size -= ntfs_cluster_to_bytes(vol, len);
5158 else
5159 ni->initialized_size = ntfs_cluster_to_bytes(vol, start_vcn);
5160 }
5161
5162 if (ni->allocated_size > 0) {
5163 ret = ntfs_attr_update_mapping_pairs(ni, 0);
5164 if (ret) {
5165 up_write(&ni->runlist.lock);
5166 goto out_rl;
5167 }
5168 }
5169 up_write(&ni->runlist.lock);
5170
5171 ctx = ntfs_attr_get_search_ctx(ni, NULL);
5172 if (!ctx) {
5173 ret = -ENOMEM;
5174 goto out_rl;
5175 }
5176
5177 ret = ntfs_attr_lookup(ni->type, ni->name, ni->name_len, CASE_SENSITIVE,
5178 0, NULL, 0, ctx);
5179 if (ret)
5180 goto out_ctx;
5181
5182 ctx->attr->data.non_resident.data_size = cpu_to_le64(ni->data_size);
5183 ctx->attr->data.non_resident.initialized_size = cpu_to_le64(ni->initialized_size);
5184 if (ni->allocated_size == 0)
5185 ntfs_attr_make_resident(ni, ctx);
5186 mark_mft_record_dirty(ctx->ntfs_ino);
5187
5188 ret = ntfs_cluster_free_from_rl(vol, punch_rl);
5189 if (ret)
5190 ntfs_error(vol->sb, "Freeing of clusters failed");
5191 out_ctx:
5192 if (ctx)
5193 ntfs_attr_put_search_ctx(ctx);
5194 out_rl:
5195 kvfree(punch_rl);
5196 mark_mft_record_dirty(ni);
--> 5197 return ret;
5198 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [bug report] ntfs: update attrib operations
2026-02-27 7:58 Dan Carpenter
@ 2026-02-27 9:46 ` Namjae Jeon
0 siblings, 0 replies; 4+ messages in thread
From: Namjae Jeon @ 2026-02-27 9:46 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-fsdevel
On Fri, Feb 27, 2026 at 4:59 PM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> [ Smatch checking is paused while we raise funding. #SadFace
> https://lore.kernel.org/all/aTaiGSbWZ9DJaGo7@stanley.mountain/ -dan ]
>
> Hello Namjae Jeon,
Hi Dan,
>
> Commit 495e90fa3348 ("ntfs: update attrib operations") from Feb 13,
> 2026 (linux-next), leads to the following Smatch static checker
> warning:
>
> fs/ntfs/attrib.c:5197 ntfs_non_resident_attr_collapse_range()
> warn: inconsistent returns '&ni->runlist.lock'.
Ethan sent the patch for this and I just applied it.
Thanks for your report!
^ permalink raw reply [flat|nested] 4+ messages in thread
* [bug report] ntfs: update attrib operations
@ 2026-04-10 6:46 Dan Carpenter
0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2026-04-10 6:46 UTC (permalink / raw)
To: Namjae Jeon; +Cc: linux-fsdevel
Hello Namjae Jeon,
Commit 495e90fa3348 ("ntfs: update attrib operations") from Feb 13,
2026 (linux-next), leads to the following Smatch static checker
warning:
fs/ntfs/attrib.c:196 ntfs_map_runlist_nolock()
error: uninitialized symbol 'ctx_needs_reset'.
fs/ntfs/attrib.c
78 int ntfs_map_runlist_nolock(struct ntfs_inode *ni, s64 vcn, struct ntfs_attr_search_ctx *ctx)
79 {
80 s64 end_vcn;
81 unsigned long flags;
82 struct ntfs_inode *base_ni;
83 struct mft_record *m;
84 struct attr_record *a;
85 struct runlist_element *rl;
86 struct folio *put_this_folio = NULL;
87 int err = 0;
88 bool ctx_is_temporary = false, ctx_needs_reset;
89 struct ntfs_attr_search_ctx old_ctx = { NULL, };
90 size_t new_rl_count;
91
92 ntfs_debug("Mapping runlist part containing vcn 0x%llx.",
93 (unsigned long long)vcn);
94 if (!NInoAttr(ni))
95 base_ni = ni;
96 else
97 base_ni = ni->ext.base_ntfs_ino;
98 if (!ctx) {
99 ctx_is_temporary = ctx_needs_reset = true;
100 m = map_mft_record(base_ni);
101 if (IS_ERR(m))
102 return PTR_ERR(m);
103 ctx = ntfs_attr_get_search_ctx(base_ni, m);
104 if (unlikely(!ctx)) {
105 err = -ENOMEM;
106 goto err_out;
107 }
108 } else {
109 s64 allocated_size_vcn;
110
111 WARN_ON(IS_ERR(ctx->mrec));
112 a = ctx->attr;
113 if (!a->non_resident) {
114 err = -EIO;
115 goto err_out;
ctx_is_temporary is false. ctx_needs_reset is uninitialized.
116 }
117 end_vcn = le64_to_cpu(a->data.non_resident.highest_vcn);
118 read_lock_irqsave(&ni->size_lock, flags);
119 allocated_size_vcn =
120 ntfs_bytes_to_cluster(ni->vol, ni->allocated_size);
121 read_unlock_irqrestore(&ni->size_lock, flags);
122 if (!a->data.non_resident.lowest_vcn && end_vcn <= 0)
123 end_vcn = allocated_size_vcn - 1;
124 /*
125 * If we already have the attribute extent containing @vcn in
126 * @ctx, no need to look it up again. We slightly cheat in
127 * that if vcn exceeds the allocated size, we will refuse to
128 * map the runlist below, so there is definitely no need to get
129 * the right attribute extent.
130 */
131 if (vcn >= allocated_size_vcn || (a->type == ni->type &&
132 a->name_length == ni->name_len &&
133 !memcmp((u8 *)a + le16_to_cpu(a->name_offset),
134 ni->name, ni->name_len) &&
135 le64_to_cpu(a->data.non_resident.lowest_vcn)
136 <= vcn && end_vcn >= vcn))
137 ctx_needs_reset = false;
138 else {
139 /* Save the old search context. */
140 old_ctx = *ctx;
141 /*
142 * If the currently mapped (extent) inode is not the
143 * base inode we will unmap it when we reinitialize the
144 * search context which means we need to get a
145 * reference to the page containing the mapped mft
146 * record so we do not accidentally drop changes to the
147 * mft record when it has not been marked dirty yet.
148 */
149 if (old_ctx.base_ntfs_ino && old_ctx.ntfs_ino !=
150 old_ctx.base_ntfs_ino) {
151 put_this_folio = old_ctx.ntfs_ino->folio;
152 folio_get(put_this_folio);
153 }
154 /*
155 * Reinitialize the search context so we can lookup the
156 * needed attribute extent.
157 */
158 ntfs_attr_reinit_search_ctx(ctx);
159 ctx_needs_reset = true;
160 }
161 }
162 if (ctx_needs_reset) {
163 err = ntfs_attr_lookup(ni->type, ni->name, ni->name_len,
164 CASE_SENSITIVE, vcn, NULL, 0, ctx);
165 if (unlikely(err)) {
166 if (err == -ENOENT)
167 err = -EIO;
168 goto err_out;
169 }
170 WARN_ON(!ctx->attr->non_resident);
171 }
172 a = ctx->attr;
173 /*
174 * Only decompress the mapping pairs if @vcn is inside it. Otherwise
175 * we get into problems when we try to map an out of bounds vcn because
176 * we then try to map the already mapped runlist fragment and
177 * ntfs_mapping_pairs_decompress() fails.
178 */
179 end_vcn = le64_to_cpu(a->data.non_resident.highest_vcn) + 1;
180 if (unlikely(vcn && vcn >= end_vcn)) {
181 err = -ENOENT;
182 goto err_out;
183 }
184 rl = ntfs_mapping_pairs_decompress(ni->vol, a, &ni->runlist, &new_rl_count);
185 if (IS_ERR(rl))
186 err = PTR_ERR(rl);
187 else {
188 ni->runlist.rl = rl;
189 ni->runlist.count = new_rl_count;
190 }
191 err_out:
192 if (ctx_is_temporary) {
193 if (likely(ctx))
194 ntfs_attr_put_search_ctx(ctx);
195 unmap_mft_record(base_ni);
--> 196 } else if (ctx_needs_reset) {
^^^^^^^^^^^^^^^
Uninitialized
197 /*
198 * If there is no attribute list, restoring the search context
199 * is accomplished simply by copying the saved context back over
200 * the caller supplied context. If there is an attribute list,
This email is a free service from the Smatch-CI project [smatch.sf.net].
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* [bug report] ntfs: update attrib operations
@ 2026-04-10 10:11 Dan Carpenter
0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2026-04-10 10:11 UTC (permalink / raw)
To: Namjae Jeon; +Cc: linux-fsdevel
Hello Namjae Jeon,
Commit 495e90fa3348 ("ntfs: update attrib operations") from Feb 13,
2026 (linux-next), leads to the following Smatch static checker
warning:
fs/ntfs/compress.c:1377 ntfs_write_cb()
warn: should bitwise negate be 'llong'?
fs/ntfs/compress.c
1274 static int ntfs_write_cb(struct ntfs_inode *ni, loff_t pos, struct page **pages,
1275 int pages_per_cb)
1276 {
[ snip ]
1363 if (!fail && !allzeroes) {
1364 outbuf[compsz++] = 0;
1365 outbuf[compsz++] = 0;
1366 rounded = ((compsz - 1) | (vol->cluster_size - 1)) + 1;
1367 memset(&outbuf[compsz], 0, rounded - compsz);
1368 bio_size = rounded;
1369 pages = pages_disk;
1370 } else if (allzeroes) {
1371 err = 0;
1372 goto out;
1373 } else {
1374 bio_size = insz;
1375 }
1376
--> 1377 new_vcn = ntfs_bytes_to_cluster(vol, pos & ~(ni->itype.compressed.block_size - 1));
This should be ~(loff_t)(ni->itype.compressed.block_size - 1). Otherwise
it zeroes out everything higher than 32 bits.
1378 new_length = ntfs_bytes_to_cluster(vol, round_up(bio_size, vol->cluster_size));
1379
1380 err = ntfs_non_resident_attr_punch_hole(ni, new_vcn, ni->itype.compressed.block_clusters);
1381 if (err < 0)
1382 goto out;
1383
1384 rlc = ntfs_cluster_alloc(vol, new_vcn, new_length, -1, DATA_ZONE,
1385 false, true, true);
1386 if (IS_ERR(rlc)) {
1387 err = PTR_ERR(rlc);
1388 goto out;
1389 }
This email is a free service from the Smatch-CI project [smatch.sf.net].
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-04-10 10:11 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-10 10:11 [bug report] ntfs: update attrib operations Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2026-04-10 6:46 Dan Carpenter
2026-02-27 7:58 Dan Carpenter
2026-02-27 9:46 ` Namjae Jeon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox