New issue
Advanced search Search tips

Issue 906339 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-12-07
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Undefined-shift in gs_malloc_init

Project Member Reported by ClusterFuzz, Nov 17

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=6268332376064000

Fuzzer: libFuzzer_chromeos_gstoraster_fuzzer
Job Type: libfuzzer_ubsan_chromeos
Platform Id: linux

Crash Type: Undefined-shift
Crash Address: 
Crash State:
  gs_malloc_init
  gsapi_new_instance
  gstoraster_fuzzer.c
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_ubsan_chromeos&range=3138112:3138335

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6268332376064000

Issue filed automatically.

See https://chromium.googlesource.com/chromiumos/docs/+/master/fuzzing.md#Reproducing-crashes-from-ClusterFuzz for more information.
 
Project Member

Comment 1 by ClusterFuzz, Nov 17

Cc: pawliczek@chromium.org briannorris@chromium.org luum@chromium.org skau@chromium.org
Labels: ClusterFuzz-Auto-CC
Automatically adding ccs based on OWNERS file / target commit history.

If this is incorrect, please add ClusterFuzz-Wrong label.
Cc: -manojgupta@google.com manojgupta@chromium.org
Components: Internals>Printing>CUPS
Ugh. Ghostscript has stuff like this:

/* Minimum and maximum values for the signed types. */
/* Avoid casts, to make them acceptable to strict ANSI compilers. */
#define min_short (-1 << (ARCH_SIZEOF_SHORT * 8 - 1))
#define max_short (~min_short)
#define min_int (-1 << (ARCH_SIZEOF_INT * 8 - 1))
#define max_int (~min_int)
#define min_long (-1L << (ARCH_SIZEOF_LONG * 8 - 1))
#define max_long (~min_long)

If only there were such thing as a C99 standard, whose <limits.h> would define all those things for us...Is 19 years too fresh?

Maybe I'm not a creative enough C nerd to figure out how to make that sort of expression that has the appropriate signedness, avoids integer overflows, and doesn't utilize undefined negative-number shifting behavior.

Is this something we should ignore? Patch locally (just replace with C99 limits.h defs)? We could probably report it upstream, but I'm pretty sure they value building on insane ancient setups
Oh, and this antipattern is found throughout the codebase [1]. Seems like something we probably won't do well to just patch over locally.

[1] A simple regex on upstream finds 24 results with only one false positive (in a comment), several of which are in reusable macros:

$ git grep -n -E -- '-1L?[ \t]*<<' | wc -l
24
If fixing Ghostscript is too hard,  Adding append-flags -fno-sanitize=shift to the configure step would disable the shift sanitizer errors. Not sure if such a thing should be done though.
Owner: skau@chromium.org
Status: Assigned (was: Untriaged)
FTR, the diff from here:

https://bugs.chromium.org/p/chromium/issues/detail?id=906438#c2

fixes this particular instance of undefined behavior. I didn't easily trip up any more UBSAN issues after fixing that one. I'll leave it up to Sean as to how to deal with this.
Cc: metzman@chromium.org
 Issue 907453  has been merged into this issue.
Project Member

Comment 8 by ClusterFuzz, Dec 1

Labels: -Reproducible Unreproducible
ClusterFuzz testcase 6268332376064000 appears to be flaky, updating reproducibility label.
Labels: -Unreproducible Reproducible
Please ignore the last comment about testcase being unreproducible. The testcase is still reproducible. This happened due to a code refactoring on ClusterFuzz side, and the underlying root cause is now fixed. Resetting the label back to Reproducible. Sorry about the inconvenience caused from these incorrect notifications.
Project Member

Comment 10 by ClusterFuzz, Dec 4

Labels: -Reproducible Unreproducible
ClusterFuzz testcase 6268332376064000 appears to be flaky, updating reproducibility label.
Labels: -Unreproducible Reproducible
Please ignore the last comment about testcase being unreproducible. 
The testcase is still reproducible. 
This was caused by a bug in ClusterFuzz that has been fixed. 
Sorry again for the inconvenience.
Project Member

Comment 12 by ClusterFuzz, Dec 5

Status: WontFix (was: Assigned)
ClusterFuzz testcase 6524123892940800 is flaky and no longer crashes, so closing issue.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
NextAction: 2018-12-07
Status: Started (was: WontFix)
The NextAction date has arrived: 2018-12-07
Fixing this only reveals another undefined shift.  Will be working on getting this one test case to pass before landing the patch.

Sign in to add a comment