[bug #59870] Segmentation Fault on GNU

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[bug #59870] Segmentation Fault on GNU

Paul D. Smith
URL:
  <https://savannah.gnu.org/bugs/?59870>

                 Summary: Segmentation Fault on GNU
                 Project: make
            Submitted by: fabse333
            Submitted on: Thu 14 Jan 2021 09:45:21 AM UTC
                Severity: 3 - Normal
              Item Group: Bug
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
       Component Version: SCM
        Operating System: POSIX-Based
           Fixed Release: None
           Triage Status: None

    _______________________________________________________

Details:

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.



    _______________________________________________________

File Attachments:


-------------------------------------------------------
Date: Thu 14 Jan 2021 09:45:21 AM UTC  Name: MAKEFILE  Size: 8B   By: fabse333

<http://savannah.gnu.org/bugs/download.php?file_id=50714>

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?59870>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/


Reply | Threaded
Open this post in threaded view
|

[bug #59870] Segmentation Fault on GNU

Paul D. Smith
Follow-up Comment #1, bug #59870 (project make):

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)


    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?59870>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/


Reply | Threaded
Open this post in threaded view
|

[bug #59870] Segmentation Fault on GNU

Paul D. Smith
Follow-up Comment #2, bug #59870 (project make):

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.

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?59870>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/


Reply | Threaded
Open this post in threaded view
|

[bug #59870] Segmentation Fault on GNU

Paul D. Smith
Update of bug #59870 (project make):

                  Status:                    None => Fixed                  
             Assigned to:                    None => psmith                
             Open/Closed:                    Open => Closed                
           Fixed Release:                    None => SCM                    
           Triage Status:                    None => Small Effort          

    _______________________________________________________

Follow-up Comment #3:

I pushed a fix.  Thanks Fabian and Dmitry!

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?59870>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/