* [PATCH] obexd: Fix crash of transfer pointer has being freed @ 2022-06-15 12:49 Youwan Wang 2022-06-15 14:39 ` bluez.test.bot 2022-06-17 22:58 ` [PATCH] " Luiz Augusto von Dentz 0 siblings, 2 replies; 4+ messages in thread From: Youwan Wang @ 2022-06-15 12:49 UTC (permalink / raw) To: linux-bluetooth; +Cc: Youwan Wang PC1 -> PC2 in the process of transferring the file, PC2 terminate the reception. It is found that PC1 will receive the Rcvd Forbiden message. transfer_response func is called and go to failed: transfer_complete func is called and go to transfer_free Invalid read of size 8 at 0x129C9B: put_get_data (gobex-transfer.c:138) by 0x128675: get_body (gobex-packet.c:397) by 0x128675: g_obex_packet_encode (gobex-packet.c:451) by 0x1264ED: write_data (gobex.c:461) by 0x1264ED: write_data (gobex.c:431) by 0x49B7E97: g_main_context_dispatch () by 0x49B8287: ??? () by 0x49B8581: g_main_loop_run () by 0x120834: main (main.c:322) Address 0x7300028 is 56 bytes inside a block of size 64 free'd at 0x48369AB: free (vg_replace_malloc.c:530) by 0x12A404: transfer_response (gobex-transfer.c:245) by 0x126B02: handle_response (gobex.c:1128) by 0x127087: incoming_data (gobex.c:1373) by 0x49B7E97: g_main_context_dispatch () by 0x49B8287: ??? () by 0x49B8581: g_main_loop_run () by 0x120834: main (main.c:322) Block was alloc'd at at 0x4837B65: calloc (vg_replace_malloc.c:752) by 0x49BD9D8: g_malloc0 () by 0x129A49: transfer_new (gobex-transfer.c:258) by 0x12A612: g_obex_put_req_pkt (gobex-transfer.c:282) by 0x12A612: g_obex_put_req_pkt (gobex-transfer.c:271) by 0x145862: transfer_start_put (transfer.c:880) by 0x145862: obc_transfer_start (transfer.c:911) by 0x13B487: session_process_transfer (session.c:836) by 0x13C128: session_process_queue (session.c:887) by 0x13C128: session_process_queue (session.c:872) by 0x13C18F: session_process (session.c:820) by 0x49B7E97: g_main_context_dispatch (in /usr/lib/x86_64-linux-gnu/) by 0x49B8287: ??? (in /usr/lib/x86_64-linux-gnu/) by 0x49B8581: g_main_loop_run (in /usr/lib/x86_64-linux-gnu/) by 0x120834: main (main.c:322) --- gobex/gobex-transfer.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/gobex/gobex-transfer.c b/gobex/gobex-transfer.c index c94d018b2..8e97fc73e 100644 --- a/gobex/gobex-transfer.c +++ b/gobex/gobex-transfer.c @@ -197,6 +197,14 @@ static void transfer_response(GObex *obex, GError *err, GObexPacket *rsp, } rspcode = g_obex_packet_get_operation(rsp, &final); + if (rspcode == G_OBEX_RSP_FORBIDDEN) { + err = g_error_new(G_OBEX_ERROR, rspcode, "%s", + g_obex_strerror(rspcode)); + g_obex_debug(G_OBEX_DEBUG_ERROR, "%s", err->message); + g_error_free(err); + return; + } + if (rspcode != G_OBEX_RSP_SUCCESS && rspcode != G_OBEX_RSP_CONTINUE) { err = g_error_new(G_OBEX_ERROR, rspcode, "%s", g_obex_strerror(rspcode)); -- 2.20.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* RE: obexd: Fix crash of transfer pointer has being freed 2022-06-15 12:49 [PATCH] obexd: Fix crash of transfer pointer has being freed Youwan Wang @ 2022-06-15 14:39 ` bluez.test.bot 2022-06-17 22:58 ` [PATCH] " Luiz Augusto von Dentz 1 sibling, 0 replies; 4+ messages in thread From: bluez.test.bot @ 2022-06-15 14:39 UTC (permalink / raw) To: linux-bluetooth, wangyouwan [-- Attachment #1: Type: text/plain, Size: 995 bytes --] 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=650581 ---Test result--- Test Summary: CheckPatch PASS 1.42 seconds GitLint PASS 0.93 seconds Prep - Setup ELL PASS 42.31 seconds Build - Prep PASS 0.67 seconds Build - Configure PASS 8.38 seconds Build - Make PASS 1244.74 seconds Make Check PASS 11.39 seconds Make Check w/Valgrind PASS 463.73 seconds Make Distcheck PASS 236.82 seconds Build w/ext ELL - Configure PASS 9.33 seconds Build w/ext ELL - Make PASS 1265.84 seconds Incremental Build with patchesPASS 0.00 seconds --- Regards, Linux Bluetooth ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] obexd: Fix crash of transfer pointer has being freed 2022-06-15 12:49 [PATCH] obexd: Fix crash of transfer pointer has being freed Youwan Wang 2022-06-15 14:39 ` bluez.test.bot @ 2022-06-17 22:58 ` Luiz Augusto von Dentz 2022-06-29 23:52 ` Luiz Augusto von Dentz 1 sibling, 1 reply; 4+ messages in thread From: Luiz Augusto von Dentz @ 2022-06-17 22:58 UTC (permalink / raw) To: Youwan Wang; +Cc: linux-bluetooth Hi Youwan, On Wed, Jun 15, 2022 at 5:54 AM Youwan Wang <wangyouwan@uniontech.com> wrote: > > PC1 -> PC2 in the process of transferring the file, > PC2 terminate the reception. > It is found that PC1 will receive the Rcvd > Forbiden message. > transfer_response func is called and go to failed: > transfer_complete func is called and go to transfer_free > > Invalid read of size 8 > at 0x129C9B: put_get_data (gobex-transfer.c:138) > by 0x128675: get_body (gobex-packet.c:397) > by 0x128675: g_obex_packet_encode (gobex-packet.c:451) > by 0x1264ED: write_data (gobex.c:461) > by 0x1264ED: write_data (gobex.c:431) > by 0x49B7E97: g_main_context_dispatch () > by 0x49B8287: ??? () > by 0x49B8581: g_main_loop_run () > by 0x120834: main (main.c:322) > Address 0x7300028 is 56 bytes inside a block of size 64 free'd > at 0x48369AB: free (vg_replace_malloc.c:530) > by 0x12A404: transfer_response (gobex-transfer.c:245) > by 0x126B02: handle_response (gobex.c:1128) > by 0x127087: incoming_data (gobex.c:1373) > by 0x49B7E97: g_main_context_dispatch () > by 0x49B8287: ??? () > by 0x49B8581: g_main_loop_run () > by 0x120834: main (main.c:322) > Block was alloc'd at > at 0x4837B65: calloc (vg_replace_malloc.c:752) > by 0x49BD9D8: g_malloc0 () > by 0x129A49: transfer_new (gobex-transfer.c:258) > by 0x12A612: g_obex_put_req_pkt (gobex-transfer.c:282) > by 0x12A612: g_obex_put_req_pkt (gobex-transfer.c:271) > by 0x145862: transfer_start_put (transfer.c:880) > by 0x145862: obc_transfer_start (transfer.c:911) > by 0x13B487: session_process_transfer (session.c:836) > by 0x13C128: session_process_queue (session.c:887) > by 0x13C128: session_process_queue (session.c:872) > by 0x13C18F: session_process (session.c:820) > by 0x49B7E97: g_main_context_dispatch (in /usr/lib/x86_64-linux-gnu/) > by 0x49B8287: ??? (in /usr/lib/x86_64-linux-gnu/) > by 0x49B8581: g_main_loop_run (in /usr/lib/x86_64-linux-gnu/) > by 0x120834: main (main.c:322) > --- > gobex/gobex-transfer.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/gobex/gobex-transfer.c b/gobex/gobex-transfer.c > index c94d018b2..8e97fc73e 100644 > --- a/gobex/gobex-transfer.c > +++ b/gobex/gobex-transfer.c > @@ -197,6 +197,14 @@ static void transfer_response(GObex *obex, GError *err, GObexPacket *rsp, > } > > rspcode = g_obex_packet_get_operation(rsp, &final); > + if (rspcode == G_OBEX_RSP_FORBIDDEN) { > + err = g_error_new(G_OBEX_ERROR, rspcode, "%s", > + g_obex_strerror(rspcode)); > + g_obex_debug(G_OBEX_DEBUG_ERROR, "%s", err->message); > + g_error_free(err); > + return; > + } This is not quite right, in fact the if statement below already treat such errors, we might be missing something like final: /* Reset if final so it can no longer be cancelled */ if (final_rsp) obex->pending_req = NULL; This would be a problem on the remote implementation which is indicating that perhaps it is just a temporary failure, anyway then: id = transfer->req_id; transfer->req_id = 0; This resets the req_id so on transfer_free the following code is not run: if (transfer->req_id > 0) g_obex_cancel_req(transfer->obex, transfer->req_id, TRUE); So either we have to restore the transfer->req_id = id or not reset it to 0, so perhaps something like this works better: diff --git a/gobex/gobex-transfer.c b/gobex/gobex-transfer.c index c94d018b2..8fae65d8d 100644 --- a/gobex/gobex-transfer.c +++ b/gobex/gobex-transfer.c @@ -200,6 +200,11 @@ static void transfer_response(GObex *obex, GError *err, GObexPacket *rsp, if (rspcode != G_OBEX_RSP_SUCCESS && rspcode != G_OBEX_RSP_CONTINUE) { err = g_error_new(G_OBEX_ERROR, rspcode, "%s", g_obex_strerror(rspcode)); + /* If not marked as final response but transfer_complete gonna + * be called restore the req_id so it can be properly cancelled + */ + if (!final) + transport->req_id = id; goto failed; } > + > if (rspcode != G_OBEX_RSP_SUCCESS && rspcode != G_OBEX_RSP_CONTINUE) { > err = g_error_new(G_OBEX_ERROR, rspcode, "%s", > g_obex_strerror(rspcode)); > -- > 2.20.1 > > > -- Luiz Augusto von Dentz ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] obexd: Fix crash of transfer pointer has being freed 2022-06-17 22:58 ` [PATCH] " Luiz Augusto von Dentz @ 2022-06-29 23:52 ` Luiz Augusto von Dentz 0 siblings, 0 replies; 4+ messages in thread From: Luiz Augusto von Dentz @ 2022-06-29 23:52 UTC (permalink / raw) To: Youwan Wang; +Cc: linux-bluetooth Hi Youwan, On Fri, Jun 17, 2022 at 3:58 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Youwan, > > On Wed, Jun 15, 2022 at 5:54 AM Youwan Wang <wangyouwan@uniontech.com> wrote: > > > > PC1 -> PC2 in the process of transferring the file, > > PC2 terminate the reception. > > It is found that PC1 will receive the Rcvd > > Forbiden message. > > transfer_response func is called and go to failed: > > transfer_complete func is called and go to transfer_free > > > > Invalid read of size 8 > > at 0x129C9B: put_get_data (gobex-transfer.c:138) > > by 0x128675: get_body (gobex-packet.c:397) > > by 0x128675: g_obex_packet_encode (gobex-packet.c:451) > > by 0x1264ED: write_data (gobex.c:461) > > by 0x1264ED: write_data (gobex.c:431) > > by 0x49B7E97: g_main_context_dispatch () > > by 0x49B8287: ??? () > > by 0x49B8581: g_main_loop_run () > > by 0x120834: main (main.c:322) > > Address 0x7300028 is 56 bytes inside a block of size 64 free'd > > at 0x48369AB: free (vg_replace_malloc.c:530) > > by 0x12A404: transfer_response (gobex-transfer.c:245) > > by 0x126B02: handle_response (gobex.c:1128) > > by 0x127087: incoming_data (gobex.c:1373) > > by 0x49B7E97: g_main_context_dispatch () > > by 0x49B8287: ??? () > > by 0x49B8581: g_main_loop_run () > > by 0x120834: main (main.c:322) > > Block was alloc'd at > > at 0x4837B65: calloc (vg_replace_malloc.c:752) > > by 0x49BD9D8: g_malloc0 () > > by 0x129A49: transfer_new (gobex-transfer.c:258) > > by 0x12A612: g_obex_put_req_pkt (gobex-transfer.c:282) > > by 0x12A612: g_obex_put_req_pkt (gobex-transfer.c:271) > > by 0x145862: transfer_start_put (transfer.c:880) > > by 0x145862: obc_transfer_start (transfer.c:911) > > by 0x13B487: session_process_transfer (session.c:836) > > by 0x13C128: session_process_queue (session.c:887) > > by 0x13C128: session_process_queue (session.c:872) > > by 0x13C18F: session_process (session.c:820) > > by 0x49B7E97: g_main_context_dispatch (in /usr/lib/x86_64-linux-gnu/) > > by 0x49B8287: ??? (in /usr/lib/x86_64-linux-gnu/) > > by 0x49B8581: g_main_loop_run (in /usr/lib/x86_64-linux-gnu/) > > by 0x120834: main (main.c:322) > > --- > > gobex/gobex-transfer.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/gobex/gobex-transfer.c b/gobex/gobex-transfer.c > > index c94d018b2..8e97fc73e 100644 > > --- a/gobex/gobex-transfer.c > > +++ b/gobex/gobex-transfer.c > > @@ -197,6 +197,14 @@ static void transfer_response(GObex *obex, GError *err, GObexPacket *rsp, > > } > > > > rspcode = g_obex_packet_get_operation(rsp, &final); > > + if (rspcode == G_OBEX_RSP_FORBIDDEN) { > > + err = g_error_new(G_OBEX_ERROR, rspcode, "%s", > > + g_obex_strerror(rspcode)); > > + g_obex_debug(G_OBEX_DEBUG_ERROR, "%s", err->message); > > + g_error_free(err); > > + return; > > + } > > This is not quite right, in fact the if statement below already treat > such errors, we might be missing something like final: > > /* Reset if final so it can no longer be cancelled */ > if (final_rsp) > obex->pending_req = NULL; > > This would be a problem on the remote implementation which is > indicating that perhaps it is just a temporary failure, anyway then: > > id = transfer->req_id; > transfer->req_id = 0; > > This resets the req_id so on transfer_free the following code is not run: > > if (transfer->req_id > 0) > g_obex_cancel_req(transfer->obex, transfer->req_id, TRUE); > > So either we have to restore the transfer->req_id = id or not reset it > to 0, so perhaps something like this works better: > > diff --git a/gobex/gobex-transfer.c b/gobex/gobex-transfer.c > index c94d018b2..8fae65d8d 100644 > --- a/gobex/gobex-transfer.c > +++ b/gobex/gobex-transfer.c > @@ -200,6 +200,11 @@ static void transfer_response(GObex *obex, GError > *err, GObexPacket *rsp, > if (rspcode != G_OBEX_RSP_SUCCESS && rspcode != G_OBEX_RSP_CONTINUE) { > err = g_error_new(G_OBEX_ERROR, rspcode, "%s", > g_obex_strerror(rspcode)); > + /* If not marked as final response but transfer_complete gonna > + * be called restore the req_id so it can be properly cancelled > + */ > + if (!final) > + transport->req_id = id; > goto failed; > } You haven't responded about this one, I believe there are quite a few patches of yours that are trying to circumvent this very issue. > > + > > if (rspcode != G_OBEX_RSP_SUCCESS && rspcode != G_OBEX_RSP_CONTINUE) { > > err = g_error_new(G_OBEX_ERROR, rspcode, "%s", > > g_obex_strerror(rspcode)); > > -- > > 2.20.1 > > > > > > > > > -- > Luiz Augusto von Dentz -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-06-29 23:52 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-06-15 12:49 [PATCH] obexd: Fix crash of transfer pointer has being freed Youwan Wang 2022-06-15 14:39 ` bluez.test.bot 2022-06-17 22:58 ` [PATCH] " Luiz Augusto von Dentz 2022-06-29 23:52 ` Luiz Augusto von Dentz
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.