On Fri, Oct 13, 2023 at 10:17:21AM +0300, Dan Carpenter wrote:
> Hello Kent Overstreet,
>
> The patch e0750d947352: "bcachefs: Initial commit" from Mar 16, 2017
> (linux-next), leads to the following Smatch static checker warning:
>
> fs/bcachefs/btree_update_interior.c:2265 bch2_btree_node_update_key()
> error: 'new_hash' dereferencing possible ERR_PTR()
>
> fs/bcachefs/btree_update_interior.c
> 2227 int bch2_btree_node_update_key(struct btree_trans *trans, struct
> btree_iter *iter,
> 2228 struct btree *b, struct bkey_i
> *new_key,
> 2229 unsigned commit_flags, bool
> skip_triggers)
> 2230 {
> 2231 struct bch_fs *c = trans->c;
> 2232 struct btree *new_hash = NULL;
> 2233 struct btree_path *path = iter->path;
> 2234 struct closure cl;
> 2235 int ret = 0;
> 2236
> 2237 ret = bch2_btree_path_upgrade(trans, path, b->c.level + 1);
> 2238 if (ret)
> 2239 return ret;
> 2240
> 2241 closure_init_stack(&cl);
> 2242
> 2243 /*
> 2244 * check btree_ptr_hash_val() after @b is locked by
> 2245 * btree_iter_traverse():
> 2246 */
> 2247 if (btree_ptr_hash_val(new_key) != b->hash_val) {
> 2248 ret = bch2_btree_cache_cannibalize_lock(c, &cl);
> 2249 if (ret) {
> 2250 ret = drop_locks_do(trans,
> (closure_sync(&cl), 0));
> 2251 if (ret)
> 2252 return ret;
> 2253 }
> 2254
> 2255 new_hash = bch2_btree_node_mem_alloc(trans, false);
>
> This allocation can fail and it returns an error pointer?
It doesn't fail when we hold the cannibalize lock. I can add an
assertion that documents that if it'll stop smatch from complaining...
