reassign 975387 libgs9 ghostscript/9.53.0~dfsg-1
retitle 975387 wrong size check for display_callback_v2_s struct
severity 975387 serious
forwarded 975387 https://bugs.ghostscript.com/show_bug.cgi?id=703301
tag 975387 + patch
merge 975387 975574
thanks
In data lunedì 21 dicembre 2020 18:23:12 CET, Simon McVittie ha scritto:
> > On my side, rebuilding libspectre1 solved this on my system.
>
> If a simple rebuild with no source changes fixes the symptoms of a bug,
> that might indicate an unintended ABI break in libgs9, or perhaps a bug
> in the old libgs9 headers (but fixed in the new headers) in code that
> gets inlined into libspectre at compile time.
Both of them are issues in ghostscript anyway.
The rebuild, while "easy as it seems", does not consider an important
fact. From what I see, libgs got new APIs in 9.53.0, and libspectre
does not (optionally) use any of them. This means that a rebuild most
probably would not cause libspectre to require the newer ghostscript,
migrating instantly to testing. Considering that what we have here
smells like a behaviour change, this means making libspectre unusable
for the users of testing, and I do not find this acceptable.
I checked the changes between ghostscript 9.52.1 and 9.53.3, and I found
one thing: gs 9.53.0 reworks a bit the display device stuff, adding a
v3 device_callback struct, and keeping the support for what is now v2:
http://git.ghostscript.com/?p=ghostpdl.git;a=commitdiff;h=eed3bad23510e59278bdaa5f7d0ab01fc1a1c21b;hp=04e937862eaa7e66bb9a87109874112cd354bf6f
display_callback is actually used in libspectre, see spectre-device.c.
Because it relies on DISPLAY_VERSION_MAJOR/DISPLAY_VERSION_MINOR, this
explains why it works after a rebuild, as it will use the
device_callback v3. This also shows that a rebuild is a no-no, as it
will get in the situation I described earlier (i.e. stop working with
ghostscript in testing).
There is actually code in ghostscript to support the old device_callback
(v2, as built in libspectre during the last built), as it can be see in
devices/gdevdsp.c, function display_check_structure:
static int display_check_structure(gx_device_display *ddev)
[...]
else if (ddev->callback->size == sizeof(struct display_callback_v2_s)) {
/* V2 structure with added display_separation callback */
if (ddev->callback->size != sizeof(display_callback))
return_error(gs_error_rangecheck);
[...]
Considering that:
- sizeof(struct display_callback_v2_s) != sizeof(display_callback);
the addition to new stuff to display_callback was the reason for the
new struct in the first place, so of course the two structs do not
have the same size
- the last libspectre build uses display_callback v2, so the check:
ddev->callback->size == sizeof(struct display_callback_v2_s)
is true
then the check "ddev->callback->size != sizeof(display_callback)" will
be always false! Indeed, tried a simple patch to drop this, and evince
shows again PS files without rebuilding libspectre.
I submitted this as ghostscript bug:
https://bugs.ghostscript.com/show_bug.cgi?id=703301
Because of this, I'm reassigning 977754/975387 to ghostscript, merging
also 975574 to them, and setting the proper metadata. I'm also attaching
a copy of the patch I submitted upstream.
Thanks,
--
Pino Toscanodiff --git a/devices/gdevdsp.c b/devices/gdevdsp.c
index 0a66a0278..52087f8d6 100644
--- a/devices/gdevdsp.c
+++ b/devices/gdevdsp.c
@@ -1430,9 +1430,6 @@ static int display_check_structure(gx_device_display *ddev)
}
else if (ddev->callback->size == sizeof(struct display_callback_v2_s)) {
/* V2 structure with added display_separation callback */
- if (ddev->callback->size != sizeof(display_callback))
- return_error(gs_error_rangecheck);
-
if (ddev->callback->version_major != DISPLAY_VERSION_MAJOR_V2)
return_error(gs_error_rangecheck);
Attachment:
signature.asc
Description: This is a digitally signed message part.