From 243f21e2ec8081e26131357a590bed45a783013b Mon Sep 17 00:00:00 2001 From: dongjiuzhu1 Date: Wed, 29 Nov 2023 22:14:12 +0800 Subject: [PATCH 1/8] microadb: fix used after free issue about handle close uv__run_closing_handles libuv/src/unix/core.c:365 uv_run libuv/src/unix/core.c:464 adb_hal_run microADB/hal/hal_uv.c:69 adbd_main /home/djz/workspace/vela_rp/apps/system/adb/adb_main.c:157 nxtask_startup sched/task_startup.c:70 nxtask_start task/task_start.c:134 pre_start sim/sim_initialstate.c:52 Signed-off-by: dongjiuzhu1 --- hal/hal_uv_client_usb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hal/hal_uv_client_usb.c b/hal/hal_uv_client_usb.c index 482fc89..038fa6f 100644 --- a/hal/hal_uv_client_usb.c +++ b/hal/hal_uv_client_usb.c @@ -111,8 +111,8 @@ static void usb_uv_close(adb_client_t *c) { adb_client_usb_t *client = (adb_client_usb_t*)c; /* Close pipe and cancel all pending write requests if any */ - uv_close((uv_handle_t*)&client->write_pipe, NULL); uv_close((uv_handle_t*)&client->read_pipe, usb_uv_on_close); + uv_close((uv_handle_t*)&client->write_pipe, NULL); } static const adb_client_ops_t adb_usb_uv_ops = { From 6b47b63c14d4bcef45df1ecb7265a93109fbf56c Mon Sep 17 00:00:00 2001 From: dongjiuzhu1 Date: Thu, 30 Nov 2023 11:46:31 +0800 Subject: [PATCH 2/8] microADB: fix memory leaks when adbd exit host_memalign sim/posix/sim_hostmemory.c:168 host_realloc sim/posix/sim_hostmemory.c:193 mm_realloc sim/sim_heap.c:334 mm_malloc sim/sim_heap.c:272 mm_zalloc sim/sim_heap.c:402 mm_calloc sim/sim_heap.c:387 calloc umm_heap/umm_calloc.c:73 uv__calloc libuv/src/uv-common.c:94 uv_loop_init libuv/src/unix/loop.c:40 uv_default_loop libuv/src/uv-common.c:821 adb_hal_create_context microADB/hal/hal_uv.c:35 adbd_main /home/djz/workspace/vela_rp/apps/system/adb/adb_main.c:151 nxtask_startup sched/task_startup.c:70 nxtask_start task/task_start.c:134 pre_start sim/sim_initialstate.c:52 Signed-off-by: dongjiuzhu1 --- hal/hal_uv.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hal/hal_uv.c b/hal/hal_uv.c index 9452c09..248fd0a 100644 --- a/hal/hal_uv.c +++ b/hal/hal_uv.c @@ -50,7 +50,10 @@ adb_context_t* adb_hal_create_context(void) { } void adb_hal_destroy_context(adb_context_t *context) { - UNUSED(context); + adb_context_uv_t *adbd = + container_of(context, adb_context_uv_t, context); + + uv_loop_close(adbd->loop); } adb_client_t *adb_hal_create_client(size_t size) { From 262457c25b7cd465181904bf5444e97dd6737596 Mon Sep 17 00:00:00 2001 From: dongjiuzhu1 Date: Thu, 14 Dec 2023 21:33:42 +0800 Subject: [PATCH 3/8] microADB: call service_close to send CLSE frame when child task exit Signed-off-by: dongjiuzhu1 --- hal/shell_service_uv.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hal/shell_service_uv.c b/hal/shell_service_uv.c index 0971dec..4b67661 100644 --- a/hal/shell_service_uv.c +++ b/hal/shell_service_uv.c @@ -89,7 +89,9 @@ static int shell_set_cloexec(int fd) { static void on_child_exit(uv_process_t *process, int64_t exit_status, int term_signal) { ash_service_t *svc = (ash_service_t*)process->data; + adb_client_uv_t *client = (adb_client_uv_t *)svc->shell_pipe.data; + adb_service_close(&client->client, &svc->service, NULL); adb_log("shell %d<->%d exited with status %ld, signal %d\n", svc->service.id, svc->service.peer_id, exit_status, term_signal); From 25236f019e1e932b16f48535fdb56f7d2eb5c13b Mon Sep 17 00:00:00 2001 From: dongjiuzhu1 Date: Thu, 14 Dec 2023 22:46:49 +0800 Subject: [PATCH 4/8] microADB: avoid kick when read/write pipe had beed close __assert nuttx/libs/libc/assert/lib_assert.c:36 usb_uv_kick apps/system/adb/microADB/hal/hal_uv_client_usb.c:197 adb_hal_apacket_release /media/liangchaozhong/ssd/x4b/apps/system/adb/microADB/hal/hal_uv_packet.c:55 adb_uv_close_client apps/system/adb/microADB/hal/hal_uv.c:100 usb_uv_on_close apps/system/adb/microADB/hal/hal_uv_client_usb.c:209 uv__finish_close apps/system/libuv/libuv/src/unix/core.c:352 adb_hal_run apps/system/adb/microADB/hal/hal_uv.c:69 adbd_main Signed-off-by: dongjiuzhu1 --- hal/hal_uv_client_usb.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hal/hal_uv_client_usb.c b/hal/hal_uv_client_usb.c index 038fa6f..45fa1a5 100644 --- a/hal/hal_uv_client_usb.c +++ b/hal/hal_uv_client_usb.c @@ -88,6 +88,10 @@ static int usb_uv_write(adb_client_t *c, apacket *p) { static void usb_uv_kick(adb_client_t *c) { adb_client_usb_t *client = container_of(c, adb_client_usb_t, uc.client); + if (uv_is_closing((uv_handle_t*)&client->read_pipe)) { + return; + } + if (!uv_is_active((uv_handle_t*)&client->read_pipe)) { /* Restart read events */ int ret = uv_read_start((uv_stream_t*)&client->read_pipe, From e70f524e218439cde3820afdfb80d61d653c1836 Mon Sep 17 00:00:00 2001 From: wangjianyu3 Date: Fri, 5 Jan 2024 14:55:06 +0800 Subject: [PATCH 5/8] Send response before adb_reboot_impl() Signed-off-by: wangjianyu3 --- adb_client.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/adb_client.c b/adb_client.c index 45a5d13..707d17f 100644 --- a/adb_client.c +++ b/adb_client.c @@ -31,6 +31,8 @@ #include "tcp_service.h" #endif +#define REBOOT_SERVICE ((adb_service_t *)(~0ul)) + /**************************************************************************** * Private Function Prototypes ****************************************************************************/ @@ -152,7 +154,7 @@ static void handle_open_frame(adb_client_t *client, apacket *p) { else { send_close_frame(client, p, 0, p->msg.arg0); } - } else { + } else if (svc != REBOOT_SERVICE) { if (p->write_len == APACKET_SERVICE_INIT_ASYNC) { /* Service init is asynchronous. Release apacket. */ adb_hal_apacket_release(client, p); @@ -383,10 +385,9 @@ static adb_service_t *adb_service_open(adb_client_t *client, const char *name, a #endif if (!strncmp(name, "reboot:", 7)) { + adb_send_okay_frame(client, p, client->next_service_id++, p->msg.arg0); adb_reboot_impl(&name[7]); - - /* One shot service, skip service register */ - return NULL; + return REBOOT_SERVICE; } } while (0); From 84f46c05f401dec658b226ae6d66195de4850a9e Mon Sep 17 00:00:00 2001 From: wangjianyu3 Date: Wed, 10 Jan 2024 20:42:52 +0800 Subject: [PATCH 6/8] Check whether client has services before close Signed-off-by: wangjianyu3 --- adb_client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/adb_client.c b/adb_client.c index 707d17f..5198830 100644 --- a/adb_client.c +++ b/adb_client.c @@ -409,7 +409,7 @@ void adb_service_close(adb_client_t *client, adb_service_t *svc, apacket *p) { goto exit_free_service; } - while (cur_svc->next) { + while (cur_svc != NULL && cur_svc->next != NULL) { if (cur_svc->next == svc) { cur_svc->next = svc->next; goto exit_free_service; From 868a51c21e40d5fc166678b598c71ce3a189cb29 Mon Sep 17 00:00:00 2001 From: dongjiuzhu1 Date: Mon, 15 Jan 2024 14:37:06 +0800 Subject: [PATCH 7/8] adb/microadb: Copy packet to svc to prevent the current packet from being overwritten When the windows/mac PC sends multiple files in a write packet, and the last DONE packet sent is incomplete, the remaining bytes of DONE are in the next wirte packet, which will lead to the following two situations. 1. When the sync message has not been completely received, we should maintain the state machine state current frame: 2f 63 6f 6d 2e 76 65 6c 61 2e 63 68 61 72 74 2e /com.vela.chart. 64 65 6d 6f 2f 69 31 38 6e 2f 65 6e 2e 6a 73 6f demo/i18n/en.jso 6e 2c 33 33 32 30 36 44 41 54 41 13 00 00 00 7b n,33206DATA....{ 22 61 22 3a 7b 22 62 22 3a 22 68 65 6c 6c 6f 22 "a":{"b":"hello" 7d 7d 44 4f 4e 45 88 e2 }}DONE.. next frame: 57 52 54 45 3a 00 00 00 01 00 00 00 00 04 00 00 WRTE:........... c6 ac 01 00 a8 ad ab ba 88 65 53 45 4e 44 43 00 .........eSENDC. 00 00 2f 64 61 74 61 2f 63 6f 6d 2e 76 65 6c 61 ../data/com.vela 2e 63 68 61 72 74 2e 64 65 6d 6f 2f 63 6f 6d 2e .chart.demo/com. 76 65 6c 61 2e 63 68 61 72 74 2e 64 65 6d 6f 2f vela.chart.demo/ The complete DONE package is: | 44 4f 4e 45 88 e2 | 88 65 | current next 2. When the device receives DONE, it needs to reply OKAY to the HOST.If the current apcket is reused, OKAY will overwrite the unprocessed content, so a local buffer needs to be used. before wirte: 57 52 54 45 3a 00 00 00 01 00 00 00 00 04 00 00 WRTE:........... c6 ac 01 00 a8 ad ab ba 88 65 53 45 4e 44 43 00 .........eSENDC. after write: 57 52 54 45 3a 00 00 00 01 00 00 00 00 04 00 00 WRTE:........... c6 ac 01 00 a8 ad ab ba 41 59 00 00 00 00 00 00 ........AY...... ^ ^ the buffer ".eSENDC" -> "AY......" Signed-off-by: dongjiuzhu1 --- file_sync_service.c | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/file_sync_service.c b/file_sync_service.c index 1f52341..b78b612 100644 --- a/file_sync_service.c +++ b/file_sync_service.c @@ -99,6 +99,7 @@ enum { typedef struct afs_service_s { adb_service_t service; uint8_t *packet_ptr; + uint8_t *payload; uint8_t state; unsigned cmd; @@ -262,6 +263,23 @@ static int create_path_directories(char *name) return 0; } +static uint8_t *get_payload(afs_service_t *svc, apacket *p) +{ + if (svc->size > 0) { + if (svc->payload == NULL) { + svc->payload = malloc(CONFIG_ADBD_PAYLOAD_SIZE); + if (svc->payload == NULL) { + return NULL; + } + } + + memcpy(svc->payload, p->data, p->msg.data_length); + return svc->payload; + } + + return p->data; +} + static void state_reset(afs_service_t *svc) { switch (svc->state) { @@ -730,7 +748,7 @@ static int state_wait_cmd_data(afs_service_t *svc, apacket *p) static int file_sync_on_write(adb_service_t *service, apacket *p) { int ret = 0; afs_service_t *svc = container_of(service, afs_service_t, service); - svc->packet_ptr = p->data; + svc->packet_ptr = get_payload(svc, p); /* Process all packet data */ @@ -779,7 +797,7 @@ static int file_sync_on_write(adb_service_t *service, apacket *p) { static int file_sync_on_ack(adb_service_t *service, apacket *p) { int ret; afs_service_t *svc = container_of(service, afs_service_t, service); - svc->packet_ptr = p->data; + svc->packet_ptr = get_payload(svc, p); /* No data in notify packet */ switch (svc->state) { @@ -791,11 +809,19 @@ static int file_sync_on_ack(adb_service_t *service, apacket *p) { ret = state_process_list(svc, p); break; + case AFS_STATE_PROCESS_SEND_FILE_DATA: case AFS_STATE_PROCESS_SEND_FILE_HDR: case AFS_STATE_PROCESS_SEND_SYM_HDR: + case AFS_STATE_WAIT_CMD_DATA: case AFS_STATE_WAIT_CMD: - /* Nothing to do */ - ret = 0; + /* Since the WRITE frame can contain multiple incomplete + * combinations of ID_SEND and ID_DONE, when the pc replies to ID_DONE, + * the OKAY frame sent can be at any time in the state machine. + * At this time, we should not reset the state machine and continue + * to process the next frame status. + */ + + ret = 1; break; default: @@ -817,6 +843,7 @@ static int file_sync_on_ack(adb_service_t *service, apacket *p) { static void file_sync_on_close(struct adb_service_s *service) { afs_service_t *svc = container_of(service, afs_service_t, service); state_reset(svc); + free(svc->payload); free(svc); } @@ -842,6 +869,7 @@ adb_service_t* file_sync_service(const char *params) } service->size = 0; + service->payload = NULL; service->state = AFS_STATE_WAIT_CMD; service->service.ops = &file_sync_ops; From 0e3390d8e13120199d7b34fbf95cd335ac350aab Mon Sep 17 00:00:00 2001 From: guohao15 Date: Wed, 6 Mar 2024 16:05:50 +0800 Subject: [PATCH 8/8] adb:do not send sigkill when shell close Signed-off-by: guohao15 --- hal/shell_service_uv.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/hal/shell_service_uv.c b/hal/shell_service_uv.c index 4b67661..dd8ba8b 100644 --- a/hal/shell_service_uv.c +++ b/hal/shell_service_uv.c @@ -221,8 +221,6 @@ static void shell_close(adb_service_t *service) { /* Terminate child process in case it is still running */ - uv_process_kill(&svc->process, SIGKILL); - uv_close((uv_handle_t *)&svc->shell_pipe, shell_close_pipe_callback); }