Hello Christian Brauner,
Commit 71aacb4c8c3d ("fs: Allow statmount() in foreign mount
namespace") from Jun 24, 2024 (linux-next), leads to the following
Smatch static checker warning:
fs/namespace.c:5350 __do_sys_statmount()
error: 'ns' dereferencing possible ERR_PTR()
fs/namespace.c
5314 SYSCALL_DEFINE4(statmount, const struct mnt_id_req __user *, req,
5315 struct statmount __user *, buf, size_t, bufsize,
5316 unsigned int, flags)
5317 {
5318 struct mnt_namespace *ns __free(mnt_ns_release) = NULL;
5319 struct kstatmount *ks __free(kfree) = NULL;
5320 struct mnt_id_req kreq;
5321 /* We currently support retrieval of 3 strings. */
5322 size_t seq_size = 3 * PATH_MAX;
5323 int ret;
5324
5325 if (flags)
5326 return -EINVAL;
5327
5328 ret = copy_mnt_id_req(req, &kreq);
Should copy_mnt_id_req() ensure that kreq->mnt_ns_id is non-zero the same as
->mnt_id?
5329 if (ret)
5330 return ret;
5331
5332 ns = grab_requested_mnt_ns(&kreq);
5333 if (!ns)
5334 return -ENOENT;
The grab_requested_mnt_ns() function returns a mix of error pointers and NULL.
5335
5336 if (kreq.mnt_ns_id && (ns != current->nsproxy->mnt_ns) &&
If kreq.mnt_ns_id is non-zero grab_requested_mnt_ns() can only return NULL so
this ns dereference is fine.
5337 !ns_capable_noaudit(ns->user_ns, CAP_SYS_ADMIN))
^^^^^^^^^^^
So this is fine.
5338 return -ENOENT;
5339
5340 ks = kmalloc(sizeof(*ks), GFP_KERNEL_ACCOUNT);
5341 if (!ks)
5342 return -ENOMEM;
5343
5344 retry:
5345 ret = prepare_kstatmount(ks, &kreq, buf, bufsize, seq_size);
5346 if (ret)
5347 return ret;
5348
5349 scoped_guard(rwsem_read, &namespace_sem)
5350 ret = do_statmount(ks, kreq.mnt_id, kreq.mnt_ns_id, ns);
^^
Smatch warning tirggered here. This dereference will crash inside the
mnt_find_id_at() function, called from lookup_mnt_in_ns().
5351
5352 if (!ret)
5353 ret = copy_statmount_to_user(ks);
5354 kvfree(ks->seq.buf);
5355 if (retry_statmount(ret, &seq_size))
5356 goto retry;
5357 return ret;
5358 }
regards,
dan carpenter
