Created attachment 248120 [details] The potential NULL dereferences We utilized a static analyzer to detect instances of omitted NULL checks in certain resource allocation functions within the Kernel. The attached document enumerates these occurrences, which could potentially lead to NULL pointer dereferences. The document is organized as follows: the first column lists the allocation functions that might return a NULL pointer under certain conditions; the second column specifies the exact locations (file +line_number) where these allocation functions are invoked without appropriate NULL checks, creating a risk of NULL pointer dereference; and the third column pinpoints the specific line of source code.
Most, if not all, of these appear to be allocations with M_WAITOK, which will never return NULL: malloc(9): "The malloc(), mallocarray(), realloc(), and reallocf() functions cannot return NULL if M_WAITOK is specified."
Comment on attachment 248120 [details] The potential NULL dereferences The allocation functions that can return NULL pointer Potential Buggy Point Potential buggy uses (the returned pointers are not properly checked) cam_simq_alloc sys/cam/cam_xpt.c +910 devq = cam_simq_alloc(16); nvme_allocate_request_vaddr sys/dev/nvme/nvme_ctrlr_cmd.c +39 req = nvme_allocate_request_vaddr(payload, sizeof(struct nvme_controller_data), cb_fn, cb_arg); nvme_allocate_request_vaddr sys/dev/nvme/nvme_ctrlr_cmd.c +63 req = nvme_allocate_request_vaddr(payload, sizeof(struct nvme_namespace_data), cb_fn, cb_arg); nvme_allocate_request_vaddr sys/dev/nvme/nvme_ctrlr_cmd.c +262 req = nvme_allocate_request_vaddr(payload, payload_size, cb_fn, cb_arg); nvme_allocate_request_vaddr sys/dev/nvme/nvme_ctrlr.c +1256/+1259 req = nvme_allocate_request_vaddr(buf->b_data, pt->len, nvme_pt_done, pt); devfs_alloc sys/kern/kern_conf.c +1331 ndev = devfs_alloc(MAKEDEV_WAITOK);
(In reply to Kristof Provost from comment #1) Thanks for the comment! I rechecked the list and removed the cases caused by the, M_WAITOK, flag setting.
Oh, sorry, the last line, devfs_alloc sys/kern/kern_conf.c +1331 ndev = devfs_alloc(MAKEDEV_WAITOK); is also M_WAITOK related. We can also skip this one.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=eb86c6c5b462c996e44c45ba496937b75ef22da3 commit eb86c6c5b462c996e44c45ba496937b75ef22da3 Author: John Baldwin <jhb@FreeBSD.org> AuthorDate: 2024-02-09 19:53:43 +0000 Commit: John Baldwin <jhb@FreeBSD.org> CommitDate: 2024-02-09 19:53:43 +0000 cam: Check if cam_simq_alloc fails for the xpt bus during module init This is very unlikely to fail (and if it does, CAM isn't going to work regardless), but fail with an error rather than a gauranteed panic via NULL pointer dereference. PR: 276770 Reported by: Qiushi <w290680224@gmail.com> sys/cam/cam_xpt.c | 2 ++ 1 file changed, 2 insertions(+)
I think for the nvme ones we probably want to change nvme_allocate_request to take a 'how' flag so these particular requests can pass M_WAITOK down to malloc instead.
A commit in branch stable/14 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=7d763eadd36e9b9419379756add71f5383f40af0 commit 7d763eadd36e9b9419379756add71f5383f40af0 Author: John Baldwin <jhb@FreeBSD.org> AuthorDate: 2024-02-09 19:53:43 +0000 Commit: John Baldwin <jhb@FreeBSD.org> CommitDate: 2024-04-08 17:53:43 +0000 cam: Check if cam_simq_alloc fails for the xpt bus during module init This is very unlikely to fail (and if it does, CAM isn't going to work regardless), but fail with an error rather than a gauranteed panic via NULL pointer dereference. PR: 276770 Reported by: Qiushi <w290680224@gmail.com> (cherry picked from commit eb86c6c5b462c996e44c45ba496937b75ef22da3) sys/cam/cam_xpt.c | 2 ++ 1 file changed, 2 insertions(+)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=9fc935855951ff8277c669b2aa248fde63a10175 commit 9fc935855951ff8277c669b2aa248fde63a10175 Author: John Baldwin <jhb@FreeBSD.org> AuthorDate: 2024-02-09 19:53:43 +0000 Commit: John Baldwin <jhb@FreeBSD.org> CommitDate: 2024-04-08 17:53:54 +0000 cam: Check if cam_simq_alloc fails for the xpt bus during module init This is very unlikely to fail (and if it does, CAM isn't going to work regardless), but fail with an error rather than a gauranteed panic via NULL pointer dereference. PR: 276770 Reported by: Qiushi <w290680224@gmail.com> (cherry picked from commit eb86c6c5b462c996e44c45ba496937b75ef22da3) sys/cam/cam_xpt.c | 2 ++ 1 file changed, 2 insertions(+)
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=f08746a7e3195a6e144e6f58003dc5c221d15d02 commit f08746a7e3195a6e144e6f58003dc5c221d15d02 Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2024-11-09 17:34:12 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2024-11-09 17:34:12 +0000 nvme: Pass malloc flags to request allocation functions There are some contexts where it is safe to sleep, so we should pass M_WAITOK to ensure that a null pointer dereference can't happen. A few places allocate with M_NOWAIT but have no way to signal an error. Flag those with an XXX comment. PR: 276770 Reviewed by: imp MFC after: 2 weeks Differential Revision: https://reviews.freebsd.org/D47307 sys/dev/nvme/nvme_ctrlr.c | 22 ++++++++++++++-------- sys/dev/nvme/nvme_ctrlr_cmd.c | 29 +++++++++++++++++++---------- sys/dev/nvme/nvme_ns_cmd.c | 24 +++++++++--------------- sys/dev/nvme/nvme_private.h | 26 +++++++++++++++----------- sys/dev/nvme/nvme_sim.c | 11 ++++++----- 5 files changed, 63 insertions(+), 49 deletions(-)
^Triage: assign to committer who resolved.