| Message ID | 20220708062548.31167-1-wangyouwan@uniontech.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | obexd: fix crashed after cancel the on-going transfer | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/checkpatch | success | Checkpatch PASS |
| tedd_an/gitlint | success | Gitlint PASS |
| tedd_an/setupell | success | Setup ELL PASS |
| tedd_an/buildprep | success | Build Prep PASS |
| tedd_an/build | success | Build Configuration PASS |
| tedd_an/makecheck | success | Make Check PASS |
| tedd_an/makecheckvalgrind | success | Make Check PASS |
| tedd_an/makedistcheck | success | Make Distcheck PASS |
| tedd_an/build_extell | success | Build External ELL PASS |
| tedd_an/build_extell_make | success | Build Make with External ELL PASS |
| tedd_an/scan_build | warning | Scan-Build: gobex/gobex-transfer.c:425:7: warning: Use of memory after it is freed if (!g_slist_find(transfers, transfer)) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1 warning generated. |
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=657710
---Test result---
Test Summary:
CheckPatch PASS 0.79 seconds
GitLint PASS 0.51 seconds
Prep - Setup ELL PASS 25.86 seconds
Build - Prep PASS 0.57 seconds
Build - Configure PASS 7.95 seconds
Build - Make PASS 733.51 seconds
Make Check PASS 11.12 seconds
Make Check w/Valgrind PASS 280.38 seconds
Make Distcheck PASS 230.86 seconds
Build w/ext ELL - Configure PASS 8.03 seconds
Build w/ext ELL - Make PASS 79.35 seconds
Incremental Build w/ patches PASS 0.00 seconds
Scan Build WARNING 483.63 seconds
Details
##############################
Test: Scan Build - WARNING
Desc: Run Scan Build with patches
Output:
*****************************************************************************
The bugs reported by the scan-build may or may not be caused by your patches.
Please check the list and fix the bugs if they are caused by your patch.
*****************************************************************************
gobex/gobex-transfer.c:425:7: warning: Use of memory after it is freed
if (!g_slist_find(transfers, transfer))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
---
Regards,
Linux Bluetooth
Hi Youwan, On Thu, Jul 7, 2022 at 11:30 PM Youwan Wang <wangyouwan@uniontech.com> wrote: > > Pointer is reused after release. > After the transfer pointer is released, > remove the response function in pending_pkg > to avoid the 'p->rsp_func' is reused > > step: > [obex]# connect 28:33:34:1E:96:98 > Attempting to connect to 28:33:34:1E:96:98 > [NEW] Session /org/bluez/obex/client/session2 [default] > [NEW] ObjectPush /org/bluez/obex/client/session2 > Connection successful > [28:33:34:1E:96:98]# send /home/uos/Desktop/systemd.zip > Attempting to send /home/uos/Desktop/systemd.zip > [NEW] Transfer /org/bluez/obex/client/session2/transfer2 > Transfer /org/bluez/obex/client/session2/transfer2 > Status: queued > Name: systemd.zip > Size: 33466053 > Filename: /home/uos/Desktop/systemd.zip > Session: /org/bluez/obex/client/session2 > [CHG] Transfer /org/bluez/obex/client/session2/transfer2 > [CHG] Transfer /org/bluez/obex/client/session2/transfer2 > [CHG] Transfer /org/bluez/obex/client/session2/transfer2 > [CHG] Transfer /org/bluez/obex/client/session2/transfer2 > [CHG] Transfer /org/bluez/obex/client/session2/transfer2 > [CHG] Transfer /org/bluez/obex/client/session2/transfer2 > [CHG] Transfer /org/bluez/obex/client/session2/transfer2 > [CHG] Transfer /org/bluez/obex/client/session2/transfer2 > [CHG] Transfer /org/bluez/obex/client/session2/transfer2 > [CHG] Transfer /org/bluez/obex/client/session2/transfer2 > [CHG] Transfer /org/bluez/obex/client/session2/transfer2 > [CHG] Transfer /org/bluez/obex/client/session2/transfer2 > er2 33:34:1E:96:98]# cancel /org/bluez/obex/client/sessi > Attempting to cancel transfer /org/bluez/obex/client/s > Cancel successful > > valgrind trace: > ==11431== Invalid read of size 4 > ==11431== at 0x12B442: transfer_response () > ==11431== by 0x127764: req_timeout () > ==11431== by 0x49B8922: ??? ( ) > ==11431== by 0x49B7E97: g_main_context_dispatch () > ==11431== by 0x49B8287: ??? (in ) > ==11431== by 0x49B8581: g_main_loop_run () > ==11431== by 0x121834: main (main.c:322) > ==11431== Address 0x7344fa0 is 16 bytes inside a block of size > ==11431== at 0x48369AB: free () > ==11431== by 0x12B459: transfer_response () > ==11431== by 0x127B3D: cancel_complete () > ==11431== by 0x49B7E97: g_main_context_dispatch () > ==11431== by 0x49B8287: ??? () > ==11431== by 0x49B8581: g_main_loop_run () > ==11431== by 0x121834: main (main.c:322) > ==11431== Block was alloc'd at > ==11431== at 0x4837B65: calloc () > ==11431== by 0x49BD9D8: g_malloc0 () > ==11431== by 0x12AB89: transfer_new () > ==11431== by 0x12B732: g_obex_put_req_pkt () > ==11431== by 0x12B732: g_obex_put_req_pkt () > ==11431== by 0x146982: transfer_start_put () > ==11431== by 0x146982: obc_transfer_start () > ==11431== by 0x13C5A7: session_process_transfer () > ==11431== by 0x13D248: session_process_queue () > ==11431== by 0x13D248: session_process_queue () > ==11431== by 0x13D2AF: session_process () > ==11431== by 0x49B7E97: g_main_context_dispatch () > ==11431== by 0x49B8287: ??? (i) > ==11431== by 0x49B8581: g_main_loop_run () > ==11431== by 0x121834: main () > ==11431== > ==11431== (action on error) vgdb me ... > --- > gobex/gobex-transfer.c | 2 ++ > gobex/gobex.c | 6 ++++++ > gobex/gobex.h | 1 + > 3 files changed, 9 insertions(+) > > diff --git a/gobex/gobex-transfer.c b/gobex/gobex-transfer.c > index c94d018b2..38c23785c 100644 > --- a/gobex/gobex-transfer.c > +++ b/gobex/gobex-transfer.c > @@ -64,6 +64,8 @@ static void transfer_free(struct transfer *transfer) > g_obex_remove_request_function(transfer->obex, > transfer->abort_id); > > + g_obex_remove_responsefunc(transfer->obex); > + > g_obex_unref(transfer->obex); > g_free(transfer); > } > diff --git a/gobex/gobex.c b/gobex/gobex.c > index bc4d52551..54ce484f2 100644 > --- a/gobex/gobex.c > +++ b/gobex/gobex.c > @@ -533,6 +533,12 @@ void g_obex_drop_tx_queue(GObex *obex) > pending_pkt_free(p); > } > > +void g_obex_remove_responsefunc(GObex *obex) > +{ > + if (obex->pending_req) > + obex->pending_req->rsp_func = NULL; > +} Can we stop treating the symptom and really fix the cause, transfer_free does already call g_obex_cancel_req which normally should take care of removing any timeout handling which seems to be the cause of the crash here: https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/gobex/gobex.c#n841 Btw, Ive already pointed out that there are test cases for it and if we are not capturing the exact scenario you are describing here would you please add a test case with the exact response that is causing such a crash: https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/unit/test-gobex-transfer.c#n480 > static gboolean g_obex_send_internal(GObex *obex, struct pending_pkt *p, > GError **err) > { > diff --git a/gobex/gobex.h b/gobex/gobex.h > index f16e4426c..1f7ae513a 100644 > --- a/gobex/gobex.h > +++ b/gobex/gobex.h > @@ -51,6 +51,7 @@ void g_obex_suspend(GObex *obex); > void g_obex_resume(GObex *obex); > gboolean g_obex_srm_active(GObex *obex); > void g_obex_drop_tx_queue(GObex *obex); > +void g_obex_remove_responsefunc(GObex *obex); > > GObex *g_obex_new(GIOChannel *io, GObexTransportType transport_type, > gssize rx_mtu, gssize tx_mtu); > -- > 2.20.1 > > >
diff --git a/gobex/gobex-transfer.c b/gobex/gobex-transfer.c index c94d018b2..38c23785c 100644 --- a/gobex/gobex-transfer.c +++ b/gobex/gobex-transfer.c @@ -64,6 +64,8 @@ static void transfer_free(struct transfer *transfer) g_obex_remove_request_function(transfer->obex, transfer->abort_id); + g_obex_remove_responsefunc(transfer->obex); + g_obex_unref(transfer->obex); g_free(transfer); } diff --git a/gobex/gobex.c b/gobex/gobex.c index bc4d52551..54ce484f2 100644 --- a/gobex/gobex.c +++ b/gobex/gobex.c @@ -533,6 +533,12 @@ void g_obex_drop_tx_queue(GObex *obex) pending_pkt_free(p); } +void g_obex_remove_responsefunc(GObex *obex) +{ + if (obex->pending_req) + obex->pending_req->rsp_func = NULL; +} + static gboolean g_obex_send_internal(GObex *obex, struct pending_pkt *p, GError **err) { diff --git a/gobex/gobex.h b/gobex/gobex.h index f16e4426c..1f7ae513a 100644 --- a/gobex/gobex.h +++ b/gobex/gobex.h @@ -51,6 +51,7 @@ void g_obex_suspend(GObex *obex); void g_obex_resume(GObex *obex); gboolean g_obex_srm_active(GObex *obex); void g_obex_drop_tx_queue(GObex *obex); +void g_obex_remove_responsefunc(GObex *obex); GObex *g_obex_new(GIOChannel *io, GObexTransportType transport_type, gssize rx_mtu, gssize tx_mtu);
Pointer is reused after release. After the transfer pointer is released, remove the response function in pending_pkg to avoid the 'p->rsp_func' is reused step: [obex]# connect 28:33:34:1E:96:98 Attempting to connect to 28:33:34:1E:96:98 [NEW] Session /org/bluez/obex/client/session2 [default] [NEW] ObjectPush /org/bluez/obex/client/session2 Connection successful [28:33:34:1E:96:98]# send /home/uos/Desktop/systemd.zip Attempting to send /home/uos/Desktop/systemd.zip [NEW] Transfer /org/bluez/obex/client/session2/transfer2 Transfer /org/bluez/obex/client/session2/transfer2 Status: queued Name: systemd.zip Size: 33466053 Filename: /home/uos/Desktop/systemd.zip Session: /org/bluez/obex/client/session2 [CHG] Transfer /org/bluez/obex/client/session2/transfer2 [CHG] Transfer /org/bluez/obex/client/session2/transfer2 [CHG] Transfer /org/bluez/obex/client/session2/transfer2 [CHG] Transfer /org/bluez/obex/client/session2/transfer2 [CHG] Transfer /org/bluez/obex/client/session2/transfer2 [CHG] Transfer /org/bluez/obex/client/session2/transfer2 [CHG] Transfer /org/bluez/obex/client/session2/transfer2 [CHG] Transfer /org/bluez/obex/client/session2/transfer2 [CHG] Transfer /org/bluez/obex/client/session2/transfer2 [CHG] Transfer /org/bluez/obex/client/session2/transfer2 [CHG] Transfer /org/bluez/obex/client/session2/transfer2 [CHG] Transfer /org/bluez/obex/client/session2/transfer2 er2 33:34:1E:96:98]# cancel /org/bluez/obex/client/sessi Attempting to cancel transfer /org/bluez/obex/client/s Cancel successful valgrind trace: ==11431== Invalid read of size 4 ==11431== at 0x12B442: transfer_response () ==11431== by 0x127764: req_timeout () ==11431== by 0x49B8922: ??? ( ) ==11431== by 0x49B7E97: g_main_context_dispatch () ==11431== by 0x49B8287: ??? (in ) ==11431== by 0x49B8581: g_main_loop_run () ==11431== by 0x121834: main (main.c:322) ==11431== Address 0x7344fa0 is 16 bytes inside a block of size ==11431== at 0x48369AB: free () ==11431== by 0x12B459: transfer_response () ==11431== by 0x127B3D: cancel_complete () ==11431== by 0x49B7E97: g_main_context_dispatch () ==11431== by 0x49B8287: ??? () ==11431== by 0x49B8581: g_main_loop_run () ==11431== by 0x121834: main (main.c:322) ==11431== Block was alloc'd at ==11431== at 0x4837B65: calloc () ==11431== by 0x49BD9D8: g_malloc0 () ==11431== by 0x12AB89: transfer_new () ==11431== by 0x12B732: g_obex_put_req_pkt () ==11431== by 0x12B732: g_obex_put_req_pkt () ==11431== by 0x146982: transfer_start_put () ==11431== by 0x146982: obc_transfer_start () ==11431== by 0x13C5A7: session_process_transfer () ==11431== by 0x13D248: session_process_queue () ==11431== by 0x13D248: session_process_queue () ==11431== by 0x13D2AF: session_process () ==11431== by 0x49B7E97: g_main_context_dispatch () ==11431== by 0x49B8287: ??? (i) ==11431== by 0x49B8581: g_main_loop_run () ==11431== by 0x121834: main () ==11431== ==11431== (action on error) vgdb me ... --- gobex/gobex-transfer.c | 2 ++ gobex/gobex.c | 6 ++++++ gobex/gobex.h | 1 + 3 files changed, 9 insertions(+)