Bug 80890

Summary: negative subscript in os/log.c on make check (signal-logging)
Product: xorg Reporter: Vittorio <zeccav>
Component: Server/GeneralAssignee: Xorg Project Team <xorg-team>
Status: RESOLVED MOVED QA Contact: Xorg Project Team <xorg-team>
Severity: normal    
Priority: medium CC: peter.hutterer
Version: 7.7 (2012.06)   
Hardware: x86-64 (AMD64)   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Attachments:
Description Flags
0001-os-prevent-negative-array-index-access.patch none

Description Vittorio 2014-07-04 05:59:40 UTC
In os/log.c:680
" newline = (buf[len - 1] == '\n');"
running make check (signal-logging)
len==0 thus accessing array buf with a negative subscript.
Comment 1 Alan Coopersmith 2014-07-05 16:59:51 UTC
The full check is:

    if (sizeof(buf) - len == 1)
        buf[len - 1] = '\n';

Referring to buf, declared as:
    char buf[1024];

Thus if 1024 - len == 1, len must be 1023, not 0, and whatever checker
is claiming 1023 - 1 can be negative needs to check it's math.
Comment 2 Vittorio 2014-07-05 17:43:40 UTC
Actually I did point to the next instruction
"newline = (buf[len - 1] == '\n');"
and this one ran with len==0, I double checked putting
"if(len <= 0) FatalError("log.c:680 len subscript check\n");"
immediately before.
Will you please double check as well?
Comment 3 Alan Coopersmith 2014-07-05 18:10:21 UTC
Sorry, you're on a different version of the code, so the line numbers don't
match up with our current git master that I'm looking at - I see the code
you are referring to on line 700 now.

There is a small chance that len could be 0 there, if something called
LogVMessageVerbSigSafe() with an invalid or empty format string, which
should only happen in case of a bug in a caller.
Comment 4 Vittorio 2014-07-06 12:25:01 UTC
Consider that this comes from "make check" so the caller shouldn't be wrong.
If it is wrong, this is a xorg 1.14.4 distribution bug.
Comment 5 Alan Coopersmith 2014-07-06 14:53:44 UTC
It does not come from "make check" with any compiler or options used by the
X.Org developers.  What have you used to cause "make check" to issue that?
Comment 6 Peter Hutterer 2014-07-06 22:32:16 UTC
Created attachment 102334 [details] [review]
0001-os-prevent-negative-array-index-access.patch

small corner case, though I don't think we're triggering this anywhere: and empty string causes len to be 0.
Comment 7 Vittorio 2014-07-07 07:39:24 UTC
I just applied the patch.
Now signal-logging PASSes, displaying lots of (EE) messages but in the end
(EE) Server terminated successfully (0). Closing log file.
PASS: signal-logging
Comment 8 Peter Hutterer 2014-07-07 23:24:44 UTC
http://patchwork.freedesktop.org/patch/29254/
Comment 9 GitLab Migration User 2018-12-13 22:30:28 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/xorg/xserver/issues/460.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.