espressif / esp-adf

Espressif Audio Development Framework
Other
1.56k stars 688 forks source link

`i2s_driver_cleanup` in `i2s_stream_idf5.c->i2s_stream_init()` is not safe (AUD-5308) #1182

Open yyjdelete opened 7 months ago

yyjdelete commented 7 months ago

INSTRUCTIONS

Before submitting a new issue, please follow the checklist and try to find the answer.

for some app like examples/speech_recognition/wwe, if you switch the order of setup_player() and start_recorder(), and create the 2nd i2s_stream while the first one is working.

The 2nd call to i2s_stream_init will delete the old handle and create an new one, but not mutex for this, and it doesn't check config->uninstall_drv, so another i2s_stream maybe still using it.

https://github.com/espressif/esp-adf/blob/74c88720a0ecdf39a2acdffc55f3705078be1682/components/audio_stream/i2s_stream_idf5.c#L641 https://github.com/espressif/esp-adf/blob/74c88720a0ecdf39a2acdffc55f3705078be1682/components/audio_stream/i2s_stream_idf5.c#L165-L166 https://github.com/espressif/esp-adf/blob/74c88720a0ecdf39a2acdffc55f3705078be1682/components/audio_stream/i2s_stream_idf5.c#L471-L478

It may lead to random crash(i2s reader block forever on deleted mutex or random broken memory). And require an safe way to do it as it does in the old i2s_stream.c

jason-mao commented 7 months ago

@yyjdelete Thanks for your report, let's fix it.

shootao commented 4 months ago

Hi @yyjdelete Please update i2s_stream_idf5.zip and have a try