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
