Re: [PATCH] quectel: fix use after free

[Denis Kenzior <denkenz@gmail.com>]

Hi Martin,

On 5/28/24 3:26 AM, Martin Hundebøll wrote:
> Exitting ofono before going online with a quectel modem procudes the
> following use-after-free error:
> 
> ^Cofonod[776]: Terminating
> =================================================================
> ==776==ERROR: AddressSanitizer: heap-use-after-free on address 0xb4150734 at pc 0x005ad063 bp 0xbefa26c0 sp 0xbefa26c4
> READ of size 4 at 0xb4150734 thread T0
>      #0 0x5ad060 in ofono_sim_remove_file_watch ../git/src/sim.c:2621
>      #1 0x5ad060 in unwatch_sim_ecc_numbers ../git/src/voicecall.c:2820
>      #2 0x5ad060 in voicecall_unregister ../git/src/voicecall.c:2849
>      #3 0x57f910 in __ofono_atom_unregister ../git/src/modem.c:336
>      #4 0x57f910 in __ofono_atom_unregister ../git/src/modem.c:329
>      #5 0x57f910 in flush_atoms ../git/src/modem.c:492
>      #6 0x57f910 in modem_change_state ../git/src/modem.c:586
>      #7 0x58013e in set_powered ../git/src/modem.c:974
>      #8 0x58054a in __ofono_modem_shutdown ../git/src/modem.c:2279
>      #9 0x58054a in signal_handler ../git/src/main.c:85
> 
> 0xb4150734 is located 4 bytes inside of 8-byte region [0xb4150730,0xb4150738)
> freed by thread T0 here:
>      #0 0xb6a88110  (/lib/libasan.so.8+0x97110) (BuildId: 1374acedfadbe21a32d37a0a1f15e27d16516851)
>      #1 0x641216 in sim_fs_free ../git/src/simfs.c:123
>      #2 0x641216 in sim_fs_free ../git/src/simfs.c:103
>      #3 0x5de12c in sim_remove ../git/src/sim.c:3239
>      #4 0x57f95a in flush_atoms ../git/src/modem.c:495
>      #5 0x57f95a in modem_change_state ../git/src/modem.c:586
>      #6 0x58013e in set_powered ../git/src/modem.c:974
>      #7 0x58054a in __ofono_modem_shutdown ../git/src/modem.c:2279
>      #8 0x58054a in signal_handler ../git/src/main.c:85
> 
> previously allocated by thread T0 here:
>      #0 0xb6a8891c in __interceptor_calloc (/lib/libasan.so.8+0x9791c) (BuildId: 1374acedfadbe21a32d37a0a1f15e27d16516851)
>      #1 0x597b3e in sim_fs_context_new ../git/src/simfs.c:155
>      #2 0x597b3e in ofono_sim_context_create ../git/src/sim.c:2549
>      #3 0x597b3e in watch_sim_ecc_numbers ../git/src/voicecall.c:2925
>      #4 0x57f506 in call_watches ../git/src/modem.c:314
>      #5 0x4977be in at_clck_query_cb ../git/drivers/atmodem/sim.c:1612
>      #6 0x556cf4 in at_chat_finish_command ../git/gatchat/gatchat.c:465
>      #7 0x5583ae in at_chat_handle_command_response ../git/gatchat/gatchat.c:527
>      #8 0x5583ae in have_line ../git/gatchat/gatchat.c:606
>      #9 0x5583ae in new_bytes ../git/gatchat/gatchat.c:765
>      #10 0x559ef6 in received_data ../git/gatchat/gatio.c:122
>      #11 0x563400 in dispatch_sources ../git/gatchat/gatmux.c:184
>      #12 0x56402c in received_data ../git/gatchat/gatmux.c:272
> 
> The reason is the voicecall atom holding a reference to the sim atom,
> which is read in the voicecall_unregister() path. Avoid the error by
> simply instantiating the sim atom before the voicecall atom, which makes
> the latter being unregistered first.

Thanks for the reported.  This should really be fixed in the core.  Can you try 
the following patch:

https://patchwork.kernel.org/project/ofono/patch/20240528220642.251435-1-denkenz@gmail.com/

> ---
>   plugins/quectel.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 

Regards,
-Denis