Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Use-after-free / list corruption in ndtest_alloc_resource() error path. - [High] Missing error check in ndtest_nvdimm_init() leads to NULL pointer dereference. -- commit 96f1b7f6e50c7c8c92043b84b02d39517266ebe3 Author: Mohammad Shahid <[email protected]> nvdimm: ndtest: remove redundant NULL check before vfree() vfree() safely handles NULL pointers, so the explicit NULL check before calling vfree() is unnecessary. This was reported by ifnullfree.cocci. > diff --git a/tools/testing/nvdimm/test/ndtest.c > b/tools/testing/nvdimm/test/ndtest.c > index 8e3b6be53839b..2051ad5d4882d 100644 > --- a/tools/testing/nvdimm/test/ndtest.c > +++ b/tools/testing/nvdimm/test/ndtest.c > @@ -376,8 +376,7 @@ static void *ndtest_alloc_resource(struct ndtest_priv *p, > size_t size, > buf_err: > if (__dma && size >= DIMM_SIZE) > gen_pool_free(ndtest_pool, __dma, size); > - if (buf) > - vfree(buf); > + vfree(buf); > kfree(res); [Severity: High] This isn't a bug introduced by this patch, but could this error path cause list corruption or a use-after-free? Earlier in ndtest_alloc_resource(), res is added to the p->resources list: spin_lock(&ndtest_lock); list_add(&res->list, &p->resources); spin_unlock(&ndtest_lock); If devm_add_action() fails, execution falls through to buf_err, where kfree(res) is called. However, res is not removed from the p->resources list before being freed. When the driver core subsequently invokes devres_release_all() on failure, the cleanup function ndtest_release_resource() for any previously successful allocations will traverse and modify this corrupted list, resulting in a use-after-free. Would it make sense to call list_del() before freeing res here? [Severity: High] This is a pre-existing issue, but while looking at this file, I noticed a potential NULL pointer dereference in ndtest_nvdimm_init(): tools/testing/nvdimm/test/ndtest.c:ndtest_nvdimm_init() { ... ndtest_dimm_register(p, d, id); ... } If ndtest_dimm_register() fails, such as due to an allocation failure for the dimm device, it returns an error but leaves d->nvdimm as NULL. Because ndtest_nvdimm_init() ignores this error, it successfully returns. Later, ndtest_create_region() passes the NULL nvdimm pointer to nvdimm_pmem_region_create(), which dereferences it inside nd_region_create(), causing a crash. Should the return value of ndtest_dimm_register() be checked to prevent this? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
