Tue 15 Mar 2016 02:49:29 AM UTC, comment #31:
I've responsibly reverted to the old implementation until I fix the overflow on line 969.
My gut feeling is that you'll find lines 1239,1247,1255,1256 next. Let's see.
|
Mon 14 Mar 2016 06:50:54 PM UTC, comment #30:
The 64-bit version does not reveal any bugs, but it does not
go much further than before.
It's hard for me to judge how more coverage is potentially possible here. Maybe you could improve the target function some more,
of maybe you could supply interesting samples.
attached is another coverage dump (from clang.llvm.org/docs/SanitizerCoverage.html)
(file #36636)
|
Sun 13 Mar 2016 11:35:46 AM UTC, comment #29:
I can confirm this (it took me a while to compile FreeType in 32bit mode – my openSuSE box doesn't come with proper 32bit .pc files for pkgconfig, and it's a pain to compile HarfBuzz without them, so I disabled it).
All three problems occur in the new implementation of `gray_render_line', AFAICS.
Alexei?
|
Sun 13 Mar 2016 12:22:46 AM UTC, comment #28:
That's only on 32-bit, btw.
64-bit running fine.
|
Sun 13 Mar 2016 12:11:44 AM UTC, comment #27:
With the recent changes if first prints a bunch of int oevrflows,
like this:
src/smooth/ftgrays.c:969:35: runtime error: signed integer overflow: -52253144 * 172 cannot be represented in type 'long'
src/smooth/ftgrays.c:1003:46: runtime error: signed integer overflow: -52253144 * 256 cannot be represented in type 'long'
Then it hangs here:
#5 0x8428ed3 in gray_set_cell src/smooth/ftgrays.c:626:19
#6 0x8428ed3 in gray_render_line src/smooth/ftgrays.c:1028
#7 0x8426acd in gray_line_to src/smooth/ftgrays.c:1306:5
#8 0x81d6cd3 in FT_Outline_Decompose src/base/ftoutln.c:165:21
#9 0x842580d in gray_convert_glyph_inner src/smooth/ftgrays.c:1903:15
#10 0x842154c in gray_convert_glyph src/smooth/ftgrays.c:2006:17
#11 0x842154c in gray_raster_render src/smooth/ftgrays.c:2141
#12 0x81dc124 in FT_Outline_Render src/base/ftoutln.c:646:15
#13 0x81dc9c0 in FT_Outline_Get_Bitmap src/base/ftoutln.c:687:12
#14 0x816d218 in LLVMFuzzerTestOneInput
Example input (in base64):
UyFU/yJMIQC2JyoAJiEn/ytU/ABNIScAIbUnKiIl
|
Sat 12 Mar 2016 10:49:45 PM UTC, comment #26:
Thanks for the explanations.
I've slightly modified `rasterfuzzer.cc' to get more coverage, please test.
|
Sat 12 Mar 2016 07:30:35 AM UTC, comment #25:
(that's experimental coverage report, we are still trying to figure out the UI part).
Green indicates the beginning of a covered basic block.
Red -- non covered.
Yellow -- an edge (comparison) that is always either true or false.
So, the most interesting here are red lines in the functions that
have green lines -- we don't execute those lines.
|
Sat 12 Mar 2016 07:23:22 AM UTC, comment #24:
Thanks. However, I have difficulties to understand the explanations.
What do the colors green, red, and yellow indicate? This isn't documented at all in the clang link...
|
Fri 11 Mar 2016 12:09:09 AM UTC, comment #23:
The fuzzer stops producing interesting coverage almost instantly,
both on 64- and 32-bit build.
I've attached a coverage report (see http://clang.llvm.org/docs/SanitizerCoverage.html#sancov-tool).
Maybe you could tell what's missing.
(file #36600)
|
Mon 07 Mar 2016 03:44:10 PM UTC, comment #22:
There are plenty of dot and cross products to overflow even with the limited outline dimensions on 32-bit platforms. Please find those.
A reasonable input size is about 200 bytes. Going larger is the same as repeating.
|
Mon 07 Mar 2016 07:37:49 AM UTC, comment #21:
Well, rasterfuzzer.cc needs to be expanded; maybe Alexei can work on this – it is intentionally very simple, without providing big code coverage. It was just meant as a starter to check the most serious overflows in both the B/W and gray rasterizer.
The expected input size is basically irrelevant; the input data simply gets reinterpreted as a points array.
For the moment, it is probably sufficient to run the fuzzer manually for a few minutes :-)
|
Mon 07 Mar 2016 07:20:22 AM UTC, comment #20:
From the first quick run the target looks boring -- the coverage stops growing almost instantly.
- How much different logic is expected there?
- Do you have any sample inputs?
- what is the expected sizes of the inputs?
(Will make more experiments in ~ 1 week).
|
Mon 07 Mar 2016 06:46:11 AM UTC, comment #19:
Looks good, thanks! With some quick tests, I wasn't able to trigger the sanitizer running `rasterfuzzer.cc'.
Kostya, Ben, it seems that you could now start using this file :-)
I'm now closing this issue.
|
Mon 07 Mar 2016 05:03:34 AM UTC, comment #18:
Let's see how things improve after this commit.
|
Wed 02 Mar 2016 08:17:49 AM UTC, comment #17:
Sounds reasonable. Will you have a look?
|
Tue 01 Mar 2016 02:51:32 PM UTC, comment #16:
We have a realistic limit onthe bitmaps that we produce, which is 32767x32767 = 1Gb. So I suggest that we outright reject outlines that exceed ±2097088 (in 26.6 fixed point) in FT_Ouline_Get_Bitmap. We are talking about 16 inch letters on 2000 dpi printers. I think this is reasonable and should cover the bulk of the overflows. Perhaps there are other function that need this limit.
Then we can look at the remaining overflows more carefully.
|
Tue 01 Mar 2016 08:44:46 AM UTC, comment #15:
Thanks for the input file. I've made it more C++ (please check!) and added it to the ftfuzzer directory in FreeType; it tests both the gray and mono rasterizers. Later on it should be refined, e.g., by constructing more than a single outline, having a larger target buffer, etc.
I can easily reproduce the overflow and shift errors by using an arbitrary binary directory as input (attached is a sample file); however, I don't have time currently to work on this issue.
Alexei, can you afford some time for this issue?
I'm reopening the issue.
(file #36517)
|
Tue 01 Mar 2016 01:51:53 AM UTC, comment #14:
I've made a quick-n-dirty fuzz target for FT_Outline_Get_Bitmap
(attached) and indeed it hangs almost instantly.
#5 0x70e083 in gray_set_cell src/smooth/ftgrays.c:634:10
#6 0x70e083 in gray_render_line src/smooth/ftgrays.c:1028
#7 0x70c7a3 in gray_line_to src/smooth/ftgrays.c:1306:5
#8 0x539c06 in FT_Outline_Decompose src/base/ftoutln.c:165:21
#9 0x70baf7 in gray_convert_glyph_inner src/smooth/ftgrays.c:1902:15
#10 0x708fc3 in gray_convert_glyph src/smooth/ftgrays.c:2005:17
#11 0x708fc3 in gray_raster_render src/smooth/ftgrays.c:2140
#12 0x53d49b in FT_Outline_Render src/base/ftoutln.c:640:15
#13 0x53d49b in FT_Outline_Get_Bitmap src/base/ftoutln.c:681
#14 0x4e4b7d in LLVMFuzzerTestOneInput
ubsan prints lots of stuff:
src/smooth/ftgrays.c:1291:9: runtime error: left shift of negative value -8867864493558781852
src/smooth/ftgrays.c:1292:9: runtime error: left shift of negative value -693208218122896854
src/smooth/ftgrays.c:1306:32: runtime error: left shift of 7216742023680857627 by 2 places cannot be represented in type 'FT_Pos' (aka 'long')
src/smooth/ftgrays.c:1306:50: runtime error: left shift of negative value -7482078255484648629
src/smooth/ftgrays.c:931:15: runtime error: signed integer overflow: -8026520052695672724 - 1422030173183975824 cannot be represented in type 'long'
src/smooth/ftgrays.c:932:15: runtime error: signed integer overflow: 6965175125480508716 - -2772832872491587416 cannot be represented in type 'long'
src/smooth/ftgrays.c:935:19: runtime error: left shift of negative value -10831378408170264
src/smooth/ftgrays.c:969:24: runtime error: signed integer overflow: 8998193847829903068 * 168 cannot be represented in type 'long'
src/smooth/ftgrays.c:969:35: runtime error: signed integer overflow: -8708736075737455484 * 144 cannot be represented in type 'long'
src/smooth/ftgrays.c:980:29: runtime error: signed integer overflow: 8998193847829903068 * 256 cannot be represented in type 'long'
src/smooth/ftgrays.c:984:22: runtime error: signed integer overflow: -8708736075737455484 * 256 cannot be represented in type 'long'
src/smooth/ftgrays.c:991:29: runtime error: signed integer overflow: 8998193847829903068 * 256 cannot be represented in type 'long'
src/smooth/ftgrays.c:992:29: runtime error: signed integer overflow: 8998193847829903068 * 256 cannot be represented in type 'long'
src/smooth/ftgrays.c:992:46: runtime error: signed integer overflow: -8708736075737455484 * 256 cannot be represented in type 'long'
src/smooth/ftgrays.c:994:22: runtime error: signed integer overflow: 8998193847829903068 * 256 cannot be represented in type 'long'
Some of these may be causing the hangs.
(file #36516)
|
Mon 29 Feb 2016 11:19:54 PM UTC, comment #13:
And yes, I agree this is very good idea!
|
Mon 29 Feb 2016 11:01:04 PM UTC, comment #12:
You can go with a command line switch or with a separate file.
I usually prefer separate files -- this makes matters simpler.
Once you have the target file in git I'll be able to hook it to
the bot (allow me 2-3 days). But please test the setting locally,
the libFuzzer workflow is pretty simple (llvm.org/docs/LibFuzzer.html).
|
Sat 27 Feb 2016 09:38:01 AM UTC, comment #11:
A very good idea IMHO! I wonder how this could be implemented: More code to `ftfuzzer.cc', to be controlled with command line switches? A separate `ftrenderfuzzer.cc' file?
Ben, what do you suggest?
|
Thu 25 Feb 2016 01:58:04 PM UTC, comment #10:
I wonder if this code can be used in the fuzzer, by-passing all the font parsing. This way we could potentially stress test both smooth and b/w rasterizer separately. Werner, please comment.
|
Wed 24 Feb 2016 12:19:13 AM UTC, comment #9:
Attached is some C source that reproduces the infinite loop by calling FT_Outline_Get_Bitmap rather than an internal API.
The supplied points contain some large values. These large values were the result of applying a transformation matrix to a glyph outline where the matrix had large values that were the result of the overflow in Skia.
(file #36447)
|
Tue 23 Feb 2016 10:25:35 PM UTC, comment #8:
Skia bug report: https://bugs.chromium.org/p/skia/issues/detail?id=4998
|
Tue 23 Feb 2016 03:36:38 AM UTC, comment #7:
Thanks for that. I'm still investigating, but at this stage it looks like another integer overflow. This time in Skia.
|
Wed 17 Feb 2016 03:39:40 AM UTC, comment #6:
See this commit. I am still curious to see what it takes to generate 56,000-point lines. Please provide the test case eventually.
|
Fri 12 Feb 2016 02:40:58 PM UTC, comment #5:
Never mind. You have 64-bit long and 32-bit int, much like I do.
|
Fri 12 Feb 2016 02:24:57 PM UTC, comment #4:
It is very possible that there are more than one bug in this report. I will make TArea unconditionally long, but still I am curious so please bear with me. To get int there, if PIXEL_BITS is 8, you must have UINT_MAX different from 0xFFFFU, i.e. you int is not 32 bits. Yet you say that it overflows and your long is 64 bits. What kind of environment is that?
What are your UINT_MAX, ULONG_MAX, sizeof(int), sizeof(long)?
|
Fri 12 Feb 2016 12:02:46 AM UTC, comment #3:
Sorry about the anonymous post. I didn't think to create an account until after I'd posted.
I haven't changed PIXEL_BITS from it's default. I'm not sure why there's such a long line segment. Possibly a bug in Skia, or my code. Or it's also possible it's just the input document supplied by the user that specified a large value. But I don't have access to that document.
From one of the Skia stack frames, it looks like it's trying to produce a bitmap for a glyph that is 1x1. At least the bitmap passed to FT_Outline_Get_Bitmap is 1x1. My guess would be that there's another integer overflow happening somewhere. Although quite how we overflow with such a small bitmap, I don't know. Perhaps if there's code that's doing fixed-point division by a value very close to 0, that could cause an overflow. Just guesses.
In case it's of interest, here's a backtrace of how it got to gray_render_line:
gray_render_line
gray_line_to
FT_Outline_Decompose
gray_convert_glyph_inner
gray_convert_glyph
gray_raster_render
FT_Outline_Render.part.11
FT_Outline_Get_Bitmap
SkScalerContext_FreeType_Base::generateGlyphImage()
SkScalerContext_FreeType::generateImage()
SkScalerContext::getImage()
SkGlyphCache::findImage()
...
SkDraw::drawPosText()
SkCanvas::onDrawPosTextH()
|
Thu 11 Feb 2016 08:48:36 PM UTC, comment #2:
The easy and correct solution is to just use long for TArea as suggested, because TPos and TCoord are not shy about it. I wonder why int was even considered for TArea.
Regrdless, that is quite a line segment of some 53 thousand pixels. Wow! Or somebody changed PIXEL_BITS. What was the motivation? Hopefully, the anonymous reporter will check back.
|
Thu 11 Feb 2016 06:20:59 AM UTC, comment #1:
Alexei, please have a look!
|
Thu 11 Feb 2016 04:01:26 AM UTC, original submission:
The function gray_render_line can go into an infinite loop. I've observed this in a production system that is using FreeType. I've been somewhat restricted in the information I've been able to get from the core files where this has happened since gcc optimizes out most of the variables, so they don't show up in gdb. So I don't know what glyph it's trying to draw nor at what scale. I was however able to get some of the arguments that were passed to gray_render_line and was able to reproduce the problem by passing these same values.
If ras.x = 0, ras.y = 0 and I pass to_x = 14418112 and to_y = 14418032, I get the infinite loop.
The code requires ex1==ex2 and ey1==ey2 concurrently for the loop to terminate. The following is a printout of these values for some relevant iterations of the loop.
prod=-1757343744 ex1=56319 ey1=56318 ex2=56320 ey2=56320
prod=1933672448 ex1=56320 ey1=56318 ex2=56320 ey2=56320
prod=-1757364224 1=56320 ey1=56319 ex2=56320 ey2=56320
prod=1933651968 ex1=56321 ey1=56319 ex2=56320 ey2=56320
prod=-1757384704 ex1=56321 ey1=56320 ex2=56320 ey2=56320
prod=1933631488 ex1=56322 ey1=56320 ex2=56320 ey2=56320
prod=-1757405184 ex1=56322 ey1=56321 ex2=56320 ey2=56320
As you can see, ex1 reached ex2 and then passed it just before ey1 reached ey2.
This appears to be due to an integer overflow in the variable prod which is only 32 bits whereas most of the other variables are long. If I change prod to a long, the function terminates normally.
Hopefully that's enough to go off. Sorry I don't have a repro using a public API.
|