bugmake - Bugs: bug #59870, Segmentation Fault on GNU

 
 

bug #59870: Segmentation Fault on GNU

Submitter:  Fabian Helfert <fabse333>
Submitted:  Thu 14 Jan 2021 09:45:21 AM UTC
   
 
Severity:  3 - Normal Item Group:  Bug
Status:  Fixed Privacy:  Public
Assigned to:  psmith Open/Closed:  Closed
Component Version:  4.4 Operating System:  POSIX-Based
Fixed Release:  4.4 Triage Status:  Small Effort
* Mandatory Fields

Add a New Comment Rich Markup
   

Mon 15 Mar 2021 05:18:53 AM UTC, comment #3: 

I pushed a fix.  Thanks Fabian and Dmitry!

Paul D. Smith <psmith>
Group administrator
Sun 14 Mar 2021 08:24:10 PM UTC, comment #2: 

I don't think this is the way we want to fix this.  It's incorrect to interpret the "define" keyword here as the introduction to a variable assignment; it should be a target name.

If we do ever decide to support adding define/endef directly as a target-specific variable then it would need to be well-formed; I'm not sure we'll ever do this anyway.

I installed this change:


diff --git a/src/read.c b/src/read.c
index 545514c5..3b4b0fe4 100644
--- a/src/read.c
+++ b/src/read.c
@@ -494,7 +494,7 @@ eval_buffer (char *buffer, const floc *flocp)
    based on the modifiers found if any, plus V_ASSIGN is 1.
  */
 static char *
-parse_var_assignment (const char *line, struct vmodifiers *vmod)
+parse_var_assignment (const char *line, int targvar, struct vmodifiers *vmod)
 {
   const char *p;
   memset (vmod, '\0', sizeof (*vmod));
@@ -529,14 +529,14 @@ parse_var_assignment (const char *line, struct vmodifiers *vmod)
         vmod->override_v = 1;
       else if (word1eq ("private"))
         vmod->private_v = 1;
-      else if (word1eq ("define"))
+      else if (!targvar && word1eq ("define"))
         {
           /* We can't have modifiers after 'define' */
           vmod->define_v = 1;
           p = next_token (p2);
           break;
         }
-      else if (word1eq ("undefine"))
+      else if (!targvar && word1eq ("undefine"))
         {
           /* We can't have modifiers after 'undefine' */
           vmod->undefine_v = 1;
@@ -721,7 +721,7 @@ eval (struct ebuffer *ebuf, int set_default)

       /* See if this is a variable assignment.  We need to do this early, to
          allow variables with names like 'ifdef', 'export', 'private', etc.  */
-      p = parse_var_assignment (p, &vmod);
+      p = parse_var_assignment (p, 0, &vmod);
       if (vmod.assign_v)
         {
           struct variable *v;
@@ -1181,7 +1181,7 @@ eval (struct ebuffer *ebuf, int set_default)
             p2 = variable_buffer + l;
           }

-        p2 = parse_var_assignment (p2, &vmod);
+        p2 = parse_var_assignment (p2, 1, &vmod);
         if (vmod.assign_v)
           {
             /* If there was a semicolon found, add it back, plus anything


and some tests.

Paul D. Smith <psmith>
Group administrator
Fri 15 Jan 2021 02:20:06 AM UTC, comment #1: 

Here is a patch.
i am not adding a test, because there is a commented out test 19 in targetvars, which expects different behavior.
Thank you for your report and the test case.


diff --git a/src/read.c b/src/read.c
index 545514c..11ef748 100644
--- a/src/read.c
+++ b/src/read.c
@@ -1802,10 +1802,11 @@ record_target_var (struct nameseq *filenames, char *defn,
           /* Get a reference for this pattern-specific variable struct.  */
           p = create_pattern_var (name, percent);
           p->variable.fileinfo = *flocp;
-          /* I don't think this can fail since we already determined it was a
-             variable definition.  */
+          /* Could be a variable definition or %:define or %:undefine.
+             sv 59870.  */
           v = assign_variable_definition (&p->variable, defn);
-          assert (v != 0);
+          if (!v)
+            O (fatal, flocp, _("Malformed pattern-specific variable definition"));
 
           v->origin = origin;
           if (v->flavor == f_simple)

Dmitry Goncharov <dgoncharov>
Thu 14 Jan 2021 09:45:21 AM UTC, original submission:  

The attached makefile will cause a Segmentation fault on GNU make 4.2 on Debian and also when building the newest newest version from the GitHub mirror.

The issue is that in the method "record_target_var" in read.c, the function "assign_variable_definition" (see https://github.com/mirror/make/blob/4.2/read.c#L1864  ) returns 0 on this MAKEFILE. Even when the Comment states "I don't think this can fail,...". ;-)
While this return value is checked with an assert statement, the asserts are not included on release builds (e.g. the one I have installed on my Debian 10 Buster). Assert can also be disabled by setting the DNDEBUG Cflag on the ./configure command for local testing:
./configure CFLAGS="-g -DNDEBUG"

The segmentation fault is then an exception when this instruction is executed where RDX is set to 0:
movzx  eax, byte ptr [rdx + 0x2f]
This should correspond to this line where the origin field of v is accessed:
https://github.com/mirror/make/blob/4.2/read.c#L1867

Steps to reproduce:

  • make -f MAKEFILE


I don't see any security concerns related with this bug, so I set Privacy of this ticket to Public.

Fabian Helfert <fabse333>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #50714:  MAKEFILE added by fabse333 (8B - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by psmith (Posted a comment)
  • -email is unavailable- added by dgoncharov (Posted a comment)
  • -email is unavailable- added by fabse333 (Submitted the item)
  • -email is unavailable- added by fabse333
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-03-15 psmith StatusNone Fixed
        Assigned toNone psmith
        Open/ClosedOpen Closed
        Fixed ReleaseNone 4.4
        Triage StatusNone Small Effort
    2021-01-14 fabse333 Attached File- Added MAKEFILE, #50714
        Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code