Bug 275515 - Out of bounds memory access in siba_bhndb.c
Summary: Out of bounds memory access in siba_bhndb.c
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: wireless (show other bugs)
Version: 14.0-RELEASE
Hardware: Any Any
: --- Affects Only Me
Assignee: Mark Johnston
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-12-03 23:19 UTC by Frank Hilgendorf
Modified: 2023-12-12 00:38 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Frank Hilgendorf 2023-12-03 23:19:21 UTC
In /usr/src/sys/dev/bhnd/siba/siba_bhndb.c, in the class definition a wrong softc struct is used. This causes out of bound memory accesses in the driver. 
These were observed with KASAN activated in the Kernel.

Hardware:
————————-
Macbook Pro 3,1 with Broadcom BCM4321 wireless card


Patch:
—————-
289 - sizeof(struct siba_softc), bhnd_bhndb_driver, siba_driver);
289 + sizeof(struct siba_bhndb_softc), bhnd_bhndb_driver, siba_driver);
Comment 1 Mark Johnston freebsd_committer freebsd_triage 2023-12-04 15:03:23 UTC
This looks good to me.  How did you notice the problem?  struct siba_bhndb_softc initializes the "quirks" field (the only out-of-bounds field) to 0 during attach, and I suspect that malloc() size padding would hide the problem.
Comment 2 Frank Hilgendorf 2023-12-05 17:37:04 UTC
I found it while figuring out why the bwn driver causes a kernel panic after connection to my WLAN… So I activated KASAN that then revealed the problem.
Unfortunately, it was not the root cause for the bwn problem. 

I soon will file a PR on that, too.
Comment 3 Mark Johnston freebsd_committer freebsd_triage 2023-12-05 18:46:15 UTC
(In reply to Frank Hilgendorf from comment #2)
I see, thank you.  Indeed, I think this bug is "harmless", but will commit a patch today.
Comment 4 commit-hook freebsd_committer freebsd_triage 2023-12-05 18:55:53 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=4c3aa00c0a0093c78f42d138bb9eef9b1a7cbb39

commit 4c3aa00c0a0093c78f42d138bb9eef9b1a7cbb39
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2023-12-05 18:47:03 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2023-12-05 18:47:03 +0000

    bhnd: Correct the softc size in the siba_bhndb_driver definition

    struct siba_bhndb_softc embeds struct siba_softc and adds an extra
    field, "quirks".  In practice, this bug was harmless since "quirks" is
    unconditionally initialized during driver attach and would have lived in
    the redzone of the softc allocation, but KASAN catches the out-of-bounds
    access.

    PR:             275515
    Reported by:    Frank Hilgendorf <frank.hilgendorf@posteo.de>
    MFC after:      1 week

 sys/dev/bhnd/siba/siba_bhndb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 5 commit-hook freebsd_committer freebsd_triage 2023-12-12 00:31:12 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=d264ddb9c7f8739be22170ec660110a0f4c2ec8f

commit d264ddb9c7f8739be22170ec660110a0f4c2ec8f
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2023-12-05 18:47:03 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2023-12-12 00:30:05 +0000

    bhnd: Correct the softc size in the siba_bhndb_driver definition

    struct siba_bhndb_softc embeds struct siba_softc and adds an extra
    field, "quirks".  In practice, this bug was harmless since "quirks" is
    unconditionally initialized during driver attach and would have lived in
    the redzone of the softc allocation, but KASAN catches the out-of-bounds
    access.

    PR:             275515
    Reported by:    Frank Hilgendorf <frank.hilgendorf@posteo.de>
    MFC after:      1 week

    (cherry picked from commit 4c3aa00c0a0093c78f42d138bb9eef9b1a7cbb39)

 sys/dev/bhnd/siba/siba_bhndb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 6 Mark Johnston freebsd_committer freebsd_triage 2023-12-12 00:38:31 UTC
Thank you for the report.