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 Toscano
diff --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);
signature.asc
Description: This is a digitally signed message part.