On error, decode_programmer_archive() and decode_sahara_config() leak
both the input blob and any images partially loaded before the failure.
Fix by extending their error paths to call sahara_images_free() and
free the blob, mirroring the cleanup already done on success. Also
introduce sahara_images_free() to consolidate teardown, drop the
misleading 'const' from sahara_image.name, and release sahara_images
in qdl_flash() on all exit paths.
Signed-off-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
The `len` parameter is size_t (unsigned) while `actual` from
libusb_bulk_transfer() is int (signed). Their direct comparison
on line 331 triggers -Wsign-compare: if `actual` were somehow
negative, the implicit conversion to size_t would produce a
large positive value, making the comparison silently wrong.
Fixes: b4030fabe6 ("usb: Fix checkpatch warning about unnecessary parenthesis")
Signed-off-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
malloc() can return NULL on allocation failure. Without a check,
the subsequent read(fd, ptr, len) dereferences a NULL pointer,
causing undefined behavior (typically a segfault).
Signed-off-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
The sector probe buffer in firehose_try_configure() is a small,
bounded allocation (max 4096 bytes) used only within this function
scope. Replace malloc() with alloca() to:
- avoid the need for a free() path and NULL-check error handling
- simplify control flow, as the buffer is automatically released
on function return
The upper bound is compile-time constant (largest entry in
sector_sizes[]), well within safe stack allocation limits.
Signed-off-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
The `part_entry_lba` field in `struct gpt_header` is uint64_t, but
the local `lba` variable was declared as `unsigned int`, silently
truncating values above UINT_MAX. The buffer `lba_buf[10]` is also
too small: a 64-bit value can be up to 20 decimal digits, requiring
at least 21 bytes including the null terminator. Combined with an
unchecked sprintf(), this is a stack buffer overflow for any LBA
value exceeding 9 digits.
Fix by:
- declaring `lba` as uint64_t to match the source type
- increasing lba_buf to 21 bytes
- using snprintf() with sizeof to prevent overruns
- using PRIu64 for correct format specifier
Signed-off-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
This got broken during some refactoring, pass the correct values through.
Fixes: 841a0a8e7f ("firehose: Improve startup time significantly")
Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
Commit '44e7be00aca0 ("sahara: Drop "single image" concept")' cleaned up
the handling of programmer selection, but in the new flow failed to
propagate the successful decoding of the sahara_config.
Fixes: 44e7be00ac ("sahara: Drop "single image" concept")
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Rather than mixing subcommands and the default "flash" operation, split
the flashing mechanism out into its own subcommand function to clean up
the main function.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Dumping all the DDR takes significant time and there's currently no
user-visible feedback provided to indicate that even the process has
started.
Solve this by wiring up the ux module and provide a progress bar while
dumping segments, as well as information as the segments are skipped or
dumped.
Add missing ux_init() to the ramdump setup, and make sure to clamp value
to max in the progress calculation, to avoid funky issues when progress
is made beyond the size of the chunk.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Providing qdl-ramdump as a separate executable has resulted in it not
being part of several of the distributions that include qdl. Also, users
that knows that qdl supports ramdumps are looking for it "in" qdl, not a
related tool.
Make "ramdump" a subcommand, just like "qdl list", to improve the
ergonomics of the tool.
The implementation is a (almost) verbatim copy of ramdump.c. The
existing executable is kept in order to not break those distributions
that explicitly do package said executable.
At some point we should figure out how to drop this.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
It might not be obvious to the user that an "invalid image id" is the
result of them not providing the correct programmer loaders.
Make the error message more user friendly.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
The Sahara implementation was written without understanding of the
special meaning of image id #13. As the implementation grew support for
multiple images the special casing of "single image" was introduced, and
this spread to the calling bodies.
Prior to the introduction of multi-programmer platforms this didn't
matter, the logic was fairly simple and usage was straight forward.
But when a single programmer image is provided on a multi-programmer
target "single_image" is true and hence the image id is ignored on the
first read, the one provided file is loaded. The typical outcome is that
the following SAHARA_END_OF_IMAGE_CMD fails with a message stating
"non-successful end-of-image result".
Few users draws the conclusion that this is because they didn't provide
the appropriate programmers.
But 13 is the image id for the programmer, so it should be fine to drop
the special logic. This results in a (somewhat) more helpful error
message telling the user that an invalid image is being requested.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
When working with multiple devices and the --serial argument, the serial
numbers must be known.
Add a new command "qdl list" to list the connected devices in EDL mode
and their serial number.
Signed-off-by: lucarin91 <lucarin@protonmail.com>
[bjorn: Replaced --list with list, changed output to only print serial, some stylistic changes]
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Commit 'a10cf7f5da7a ("firehose: Increase image write timeout to 60
seconds")' bumped the timeout for writes in the program operation, per
observed behavior. This was confirmed to solve the problems seen when
programming SPINOR in Nord devices
New reports (and measurements) shows the write taking a negligible
amount of time and the read of the following "program ack" taking the
whole reported ~35 second, resulting in a timeout.
The proprietary tools has a write timeout of 60 seconds in some
operating systems and no timeout for others, and a read timeout of 120
seconds.
Update this timeout for the "program ack" to 120 seconds, to match the
proprietary tools.
Fixes: a10cf7f5da ("firehose: Increase image write timeout to 60 seconds")
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
The expected digest value changed after recent updates to the
generated image content. The hardcoded checksum no longer matched
the artifact produced by the current build, causing verification
to fail.
Update the expected digest so checksum verification succeeds with
the latest generated image.
Signed-off-by: Yvonne Kaire <ykaire@qti.qualcomm.com>
On RB3 and IQ9, devices may intermittently hang if reboot is issued too
soon after flashing, sometimes requiring a full power cycle to recover.
During testing, shorter post-flash delays were more likely to reproduce
the issue. Add a conservative 10s delay between flash completion and
reboot to provide a stabilization window.
This mirrors long-standing behavior in fh_loader and PCAT.
Results (30 runs each, Rubik Pi/IQ8/IQ9):
- QDL 2.4 baseline: 18/30 flash attempts failed
- QDL 2.4 + this change: 30/30 passes
Signed-off-by: Yvonne Kaire <ykaire@qti.qualcomm.com>
Commit 'ff260604e774 ("sahara: Load programmer at parse time")' added a
convenient debug function to dump the list of loaded images, but failed
to recognize that when this is invoked from qdl-ramdump `images` will be
NULL, and hence we have a segfault.
Make the debug call conditional on the presence of images.
Fixes: ff260604e7 ("sahara: Load programmer at parse time")
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Commit '5b768d8be070 ("qdl: Allow multiple Sahara images")' introduced
support for specifying multiple programmer images, by splitting the
requested programmer on ':'-character and use the pair as id and
filename specifiers. This obviously breaks absolute paths in Windows.
Reorder the programmer decoder such that if the specifier starts with an
id followed by a ':', then the specifier must be a comma-separated list
of id:filename (where ',' is not allowed in the filename).
In all other cases consider the whole specifier the one file to use as
programmer, programmer archive, or programmer specifier XML. This brings
back the ability to specify programmer by absolute path in Windows.
The parsing of the programmer specifier is also altered to not tokenize
on ':' but instead only find the first one and then treat the rest as
the filename. This makes it possible use absolute paths here as well.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Fix the capitalization of the 'ZlpAwareHost' and 'Verbose'
options, since the mis-capitalized versions seem to have no
effect on programmer behavior.
Signed-off-by: Steve Moskovchenko <stevemo@skydio.com>
[bjorn: Updated VIP digest]
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
If no table(s) of digests have been found, we'll hit an
integer underflow while trying to clean up n-1 table
entries. Fix that.
Signed-off-by: Steve Moskovchenko <stevemo@skydio.com>
Signed-off-by: Jerry Zhang <Jerry@skydio.com>
Checkpatch was warning about unnecessary parenthesis in the change to
the ZLP handling, but Github apparently doesn't highlight warnings.
Fix this to avoid leaving the building with broken windows.
Fixes: b9ad4ceaf8 ("firehose/usb: Explicitly handle ZLP on USB read transfers")
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
The somewhat arbitrary timeout of 30 seconds was chosen by observing a
worst case time of ~15 seconds when flashing Hamoa and Nord. But, as
reported, flashing SAIL on Nord sometimes needs this timeout to be ~35
seconds.
While the relationship between the different factors and the needed
timeout isn't fully understood, bump the value to 60 seconds to make it
work for this case as well.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
The Windows builds suddenly fails with:
Cannot find path 'C:\a\_temp\msys64\mingw64\bin\liblzma-5.dll' because it does not exist.
The internet indicates that this is caused by the lack of
mingw-w64-x86_64-xz, so let's introduce this.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
