Re: 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)
--- hplip-3.20.11+dfsg0.orig/prnt/hpcups/Halftoner.cpp
+++ hplip-3.20.11+dfsg0/prnt/hpcups/Halftoner.cpp
@@ -181,12 +181,12 @@ Halftoner::Halftoner
{
PlaneSize= OutputWidth[i]/8 + // doublecheck ... should already be divisble by 8
((OutputWidth[i] % 8)!=0);
- ColorPlane[i][j][k] = (BYTE*) new BYTE[(PlaneSize)];
+ ColorPlane[i][j][k] = (BYTE*) new BYTE[(PlaneSize) + 32];
if (ColorPlane[i][j] == NULL)
{
goto MemoryError;
}
- memset(ColorPlane[i][j][k], 0, PlaneSize);
+ memset(ColorPlane[i][j][k], 0, PlaneSize + 32);
}
}
}
Description: Workaround: Add 32 bytes to allocation Compressor
Bug-Debian: https://bugs.debian.org/972339
Bug-Ubuntu: https://launchpad.net/bugs/1901209
Last-Update: 2021-02-27
==183233== Invalid read of size 1
==183233== at 0x11465A: Mode9::Process(RASTERDATA*) (Mode9.cpp:215)
==183233== by 0x11EDA4: Process (Pipeline.cpp:72)
==183233== by 0x11EDA4: Pipeline::Execute(RASTERDATA*) (Pipeline.cpp:79)
==183233== by 0x11EDDF: Pipeline::Execute(RASTERDATA*) (Pipeline.cpp:83)
==183233== by 0x11EDDF: Pipeline::Execute(RASTERDATA*) (Pipeline.cpp:83)
==183233== by 0x112677: HPCupsFilter::processRasterData(_cups_raster_s*) (HPCupsFilter.cpp:779)
==183233== by 0x112DE8: HPCupsFilter::StartPrintJob(int, char**) (HPCupsFilter.cpp:597)
==183233== by 0x4C17D09: (below main) (libc-start.c:308)
==183233== Address 0x5a8f0a1 is 0 bytes after a block of size 3,025 alloc'd
==183233== at 0x483950F: operator new[](unsigned long) (vg_replace_malloc.c:431)
==183233== by 0x113AE6: Compressor::Compressor(unsigned int, bool) (Compressor.cpp:44)
==183233== by 0x114AEB: Mode9::Mode9(unsigned int, bool) (Mode9.cpp:34)
==183233== by 0x1181C1: Pcl3::Configure(Pipeline**) (Pcl3.cpp:95)
==183233== by 0x11EA30: Job::Configure() (Job.cpp:248)
==183233== by 0x11EB67: Job::Init(SystemServices*, JobAttributes_s*, Encapsulator*) (Job.cpp:63)
==183233== by 0x111A35: HPCupsFilter::startPage(cups_page_header2_s*) (HPCupsFilter.cpp:481)
==183233== by 0x112792: HPCupsFilter::processRasterData(_cups_raster_s*) (HPCupsFilter.cpp:668)
==183233== by 0x112DE8: HPCupsFilter::StartPrintJob(int, char**) (HPCupsFilter.cpp:597)
==183233== by 0x4C17D09: (below main) (libc-start.c:308)
--- hplip-3.20.11+dfsg0.orig/prnt/hpcups/Compressor.cpp
+++ hplip-3.20.11+dfsg0/prnt/hpcups/Compressor.cpp
@@ -41,8 +41,9 @@ Compressor::Compressor (unsigned int Ras
if (!UseSeedRow)
return;
- SeedRow = (BYTE *) new BYTE[RasterSize];
+ SeedRow = (BYTE *) new BYTE[RasterSize + 32];
CNEWCHECK(SeedRow);
+ memset(SeedRow, 0, RasterSize + 32);
}
Compressor::~Compressor()
Reply to: