Bug#972339: armhf: hpcups crashes with free() invalid pointer for some printers
Hello Didier,
I have retried with the patch in #974828, but it still
crashed with the test files from this bug, therefore
I guess #974828 is similar but unrelated.
Then I took another look at the valgrind runs and found
that these invalid reads and writes also appear at amd64.
After some digging I gave up to understand the pointer
calculations and such and tried to just increase the
allocations and came up with the attached three patches.
(While working at #974828 I found one such "+ 32" in
HPCupsFilter.cpp which might be already a workaround by upstream?)
With these three applied a valgrind run shows no more errors
with amd64 or armhf, and also does not abort at armhf.
As this just allocates a few extra bytes I assume that the
print result should not be different by these patches.
And I hope that memset'ing these buffers has no security
related effects.
For the crash is just the Halftoner patch important.
The other two are currently just for valgrind, but that
might change in future with compiler changes or similar.
What do you think?
Kind regards,
Bernhard
Description: Workaround: Add 32 bytes to allocation ColorMatcher
Bug-Debian: https://bugs.debian.org/972339
Bug-Ubuntu: https://launchpad.net/bugs/1901209
Last-Update: 2021-02-27
==12899== Invalid read of size 1
==12899==at 0x1174D6: Backward16PixelsNonWhite (Halftoner.h:106)
==12899==by 0x1174D6: Halftoner::HTEDiffOpen(Halftoner::THTDitherParms*, unsigned short) (Halftoner.cpp:734)
==12899==by 0x117C67: Halftoner::Process(RASTERDATA*) (Halftoner.cpp:548)
==12899==by 0x115D5F: Process (Pipeline.cpp:72)
==12899==by 0x115D5F: Pipeline::Execute(RASTERDATA*) (Pipeline.cpp:79)
==12899==by 0x115D81: Pipeline::Execute(RASTERDATA*) (Pipeline.cpp:83)
==12899==by 0x10D151: HPCupsFilter::processRasterData(_cups_raster_s*) (HPCupsFilter.cpp:779)
==12899==by 0x10D651: HPCupsFilter::StartPrintJob(int, char**) (HPCupsFilter.cpp:597)
==12899==by 0x4B9BA1F: (below main) (libc-start.c:308)
==12899== Address 0x55f8ed2 is 6 bytes after a block of size 12,100 alloc'd
==12899==at 0x48416F4: operator new[](unsigned int) (vg_replace_malloc.c:425)
==12899==by 0x116011: ColorMatcher::ColorMatcher(ColorMap_s, unsigned int, unsigned int) (ColorMatcher.cpp:63)
==12899==by 0x11101B: Pcl3::Configure(Pipeline**) (Pcl3.cpp:90)
==12899==by 0x115B71: Job::Configure() (Job.cpp:248)
==12899==by 0x115C07: Job::Init(SystemServices*, JobAttributes_s*, Encapsulator*) (Job.cpp:63)
==12899==by 0x10C9C1: HPCupsFilter::startPage(cups_page_header2_s*) (HPCupsFilter.cpp:481)
==12899==by 0x10D273: HPCupsFilter::processRasterData(_cups_raster_s*) (HPCupsFilter.cpp:668)
==12899==by 0x10D651: HPCupsFilter::StartPrintJob(int, char**) (HPCupsFilter.cpp:597)
==12899==by 0x4B9BA1F: (below main) (libc-start.c:308)
--- hplip-3.20.11+dfsg0.orig/prnt/hpcups/ColorMatcher.cpp
+++ hplip-3.20.11+dfsg0/prnt/hpcups/ColorMatcher.cpp
@@ -60,7 +60,8 @@ ColorMatcher::ColorMatcher
EndPlane = K;
}
-Contone = (BYTE *) new BYTE[InputWidth * ColorPlaneCount];
+Contone = (BYTE *) new BYTE[InputWidth * ColorPlaneCount + 32];
+memset(Contone, 0, InputWidth * ColorPlaneCount + 32);
if (Contone == NULL)
{
goto MemoryError;
Description: Workaround: Add 32 bytes to allocation Halftoner
Bug-Debian: https://bugs.debian.org/972339
Bug-Ubuntu: https://launchpad.net/bugs/1901209
Last-Update: 2021-02-27
==144269== Invalid read of size 1
==144269==at 0x114577: Mode9::Process(RASTERDATA*) (Mode9.cpp:332)
==144269==by 0x11EDA4: Process (Pipeline.cpp:72)
==144269==by 0x11EDA4: Pipeline::Execute(RASTERDATA*) (Pipeline.cpp:79)
==144269==by 0x11EDDF: Pipeline::Execute(RASTERDATA*) (Pipeline.cpp:83)
==144269==by 0x11EDDF: Pipeline::Execute(RASTERDATA*) (Pipeline.cpp:83)
==144269==by 0x112677: HPCupsFilter::processRasterData(_cups_raster_s*) (HPCupsFilter.cpp:779)
==144269==by 0x112DE8: HPCupsFilter::StartPrintJob(int, char**) (HPCupsFilter.cpp:597)
==144269==by 0x4C17D09: (below main) (libc-start.c:308)
==144269== Address 0x5a8cc0b is 0 bytes after a block of size 379 alloc'd
==144269==at 0x483950F: operator new[](unsigned long) (vg_replace_malloc.c:431)
==144269==by 0x12047B: Halftoner::Halftoner(PrintMode_s*, unsigned int, int*, int, bool) (Halftoner.cpp:184)
==144269==by 0x11817B: Pcl3::Configure(Pipeline**) (Pcl3.cpp:92)
==144269==by 0x11EA30: Job::Configure() (Job.cpp:248)
==144269==by 0x11EB67: Job::Init(SystemServices*, JobAttributes_s*, Encapsulator*) (Job.cpp:63)
==144269==by 0x111A35: HPCupsFilter::startPage(cups_page_header2_s*) (HPCupsFilter.cpp:481)
==144269==by 0x112792: HPCupsFilter::processRasterData(_cups_raster_s*) (HPCupsFilter.cpp:668)
==144269==by 0x112DE8: HPCupsFilter::StartPrintJob(int, char**) (HPCupsFilter.cpp:597)
==144269==by 0x4C17D09: (below main) (libc-start.c:308)
---
Bug#972339: armhf: hpcups crashes with free() invalid pointer for some printers
On Sat, Feb 27, 2021 at 1:21 PM Bernhard Übelacker wrote: > > I have retried with the patch in #974828, but it still > crashed with the test files from this bug, therefore > I guess #974828 is similar but unrelated. > > Then I took another look at the valgrind runs and found > that these invalid reads and writes also appear at amd64. > > After some digging I gave up to understand the pointer > calculations and such and tried to just increase the > allocations and came up with the attached three patches. > > (While working at #974828 I found one such "+ 32" in > HPCupsFilter.cpp which might be already a workaround by upstream?) > > With these three applied a valgrind run shows no more errors > with amd64 or armhf, and also does not abort at armhf. > > As this just allocates a few extra bytes I assume that the > print result should not be different by these patches. > And I hope that memset'ing these buffers has no security > related effects. > > For the crash is just the Halftoner patch important. > The other two are currently just for valgrind, but that > might change in future with compiler changes or similar. > > What do you think? For the 0079 patch, this is probably a little more efficient since it avoids a mod which may be implemented as a division: PlaneSize= (OutputWidth[i]+7)/8; // doublecheck ... should already be divisible by 8 The thing that is not clear to me... What is the datatype of OutputWidth[i]? If it is a 16-bit type (or larger), then you may want: const size_t width = sizeof(OutputWidth[0])*CHAR_BITS; PlaneSize= (OutputWidth[i]+width-1)/8; Jeff
Bug#972339: armhf: hpcups crashes with free() invalid pointer for some printers
Control: found -1 3.21.2+dfsg1-1 Hello there Bernhard, (CC'ing d-arm for help) Sadly, I could confirm on a local armhf QEMU instance that this serious bug is still present, in sid and bullseye; the steps in https://bugs.debian.org/972339#10 still apply and trigger the SIGABRT. Although I understand what you're saying in theoretical terms here, I'm completely at loss to propose a patch: I'm way over my head with my 10+years- old C and gdb competences. In the absence of any interest from upstream, I need help to fix hplip on armhf. (Note that amd64 is apparently also affected; see #974828) Whoever willing to help; if you need anything from me (as maintainer), please ask! I'm happy to explain my use of git-debrebase, or provide a different git history if it helps, I mostly don't want to be in the way of a fix! Humbly, OdyX Le samedi, 24 octobre 2020, 14.05:04 h CET Bernhard Übelacker a écrit : > I could reproduce this issue too. > > Attached is a valgrind run showing one invalid write > and a gdb session showing the issue. > > It looks like mallocs management data, which resides in the 8 bytes > before a returned pointer, gets overwritten and therefore > the free fails because "mchunk_size" is then 0. > > Kind regards, > Bernhard > > > Old value = 6057 > New value = 0 > __memcpy_neon () at ../sysdeps/arm/armv7/multiarch/memcpy_impl.S:295 > warning: Source file is more recent than executable. > 295 tst count, #4 > 1: compressBuf = named `this'> 2: /x *(int*)(0x7f5f43e8-4) = 0x0 > (gdb) bt > #0 __memcpy_neon () at ../sysdeps/arm/armv7/multiarch/memcpy_impl.S:295 > #1 0x7f55b8d2 in memcpy (__len=379, __src=, > __dest=) at > /usr/include/arm-linux-gnueabihf/bits/string_fortified.h:34 #2 > Mode9::Process (this=0x7f5e0e70, input=0x7f5e0e84) at > prnt/hpcups/Mode9.cpp:405 #3 0x7f562de0 in Pipeline::Process > (raster=, this=0x7f5d7340) at prnt/hpcups/Pipeline.cpp:79 #4 > Pipeline::Execute (this=0x7f5d7340, InputRaster=) at > prnt/hpcups/Pipeline.cpp:79 #5 0x7f562e02 in Pipeline::Execute > (this=0x7f5e6b88, InputRaster=) at > prnt/hpcups/Pipeline.cpp:83 #6 0x7f562e02 in Pipeline::Execute > (this=0x7f5e6b70, InputRaster=) at > prnt/hpcups/Pipeline.cpp:83 #7 0x7f55a20a in > HPCupsFilter::processRasterData (this=0x7f5b87c4 , > cups_raster=) at prnt/hpcups/HPCupsFilter.cpp:766 #8 > 0x7f55a6ee in HPCupsFilter::StartPrintJob (this=0x7f5b87c4 , > argc=6, argv=0xbefff7b4) at prnt/hpcups/HPCupsFilter.cpp:584 #9 0xb6bd9a20 > in __libc_start_main (main=0x7f5587d1 , argc=6, > argv=0xbefff7b4, init=, fini=0x7f56ed5d <__libc_csu_fini>, > rtld_fini=0xb6fe1075 <_dl_fini>, stack_end=0xbefff7b4) at libc-start.c:308 > #10 0x7f55889c in _start () at prnt/hpcups/HPCupsFilter.cpp:919 > > > https://sources.debian.org/src/hplip/3.21.2+dfsg1-1/prnt/hpcups/Mode9.cpp/#L > 405 -- OdyX signature.asc Description: This is a digitally signed message part.
Bug#972339: armhf: hpcups crashes with free() invalid pointer for some printers
Control: tags -1 +help Hello there Paul, Le jeudi, 11 février 2021, 16.55:09 h CET Paul Gevers a écrit : > On Fri, 16 Oct 2020 14:23:59 +0200 Didier 'OdyX' Raboud wrote: > > According to the 3.20.9-3 armhf auutopkgtest run for migration testing; > > https://ci.debian.net/data/autopkgtest/testing/armhf/h/hplip/7460676/log.g > > z > > > > hpcups sometimes crashes with free(): invalid pointer. For instance, it > > seems that setting up a 'drv:///hpcups.drv/hp-officejet_pro_1150c.ppd' > > printer will let hpcups crash. > > Just to have the information for the release process, do you think this > is a regression compared to buster, or did you just found out now > because of autopkgtest? I found out because of autopkgtest only. I think it's a real bug (printing a test PDF to a specific printer should naturally work), unlikely to be fixed by upstream (we carry 77 patches, none of which ever got merged), on a non- classical architecture. It's very likely not a regression from buster, but I couldn't formally confirm this. > Is there any progress on this issue? No, and I don't expect to make progress myself there; my C-fu is way too limited. I hope that helps, and sure hope you're well! Cheers, OdyX signature.asc Description: This is a digitally signed message part.
Bug#972339: armhf: hpcups crashes with free() invalid pointer for some printers
Dear Didier, On Fri, 16 Oct 2020 14:23:59 +0200 Didier 'OdyX' Raboud wrote: > According to the 3.20.9-3 armhf auutopkgtest run for migration testing; > https://ci.debian.net/data/autopkgtest/testing/armhf/h/hplip/7460676/log.gz > > hpcups sometimes crashes with free(): invalid pointer. For instance, it > seems that setting up a 'drv:///hpcups.drv/hp-officejet_pro_1150c.ppd' > printer will let hpcups crash. Just to have the information for the release process, do you think this is a regression compared to buster, or did you just found out now because of autopkgtest? Is there any progress on this issue? Paul OpenPGP_signature Description: OpenPGP digital signature
Bug#972339: armhf: hpcups crashes with free() invalid pointer for some printers
Dear Maintainer, I could reproduce this issue too. Attached is a valgrind run showing one invalid write and a gdb session showing the issue. It looks like mallocs management data, which resides in the 8 bytes before a returned pointer, gets overwritten and therefore the free fails because "mchunk_size" is then 0. Kind regards, Bernhard Old value = 6057 New value = 0 __memcpy_neon () at ../sysdeps/arm/armv7/multiarch/memcpy_impl.S:295 warning: Source file is more recent than executable. 295 tst count, #4 1: compressBuf = 2: /x *(int*)(0x7f5f43e8-4) = 0x0 (gdb) bt #0 __memcpy_neon () at ../sysdeps/arm/armv7/multiarch/memcpy_impl.S:295 #1 0x7f55b8d2 in memcpy (__len=379, __src=, __dest=) at /usr/include/arm-linux-gnueabihf/bits/string_fortified.h:34 #2 Mode9::Process (this=0x7f5e0e70, input=0x7f5e0e84) at prnt/hpcups/Mode9.cpp:405 #3 0x7f562de0 in Pipeline::Process (raster=, this=0x7f5d7340) at prnt/hpcups/Pipeline.cpp:79 #4 Pipeline::Execute (this=0x7f5d7340, InputRaster=) at prnt/hpcups/Pipeline.cpp:79 #5 0x7f562e02 in Pipeline::Execute (this=0x7f5e6b88, InputRaster=) at prnt/hpcups/Pipeline.cpp:83 #6 0x7f562e02 in Pipeline::Execute (this=0x7f5e6b70, InputRaster=) at prnt/hpcups/Pipeline.cpp:83 #7 0x7f55a20a in HPCupsFilter::processRasterData (this=0x7f5b87c4 , cups_raster=) at prnt/hpcups/HPCupsFilter.cpp:766 #8 0x7f55a6ee in HPCupsFilter::StartPrintJob (this=0x7f5b87c4 , argc=6, argv=0xbefff7b4) at prnt/hpcups/HPCupsFilter.cpp:584 #9 0xb6bd9a20 in __libc_start_main (main=0x7f5587d1 , argc=6, argv=0xbefff7b4, init=, fini=0x7f56ed5d <__libc_csu_fini>, rtld_fini=0xb6fe1075 <_dl_fini>, stack_end=0xbefff7b4) at libc-start.c:308 #10 0x7f55889c in _start () at prnt/hpcups/HPCupsFilter.cpp:919 https://sources.debian.org/src/hplip/3.20.5+dfsg0-3/prnt/hpcups/Mode9.cpp/#L405 # Bullseye/testing chroot 2020-10-23 running on Android/LineageOS kernel apt update apt dist-upgrade apt install mc htop psmisc net-tools strace sshfs wget gdb gdbserver cups printer-driver-hpcups printer-driver-hpcups-dbgsym apt build-dep libc6 root@localhost:~# lscpu Architecture: armv7l Byte Order: Little Endian CPU(s): 4 On-line CPU(s) list: 0 Off-line CPU(s) list: 1-3 Thread(s) per core: 1 Core(s) per socket: 1 Socket(s):1 Vendor ID:Qualcomm Model:0 Model name: Krait Stepping: 0x1 CPU max MHz: 1728, CPU min MHz: 384, BogoMIPS: 13.50 Flags:swp half thumb fastmult vfp edsp neon vfpv3 tls vfpv4 idiva idivt root@localhost:~# uname -a Linux localhost 3.4.113-g2fff5b1955c0 #1 SMP PREEMPT Sun Mar 8 06:23:52 CST 2020 armv7l GNU/Linux groupadd -g 3001 aid_net_bt_admin groupadd -g 3002 aid_net_bt groupadd -g 3003 aid_inet groupadd -g 3004 aid_net_raw groupadd -g 3005 aid_net_admin groupadd -g 3006 aid_net_bw_stats groupadd -g 3007 aid_net_bw_acct groupadd -g 3008 aid_net_bt_stack usermod -G 3003,3004 -a root usermod -G 3003 -a benutzer usermod -g 3003 -G 3003,3004 -a _apt root@localhost:~# dpkg -l | grep driver-hpcups ii printer-driver-hpcups 3.20.5+dfsg0-3+b1 armhf HP Linux Printing and Imaging - CUPS Raster driver (hpcups) ii printer-driver-hpcups-dbgsym 3.20.5+dfsg0-3+b1 armhf debug symbols for printer-driver-hpcups mkdir /home/benutzer/source/libc6/orig -p cd/home/benutzer/source/libc6/orig apt source libc6 cd wget https://sources.debian.org/data/main/h/hplip/3.20.9+dfsg0-3/ppd/hpcups/hp-officejet_pro_1150c.ppd gzip hp-officejet_pro_1150c.ppd export PPD=/home/benutzer/hp-officejet_pro_1150c.ppd.gz /usr/lib/cups/filter/pdftopdf 1 debian '' 1 '' print_step_1.pdf /usr/lib/cups/filter/gstoraster 1 debian '' 1 '' print_step_2.raster /usr/lib/cups/filter/hpcups 1 debian '' 1 '' print_step_3.hpcups /usr/bin/gdbserver localhost: /usr/lib/cups/filter/hpcups 1 debian '' 1 '' print_step_3.hpcups gdb -q set width 0 set pagination off target remote localhost: cont benutzer@localhost:~$ /usr/bin/gdbserver localhost: /usr/lib/cups/filter/hpcups 1 debian x 1 x print_step_3.hpcups Process /usr/lib/cups/filter/hpcups created; pid = 9723 Listening on port Remote debugging from host ::1, port 42055 STATE: -marker-supply-low-warning PAGE: 1 1 free(): invalid pointer benutzer@localhost:~$ gdb -q (gdb) set width 0 (gdb) set pagination off (gdb) target remote localhost: Remote debugging using localhost: Reading /usr/lib/cups/filter/hpcups from remote target... warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead. Reading /usr/lib/cups/filter/hpcups from remote target... Reading symbols from target:/usr/lib/cups/filter/hpcups... Reading /usr/lib/cups/filter/25b6b40d5874920ba6c57ce85bb60b35661f71.debug from
Bug#972339: armhf: hpcups crashes with free() invalid pointer for some printers
Control: forwarded -1 https://bugs.launchpad.net/hplip/+bug/1901209 Le vendredi, 23 octobre 2020, 09.42:59 h CEST Didier 'OdyX' Raboud a écrit : > As this is testable at build-time, I'll add a test for this and upload this > to experimental. I'll report this to upstream today. Damn. It seems the bug doesn't trigger in buildd environments. I have also tried building hplip on the abel.debian.org porterbox, and the build-time test doesn't fail there. So it seems that there's a reproductible bug when run: - in qemu - in ci.debian.net's - in a sid chroot in abel.debian.org … but not: - in a buildd build; - in a manual build in abel.debian.org. I'm wondering what makes the build process immune to that error. The attached script will fail in a sid chroot on armhf, and I have reported this to the upstream bugtracker at https://bugs.launchpad.net/hplip/+bug/1901209 -- OdyX test_972339.sh Description: application/shellscript signature.asc Description: This is a digitally signed message part.
Bug#972339: armhf: hpcups crashes with free() invalid pointer for some printers
Control: found -1 3.20.5+dfsg0-3 Control: tags -1 +bullseye +upstream Le vendredi, 16 octobre 2020, 14.23:59 h CEST Didier 'OdyX' Raboud a écrit : > According to the 3.20.9-3 armhf auutopkgtest run for migration testing; > https://ci.debian.net/data/autopkgtest/testing/armhf/h/hplip/7460676/log.gz > > hpcups sometimes crashes with free(): invalid pointer. For instance, it > seems that setting up a 'drv:///hpcups.drv/hp-officejet_pro_1150c.ppd' > printer will let hpcups crash. > > I'd welcome assistance here as I'm no C gdb fluent person. So. This bug can be reproduced by the following suite of commands on armhf: $ export PPD=./prnt/hp-officejet_pro_1150c.ppd.gz $ /usr/lib/cups/filter/pdftopdf 1 debian '' 1 '' print_step_1.pdf $ /usr/lib/cups/filter/gstoraster 1 debian '' 1 '' print_step_2.raster $ /usr/lib/cups/filter/hpcups 1 debian '' 1 '' print_step_3.hpcups As I have confirmed that this is also _already_ a bug in the current bullseye version, I'll mark this RC bug as affecting the corresponding versions, and I'll upload a version without the autopkgtest to unstable, to let this version migrate. As this is testable at build-time, I'll add a test for this and upload this to experimental. I'll report this to upstream today. Cheers, OdyX signature.asc Description: This is a digitally signed message part.
Bug#972339: armhf: hpcups crashes with free() invalid pointer for some printers
Package: printer-driver-hpcups Version: 3.20.9+dfsg0-3 Severity: serious Tags: upstream help According to the 3.20.9-3 armhf auutopkgtest run for migration testing; https://ci.debian.net/data/autopkgtest/testing/armhf/h/hplip/7460676/log.gz hpcups sometimes crashes with free(): invalid pointer. For instance, it seems that setting up a 'drv:///hpcups.drv/hp-officejet_pro_1150c.ppd' printer will let hpcups crash. I'd welcome assistance here as I'm no C gdb fluent person. -- Package-specific info: -- System Information: Debian Release: bullseye/sid APT prefers buildd-unstable APT policy: (990, 'buildd-unstable'), (500, 'unstable-debug'), (500, 'unstable'), (500, 'testing'), (500, 'stable'), (500, 'oldstable'), (100, 'experimental'), (1, 'experimental-debug') Architecture: amd64 (x86_64) Foreign Architectures: i386 Kernel: Linux 5.8.0-3-amd64 (SMP w/4 CPU threads) Kernel taint flags: TAINT_FIRMWARE_WORKAROUND Locale: LANG=fr_CH.UTF-8, LC_CTYPE=fr_CH.UTF-8 (charmap=UTF-8), LANGUAGE=fr_CH:fr Shell: /bin/sh linked to /usr/bin/dash Init: systemd (via /run/systemd/system) LSM: AppArmor: enabled Versions of packages printer-driver-hpcups depends on: ii cups 2.3.3-3 ii cups-filters [ghostscript-cups] 1.28.5-1 ii libc62.31-4 ii libcups2 2.3.3-3 ii libdbus-1-3 1.12.20-1 ii libgcc-s110.2.0-15 ii libhpmud03.20.9+dfsg0-3 ii libjpeg62-turbo 1:2.0.5-1.1 ii libstdc++6 10.2.0-15 ii zlib1g 1:1.2.11.dfsg-2 printer-driver-hpcups recommends no packages. Versions of packages printer-driver-hpcups suggests: pn hplip pn hplip-doc -- no debconf information
