search

espressif / esp-idf

Espressif IoT Development Framework. Official development framework for Espressif SoCs.
Apache License 2.0
13.33k stars 7.2k forks source link

Validate the input file sizes based on the partition table (IDFGH-13448) #14354

Open vshymanskyy opened 1 month ago

vshymanskyy commented 1 month ago

Is your feature request related to a problem?

If the file size exceeds the partition size, the esptool still finishes with success, presumably producing a broken firmware.

Describe the solution you'd like

No response

Describe alternatives you've considered

No response

Additional context

No response

peterdragun commented 1 month ago

Hi @vshymanskyy, if I understand correctly you would like esptool to detect overlaps based on the partition table?

In the current implementation, esptool is detecting overlaps of binary files when running the merge_bin command. However esptool is not trying to understand the data during merging and does not have any information regarding the partition sizes, so I don't think this would be possible. Detecting such overlaps should be done on the upper level, in some tool that integrates esptool and parttool.

vshymanskyy commented 1 month ago

I think merge_bin has everything it needs to validate the input. If the upper infrastructure implements this check, it effectively can merge the bin by itself. Using esptool for this becomes redundant.

igrr commented 1 month ago

@vshymanskyy Could you please clarify, what is the case when esptool.py merge_bin result would exceed the partition size?

We already validate that the bootloader and the app fit into the partition table, so idf.py build should fail in the first place. Could you please give a more detailed example illustrating the issue? Thanks.

vshymanskyy commented 1 month ago

Yes, for example if i build a single image, and then to merge it with different partition tables (for esp modules with different flash size). Of course, the image headers need to be adjusted, but merge_bin already does this.

igrr commented 1 month ago

I see, thanks for explaining. In this case I'm afraid what @peterdragun mentioned is still the case:

However esptool is not trying to understand the data during merging and does not have any information regarding the partition sizes, so I don't think this would be possible.

Basically, esptool isn't aware of the partition tables, which are an ESP-IDF-specific feature, and it's unlikely that we would add new features into esptool which would be too coupled to ESP-IDF.

The only solution I can recommend now is to run idf.py merge-bin (https://github.com/espressif/esp-idf/pull/13546) command for each flash size. This will result in the app being rebuilt, but if you use ccache this will happen pretty quickly. This way, partition size issues will be detected.

vshymanskyy commented 1 month ago

I'm implementing the check in an outer infrastructure already, i just wanted to bring it to you.