* [bug report] bcachefs: bch_err_throw()
@ 2025-08-06 11:44 Dan Carpenter
2025-08-06 17:02 ` Kent Overstreet
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2025-08-06 11:44 UTC (permalink / raw)
To: Kent Overstreet; +Cc: linux-bcachefs
Hello Kent Overstreet,
Commit 09b9c72bd4b7 ("bcachefs: bch_err_throw()") from May 28, 2025
(linux-next), leads to the following Smatch static checker warning:
fs/bcachefs/journal_sb.c:213 bch2_journal_buckets_to_sb()
error: we previously assumed 'c' could be null (see line 197)
fs/bcachefs/journal_sb.c
191 int bch2_journal_buckets_to_sb(struct bch_fs *c, struct bch_dev *ca,
192 u64 *buckets, unsigned nr)
193 {
194 struct bch_sb_field_journal_v2 *j;
195 unsigned i, dst = 0, nr_compacted = 1;
196
197 if (c)
^^^^^^
Here we assume c can be NULL.
198 lockdep_assert_held(&c->sb_lock);
199
200 if (!nr) {
201 bch2_sb_field_delete(&ca->disk_sb, BCH_SB_FIELD_journal);
202 bch2_sb_field_delete(&ca->disk_sb, BCH_SB_FIELD_journal_v2);
203 return 0;
204 }
205
206 for (i = 0; i + 1 < nr; i++)
207 if (buckets[i] + 1 != buckets[i + 1])
208 nr_compacted++;
209
210 j = bch2_sb_field_resize(&ca->disk_sb, journal_v2,
211 (sizeof(*j) + sizeof(j->d[0]) * nr_compacted) / sizeof(u64));
212 if (!j)
--> 213 return bch_err_throw(c, ENOSPC_sb_journal);
^
The patch adds an unchecked dereference
214
215 bch2_sb_field_delete(&ca->disk_sb, BCH_SB_FIELD_journal);
216
217 j->d[dst].start = cpu_to_le64(buckets[0]);
218 j->d[dst].nr = cpu_to_le64(1);
219
220 for (i = 1; i < nr; i++) {
221 if (buckets[i] == buckets[i - 1] + 1) {
222 le64_add_cpu(&j->d[dst].nr, 1);
223 } else {
224 dst++;
225 j->d[dst].start = cpu_to_le64(buckets[i]);
226 j->d[dst].nr = cpu_to_le64(1);
227 }
228 }
229
230 BUG_ON(dst + 1 != nr_compacted);
231 return 0;
232 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* [bug report] bcachefs: bch_err_throw()
@ 2025-08-06 11:44 Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2025-08-06 11:44 UTC (permalink / raw)
To: Kent Overstreet; +Cc: linux-bcachefs
Hello Kent Overstreet,
Commit 09b9c72bd4b7 ("bcachefs: bch_err_throw()") from May 28, 2025
(linux-next), leads to the following (unpublished) Smatch static checker
warning:
fs/bcachefs/super.c:2546 bch2_fs_open()
warn: pointer dereferenced without being set 'c'
fs/bcachefs/super.c
2515 struct bch_fs *bch2_fs_open(darray_const_str *devices,
2516 struct bch_opts *opts)
2517 {
2518 bch_sb_handles sbs = {};
2519 struct bch_fs *c = NULL;
^^^^^^^^^
c is NULL
2520 struct bch_sb_handle *best = NULL;
2521 int ret = 0;
2522
2523 if (!try_module_get(THIS_MODULE))
2524 return ERR_PTR(-ENODEV);
2525
2526 if (!devices->nr) {
2527 ret = -EINVAL;
2528 goto err;
2529 }
2530
2531 ret = darray_make_room(&sbs, devices->nr);
2532 if (ret)
2533 goto err;
2534
2535 darray_for_each(*devices, i) {
2536 struct bch_sb_handle sb = { NULL };
2537
2538 ret = bch2_read_super(*i, opts, &sb);
2539 if (ret)
2540 goto err;
2541
2542 BUG_ON(darray_push(&sbs, sb));
2543 }
2544
2545 if (opts->nochanges && !opts->read_only) {
--> 2546 ret = bch_err_throw(c, erofs_nochanges);
^
It hasn't been set yet.
2547 goto err_print;
2548 }
2549
2550 darray_for_each(sbs, sb)
2551 if (!best || sb_cmp(sb->sb, best->sb) > 0)
2552 best = sb;
2553
2554 darray_for_each_reverse(sbs, sb) {
2555 ret = bch2_dev_in_fs(best, sb, opts);
2556
2557 if (ret == -BCH_ERR_device_has_been_removed ||
2558 ret == -BCH_ERR_device_splitbrain) {
2559 bch2_free_super(sb);
2560 darray_remove_item(&sbs, sb);
2561 best -= best > sb;
2562 ret = 0;
2563 continue;
2564 }
2565
2566 if (ret)
2567 goto err_print;
2568 }
2569
2570 c = bch2_fs_alloc(best->sb, opts, &sbs);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
set here.
2571 ret = PTR_ERR_OR_ZERO(c);
2572 if (ret)
2573 goto err;
2574
2575 scoped_guard(rwsem_write, &c->state_lock)
2576 darray_for_each(sbs, sb) {
2577 ret = bch2_dev_attach_bdev(c, sb);
2578 if (ret)
2579 goto err;
2580 }
2581
2582 if (!c->opts.nostart) {
2583 ret = bch2_fs_start(c);
2584 if (ret)
2585 goto err;
2586 }
2587 out:
2588 darray_for_each(sbs, sb)
2589 bch2_free_super(sb);
2590 darray_exit(&sbs);
2591 module_put(THIS_MODULE);
2592 return c;
2593 err_print:
2594 pr_err("bch_fs_open err opening %s: %s",
2595 devices->data[0], bch2_err_str(ret));
2596 err:
2597 if (!IS_ERR_OR_NULL(c))
2598 bch2_fs_stop(c);
2599 c = ERR_PTR(ret);
2600 goto out;
2601 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] bcachefs: bch_err_throw()
2025-08-06 11:44 Dan Carpenter
@ 2025-08-06 17:02 ` Kent Overstreet
0 siblings, 0 replies; 3+ messages in thread
From: Kent Overstreet @ 2025-08-06 17:02 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-bcachefs
On Wed, Aug 06, 2025 at 02:44:19PM +0300, Dan Carpenter wrote:
> Hello Kent Overstreet,
>
> Commit 09b9c72bd4b7 ("bcachefs: bch_err_throw()") from May 28, 2025
> (linux-next), leads to the following Smatch static checker warning:
>
> fs/bcachefs/journal_sb.c:213 bch2_journal_buckets_to_sb()
> error: we previously assumed 'c' could be null (see line 197)
>
> fs/bcachefs/journal_sb.c
> 191 int bch2_journal_buckets_to_sb(struct bch_fs *c, struct bch_dev *ca,
> 192 u64 *buckets, unsigned nr)
> 193 {
> 194 struct bch_sb_field_journal_v2 *j;
> 195 unsigned i, dst = 0, nr_compacted = 1;
> 196
> 197 if (c)
> ^^^^^^
> Here we assume c can be NULL.
>
> 198 lockdep_assert_held(&c->sb_lock);
> 199
> 200 if (!nr) {
> 201 bch2_sb_field_delete(&ca->disk_sb, BCH_SB_FIELD_journal);
> 202 bch2_sb_field_delete(&ca->disk_sb, BCH_SB_FIELD_journal_v2);
> 203 return 0;
> 204 }
> 205
> 206 for (i = 0; i + 1 < nr; i++)
> 207 if (buckets[i] + 1 != buckets[i + 1])
> 208 nr_compacted++;
> 209
> 210 j = bch2_sb_field_resize(&ca->disk_sb, journal_v2,
> 211 (sizeof(*j) + sizeof(j->d[0]) * nr_compacted) / sizeof(u64));
> 212 if (!j)
> --> 213 return bch_err_throw(c, ENOSPC_sb_journal);
This is a real bug.
> ^
> The patch adds an unchecked dereference
>
> 214
> 215 bch2_sb_field_delete(&ca->disk_sb, BCH_SB_FIELD_journal);
> 216
> 217 j->d[dst].start = cpu_to_le64(buckets[0]);
> 218 j->d[dst].nr = cpu_to_le64(1);
> 219
> 220 for (i = 1; i < nr; i++) {
> 221 if (buckets[i] == buckets[i - 1] + 1) {
> 222 le64_add_cpu(&j->d[dst].nr, 1);
> 223 } else {
> 224 dst++;
> 225 j->d[dst].start = cpu_to_le64(buckets[i]);
> 226 j->d[dst].nr = cpu_to_le64(1);
> 227 }
> 228 }
> 229
> 230 BUG_ON(dst + 1 != nr_compacted);
> 231 return 0;
> 232 }
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-08-06 17:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-06 11:44 [bug report] bcachefs: bch_err_throw() Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2025-08-06 11:44 Dan Carpenter
2025-08-06 17:02 ` Kent Overstreet
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).