diff options
| author | Jeff Epler <jepler@gmail.com> | 2020-10-01 09:21:39 -0500 |
|---|---|---|
| committer | Jeff Epler <jepler@gmail.com> | 2020-10-01 09:22:09 -0500 |
| commit | d7c3f811afd924bae8e0b009a4cfadd6fab29584 (patch) | |
| tree | 0ba5909d8fc34fe4a1a85a7159c9a5fe98349285 /shared-module | |
| parent | 9e66c3aee17b93404734f3bfaddc0f326d1bc467 (diff) | |
sharpdisplay: Fix memory corruption across soft-reset
It was incorrect to NULL out the pointer to our heap allocated buffer in
`reset`, because subsequent to framebuffer_reset, but while
the heap was still active, we could call `get_bufinfo` again,
leading to a fresh allocation on the heap that is about to be destroyed.
Typical stack trace:
```
#1 0x0006c368 in sharpdisplay_framebuffer_get_bufinfo
#2 0x0006ad6e in _refresh_display
#3 0x0006b168 in framebufferio_framebufferdisplay_background
#4 0x00069d22 in displayio_background
#5 0x00045496 in supervisor_background_tasks
#6 0x000446e8 in background_callback_run_all
#7 0x00045546 in supervisor_run_background_tasks_if_tick
#8 0x0005b042 in common_hal_neopixel_write
#9 0x00044c4c in clear_temp_status
#10 0x000497de in spi_flash_flush_keep_cache
#11 0x00049a66 in supervisor_external_flash_flush
#12 0x00044b22 in supervisor_flash_flush
#13 0x0004490e in filesystem_flush
#14 0x00043e18 in cleanup_after_vm
#15 0x0004414c in run_repl
#16 0x000441ce in main
```
When this happened -- which was inconsistent -- the display would keep
some heap allocation across reset which is exactly what we need to avoid.
NULLing the pointer in reconstruct follows what RGBMatrix does, and that
code is a bit more battle-tested anyway.
If I had a motivation for structuring the SharpMemory code differently,
I can no longer recall it.
Testing performed: Ran my complicated calculator program over multiple
iterations without observing signs of heap corruption.
Closes: #3473
Diffstat (limited to 'shared-module')
| -rw-r--r-- | shared-module/sharpdisplay/SharpMemoryFramebuffer.c | 8 |
1 files changed, 3 insertions, 5 deletions
diff --git a/shared-module/sharpdisplay/SharpMemoryFramebuffer.c b/shared-module/sharpdisplay/SharpMemoryFramebuffer.c index 71e00d7f0..c23446828 100644 --- a/shared-module/sharpdisplay/SharpMemoryFramebuffer.c +++ b/shared-module/sharpdisplay/SharpMemoryFramebuffer.c @@ -90,10 +90,6 @@ bool common_hal_sharpdisplay_framebuffer_get_pixels_in_byte_share_row(sharpdispl } void common_hal_sharpdisplay_framebuffer_reset(sharpdisplay_framebuffer_obj_t *self) { - if (!allocation_from_ptr(self->bufinfo.buf)) { - self->bufinfo.buf = NULL; - } - if (self->bus != &self->inline_bus #if BOARD_SPI && self->bus != common_hal_board_get_spi() @@ -105,7 +101,9 @@ void common_hal_sharpdisplay_framebuffer_reset(sharpdisplay_framebuffer_obj_t *s } void common_hal_sharpdisplay_framebuffer_reconstruct(sharpdisplay_framebuffer_obj_t *self) { - + if (!allocation_from_ptr(self->bufinfo.buf)) { + self->bufinfo.buf = NULL; + } } void common_hal_sharpdisplay_framebuffer_get_bufinfo(sharpdisplay_framebuffer_obj_t *self, mp_buffer_info_t *bufinfo) { |