[backport v3.6-branch] Bluetooth: host: sched-lock bt_recv() #75361
Conversation
`bt_recv` is invoked from the BT long work queue, which is preemptible. The host uses cooperative scheduling to ensure thread safety. Signed-off-by: Aleksander Wasaznik <[email protected]>
zephyrbot
requested review from
hermabe,
HoZHel,
jori-nordic,
sjanc,
Thalley and
theob-pro
|
I have removed "Fixes #75356" from the description, as that is just the issue for the failed backport. This needs a proper bug issue in order to be backported. |
Isn't the referenced advisory (which is now also public) essentially a bug report? |
Should that just be closed then? I thought it would have been natural to close it automatically when this PR gets merged (hence the reference) but I guess it's also fair to say that already the creation of this PR is in itself a fix for the automated backport failure. |
It's fine to close it when this is merged, but it doesn't provide justification for merging the backport (and as such should not cause the backport CI check to succeed). |
|
@jhedberg This is still pending a bug report. |
Again, IMO the referenced public security advisory is the bug report and there shouldn't be a need to introduce additional bureaucracy in this case. |
I am only following our established process. |
Understood, but I think it's also everyone's duty to call out issues with it when we see them. I think in this case it's also up for interpretation, i.e. do you consider a security advisory a bug report or not (I do). Based on #75361 (comment) it seems like you're still waiting for additional feedback on this. |
A hundred times, yes. @henrikbrixandersen instead of nagging people, and wasting time arguing from both sides: could we use the github checks to enforce this? Then there will be no discussion. |
The GitHub check already verifies that a backport description contains a "Fixes:" line - this one does, but it does not refer to a bug issue, which deviates from our established process. |
Then fix the check. What's the point of checks if we still have to manually check?? |
|
In the past (at least on the 2.7.5 branch) when we had to deprecate TF-M support due to multiple vulnerabilities, we had #56071 filed to track it. A large part of the issue inclusion (in my view) is to ensure the change gets properly called out in the release notes automation. |
Again, can't we fix the release automation to also track published Security Advisories? |
henrikbrixandersen
added
DNM
|
Hi, I think it is redundant to have issues for published advisories. Backporting these fixes is already documented in https://docs.zephyrproject.org/latest/security/reporting.html#backporting-of-security-vulnerabilities. We do need issues for unpublished advisories, as the vulnerability is still under embargo, but we must avoid making it obvious that a specific PR is addressing the vulnerability. |
|
On today's Process WG meeting, it was decided to update https://github.com/zephyrproject-rtos/zephyr/wiki/Zephyr-Change-Control-Board-(CCB) to also allow published security advisories as justification for backporting. |
MaureenHelm
merged commit d5a127a
into
zephyrproject-rtos:v3.6-branch
bt_recv is invoked from the BT long work queue, which is preemptible. The host uses cooperative scheduling to ensure thread safety.
Fixes: GHSA-jmr9-xw2v-5vf4