Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: comp_dev: initialize buffer lists to prevent NULL dereference #9689

Conversation

tmleman
Copy link
Contributor

@tmleman tmleman commented Nov 28, 2024

This patch addresses a NULL dereference issue in the SOF firmware that was exposed by a recent change in Zephyr's MMU mapping for Intel ADSP ACE30. The change prevents mapping of the 0x0 address, which helps catch NULL pointer accesses.

The issue was identified during testing, where an exception occurred due to uninitialized buffer lists in the comp_dev structure. To fix this, the bsink_list and bsource_list are now initialized in the comp_alloc function.

This ensures that the lists point to themselves before any use, preventing NULL dereference and subsequent exceptions.

Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent, thanks for submitting fix @tmleman !

One request to the commit message: The commit description leaves a bit open in which case this is triggered. I think it would help to mention the list_init is called in comp_new() (for both IPC3 and IPC4), but NULL dereference can happen in the component ops->create() which is called before list is initialized. One affected component is IPC4 copier_ipcgtw (you can add a "Link: #9687"

@tmleman tmleman force-pushed the topic/upstream/pr/comp/fix/null_dereference branch from 955d7ee to 5f5588c Compare November 28, 2024 12:50
@tmleman
Copy link
Contributor Author

tmleman commented Nov 28, 2024

One request to the commit message: The commit description leaves a bit open in which case this is triggered. I think it would help to mention the list_init is called in comp_new() (for both IPC3 and IPC4), but NULL dereference can happen in the component ops->create() which is called before list is initialized. One affected component is IPC4 copier_ipcgtw (you can add a "Link: #9687"

Done.

@tmleman tmleman requested a review from kv2019i November 28, 2024 12:53
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent, thanks for the quick update!

This patch addresses a NULL dereference issue in the SOF firmware that
was exposed by a recent change in Zephyr's MMU mapping for Intel ADSP
ACE30. The change prevents mapping of the 0x0 address, which helps catch
NULL pointer accesses.

The issue was identified during testing, where an exception occurred due
to uninitialized buffer lists in the `comp_dev` structure. The
`list_init` function is called in `comp_new()` (for both IPC3 and IPC4),
but a NULL dereference can happen in the component `ops->create()`
function, which is called before the list is initialized. One affected
component is IPC4 `copier_ipcgtw`.

To fix this, the `bsink_list` and `bsource_list` are now initialized in
the `comp_alloc` function. This ensures that the lists point to
themselves before any use, preventing NULL dereference and subsequent
exceptions.

Link: thesofproject#9687

Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
@kv2019i
Copy link
Collaborator

kv2019i commented Dec 3, 2024

SOFCI TEST

@kv2019i
Copy link
Collaborator

kv2019i commented Dec 3, 2024

sof-docs fail and Intel LNL fails all known and tracked in https://github.com/thesofproject/sof/issues?q=is%3Aissue+is%3Aopen+label%3A%22Known+PR+Failures%22+

@kv2019i kv2019i merged commit bfb567d into thesofproject:main Dec 3, 2024
43 of 47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants