https://bugs.kde.org/show_bug.cgi?id=523105
Bug ID: 523105
Summary: HEIF: dangling stack pointer to heif_reader causes
SIGSEGV when decoding HEIF/HEIC from a non-sequential
QIODevice (regression in 6.28.0)
Classification: Frameworks and Libraries
Product: frameworks-kimageformats
Version First 6.28.0
Reported In:
Platform: Arch Linux
OS: Linux
Status: REPORTED
Severity: normal
Priority: NOR
Component: general
Assignee: [email protected]
Reporter: [email protected]
CC: [email protected], [email protected]
Target Milestone: ---
Since kimageformats 6.28.0, decoding any HEIF/HEIC image from a file crashes
with
SIGSEGV. 6.27.0 with the identical libheif version is unaffected.
On a Plasma desktop this is very visible: every thumbnail job for a HEIC file
kills
its kioworker. Copying a folder of HEIC files to the clipboard made Plasma
spawn
(and crash) ~130 kioworkers on each session start, because Klipper restores the
clipboard entry and requests previews for it. Gwenview and Dolphin previews are
affected the same way.
The crash is not file-specific: every HEIC I tried crashes, and the same files
decode
correctly with heif-dec from the same libheif build, so the files themselves
are valid.
STEPS TO REPRODUCE
Build this against Qt6Gui:
g++ -fPIC t.cpp -o t $(pkg-config --cflags --libs Qt6Gui)
// t.cpp
#include <QGuiApplication>
#include <QImageReader>
#include <QImage>
#include <cstdio>
int main(int argc, char **argv)
{
QGuiApplication app(argc, argv);
QImageReader r(argv[1]); // plain file path -> QFile ->
non-sequential
QImage img = r.read();
if (img.isNull()) {
printf("FAIL: %s\n", qPrintable(r.errorString()));
return 2;
}
printf("OK %dx%d\n", img.width(), img.height());
return 0;
}
Then run it on any HEIC file:
$ QT_QPA_PLATFORM=offscreen ./t some_image.HEIC
OBSERVED RESULT
Segmentation fault (core dumped) -- every time, on every HEIC file.
EXPECTED RESULT
OK 1737x3088
That is exactly what the same binary prints when the 6.27.0 build of
kimg_heif.so is
used via QT_PLUGIN_PATH, with everything else unchanged.
ADDITIONAL INFORMATION
Narrowing it down: feeding the identical bytes through a QIODevice whose
isSequential() returns true decodes fine (OK 1737x3088); only the
non-sequential path
crashes. That is the distinction between the two branches in ensureDecoder().
Backtrace -- note frame #0, a jump through a garbage function pointer:
#0 0x0000000000000000 n/a (n/a + 0x0)
#1 0x00007efc23885b81 n/a (libheif.so.1 + 0x4bb81)
#2 0x00007efc238cb9e8 n/a (libheif.so.1 + 0x919e8)
#3 0x00007efc23908e11 n/a (libheif.so.1 + 0xcee11)
...
#11 0x00007efc2393b0d6 n/a (libheif.so.1 + 0x1010d6)
#12 0x00007efc26c9ce9c n/a (libc.so.6 + 0x9ce9c)
#13 0x00007efc26c9cf19 pthread_once (libc.so.6 + 0x9cf19)
The upper frames are unreliable here -- the unwinder is walking a stack that
has
already been overwritten, which is consistent with the analysis below.
ROOT CAUSE
Introduced by MR !504 ("HEIF: use heif_reader for random access devices",
commit
8768a8cf97), which landed after v6.27.0 and before v6.28.0 -- matching the
observed
good/bad boundary.
In HEIFHandler::ensureDecoder(), src/imageformats/heif.cpp (v6.28.0, ~line
725):
if (dev->isSequential()) {
buffer = deviceRead(dev, kMaxQVectorSize);
err = heif_context_read_from_memory(ctx, static_cast<const void
*>(buffer.constData()), buffer.size(), nullptr);
} else {
heif_reader reader = create_heif_reader_for_qiodevice();
err = heif_context_read_from_reader(ctx, &reader, dev, nullptr);
} // <-- `reader` dies here, but libheif kept `&reader`
`reader` is a local scoped to the else block, but libheif does not copy the
struct.
In libheif 1.23.1, heif_context_read_from_reader() wraps it in
StreamReader_CApi,
which stores a raw pointer and dereferences it lazily on every access
(libheif/bitstream.h):
private:
const heif_reader* m_func_table;
void* m_userdata;
So every later call -- heif_context_get_primary_image_handle(),
heif_decode_image()
-- loads a function pointer out of a stack slot that intervening frames have
long
since clobbered, and jumps to it. Whether that lands on nullptr or on something
worse
is pure luck; the file contents are irrelevant, which is why all HEIC files
fail.
PROPOSED FIX
All four lambdas in create_heif_reader_for_qiodevice() are captureless and the
device
is passed separately as userdata, so the function table is stateless and can
safely be
shared and outlive the context:
} else {
static const heif_reader reader = create_heif_reader_for_qiodevice();
err = heif_context_read_from_reader(ctx, &reader, dev, nullptr);
}
If you would rather avoid a function-local static, making `reader` a member of
HEIFHandler works equally well -- the only requirement is that it outlives ctx.
Note that master is still affected: the same two lines are present at
heif.cpp:848-849 as of commit c748c6c2c6.
SOFTWARE/OS VERSIONS
kimageformats: 6.28.0 (Arch Linux extra)
libheif: 1.23.1
Qt: 6.11.1
KDE Plasma: 6.7.3
KDE Frameworks: 6.28.0
Linux/KDE Plasma: Arch Linux, kernel 7.1.3, x86_64
Confirmed working: kimageformats 6.27.0 with the same libheif 1.23.1 and Qt
6.11.1.
--
You are receiving this mail because:
You are watching all bug changes.
