Bug 276770 - Some potential NULL-pointer dereferences
Summary: Some potential NULL-pointer dereferences
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-02-01 21:36 UTC by Qiushi
Modified: 2024-04-08 20:26 UTC (History)
3 users (show)

See Also:


Attachments
The potential NULL dereferences (3.05 KB, text/plain)
2024-02-01 21:36 UTC, Qiushi
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Qiushi 2024-02-01 21:36:26 UTC
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.
Comment 1 Kristof Provost freebsd_committer freebsd_triage 2024-02-01 21:43:18 UTC
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 2 Qiushi 2024-02-01 23:08:53 UTC
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);
Comment 3 Qiushi 2024-02-01 23:10:23 UTC
(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.
Comment 4 Qiushi 2024-02-01 23:14:25 UTC
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.
Comment 5 commit-hook freebsd_committer freebsd_triage 2024-02-09 19:57:52 UTC
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(+)
Comment 6 John Baldwin freebsd_committer freebsd_triage 2024-02-09 20:01:52 UTC
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.
Comment 7 commit-hook freebsd_committer freebsd_triage 2024-04-08 19:05:25 UTC
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(+)
Comment 8 commit-hook freebsd_committer freebsd_triage 2024-04-08 20:26:35 UTC
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(+)