[PW_SID:1033902] Bluetooth: hci: fix refcounts in LE remote features command #3221
Conversation
This patch adds workflow files for ci: [sync.yml] - The workflow file for scheduled work - Sync the repo with upstream repo and rebase the workflow branch - Review the patches in the patchwork and creates the PR if needed [ci.yml] - The workflow file for CI tasks - Run CI tests when PR is created Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
KASAN reported a slab-use-after-free in le_read_features_complete() running from hci_cmd_sync_work. le_read_features_complete() can run after hci_rx_work/hci_conn_del() has removed the link, so the destroy callback may touch a freed hci_conn and trigger a KASAN use-after-free. Duplicate submissions also need to drop the references to avoid leaking the hold and blocking teardown. Fix this by taking hci_conn_get() before queueing, passing the conn directly as work data, and balancing hci_conn_hold()/drop() and hci_conn_get()/put() in the completion and all error/-EEXIST paths so the connection stays referenced while the work runs and is released afterwards. Reported-by: syzbot+87badbb9094e008e0685@syzkaller.appspotmail.com Signed-off-by: Cihangir Akturk <cakturk@gmail.com>
|
CheckPatch |
|
GitLint |
|
SubjectPrefix |
|
BuildKernel |
|
CheckAllWarning |
|
CheckSparse |
|
BuildKernel32 |
|
TestRunnerSetup |
|
TestRunner_l2cap-tester |
|
TestRunner_iso-tester |
|
TestRunner_bnep-tester |
|
TestRunner_mgmt-tester |
|
TestRunner_rfcomm-tester |
|
TestRunner_sco-tester |
|
TestRunner_ioctl-tester |
|
TestRunner_mesh-tester |
|
TestRunner_smp-tester |
|
TestRunner_userchan-tester |
|
IncrementalBuild |
github-actions
bot
force-pushed
the
workflow
branch
5 times, most recently
from
8eabaf6 to
b7ba526
Compare
KASAN reported a slab-use-after-free in le_read_features_complete()
running from hci_cmd_sync_work. le_read_features_complete() can run
after hci_rx_work/hci_conn_del() has removed the link, so the destroy
callback may touch a freed hci_conn and trigger a KASAN use-after-free.
Duplicate submissions also need to drop the references to avoid leaking
the hold and blocking teardown.
Fix this by taking hci_conn_get() before queueing, passing the conn
directly as work data, and balancing hci_conn_hold()/drop() and
hci_conn_get()/put() in the completion and all error/-EEXIST paths so
the connection stays referenced while the work runs and is released
afterwards.
Reported-by: syzbot+87badbb9094e008e0685@syzkaller.appspotmail.com
Signed-off-by: Cihangir Akturk cakturk@gmail.com
net/bluetooth/hci_sync.c | 37 ++++++++++++++++++++++++++-----------
1 file changed, 26 insertions(+), 11 deletions(-)
I am not entirely sure that removing the err == -ECANCELED early return
is strictly correct. My assumption is that, with the changes in this
patch, there does not appear to be another cancellation path that
reliably balances hci_conn_drop() and hci_conn_put() for this command.
Based on that assumption, keeping the early return could leave the
references taken before queuing unbalanced on cancellation, so I opted
to remove it to keep the lifetime handling consistent.