Re: mg(1) segfault

2015-04-10 Thread Gleydson Soares 
 I hate the startup file.
 Look, this is a use after free, but I can't find it...
 
 #0  0x1b9de0b1b77f in definemacro (f=0, n=1)
 at /usr/src/usr.bin/mg/macro.c:43
 43  lp2 = lp1-l_fp;
 (gdb) p *maclhead
 $1 = {l_fp = 0xdfdfdfdfdfdfdfdf, l_bp = 0xdfdfdfdfdfdfdfdf, 
   l_size = -538976289, l_used = -538976289, 
   l_text = 0xdfdfdfdfdfdfdfdf Address 0xdfdfdfdfdfdfdfdf out of bounds}

seems that it is in excline(), look:

src/usr.bin/mg/extend.c:907
lp = maclcur-l_fp;
while (lp != maclcur) {
np = lp-l_fp;
free(lp);
lp = np;
}
free(lp);
return (status);

excline() loads .mg file and free(lp) lines afterwards. 

following diff add a cleanline check to make sure that the cleanup was already 
done or not.
avoid user after free in definemacro()/macro.c:45 in cases where excline() take 
care of the free lines cleanup. 

? mg
Index: extend.c
===
RCS file: /cvs/src/usr.bin/mg/extend.c,v
retrieving revision 1.61
diff -u -p -u -p -r1.61 extend.c
--- extend.c24 Mar 2015 22:28:10 -  1.61
+++ extend.c11 Apr 2015 04:41:38 -
@@ -910,6 +910,7 @@ cleanup:
free(lp);
lp = np;
}
+   cleanline = 1;
free(lp);
return (status);
 }
Index: macro.c
===
RCS file: /cvs/src/usr.bin/mg/macro.c,v
retrieving revision 1.16
diff -u -p -u -p -r1.16 macro.c
--- macro.c 19 Mar 2015 21:22:15 -  1.16
+++ macro.c 11 Apr 2015 04:41:38 -
@@ -15,6 +15,7 @@
 #include key.h
 #include macro.h
 
+int cleanline = 0;
 int inmacro = FALSE;   /* Macro playback in progess */
 int macrodef = FALSE;  /* Macro recording in progress */
 int macrocount = 0;
@@ -38,7 +39,7 @@ definemacro(int f, int n)
}
 
/* free lines allocated for string arguments */
-   if (maclhead != NULL) {
+   if (!cleanline  maclhead != NULL) {
for (lp1 = maclhead-l_fp; lp1 != maclhead; lp1 = lp2) {
lp2 = lp1-l_fp;
free(lp1);
Index: macro.h
===
RCS file: /cvs/src/usr.bin/mg/macro.h,v
retrieving revision 1.7
diff -u -p -u -p -r1.7 macro.h
--- macro.h 18 Nov 2005 20:56:53 -  1.7
+++ macro.h 11 Apr 2015 04:41:38 -
@@ -6,6 +6,7 @@
 
 #define MAXMACRO 256   /* maximum functs in a macro */
 
+extern int cleanline;
 extern int inmacro;
 extern int macrodef;
 extern int macrocount;

Re: mg(1) segfault

2015-04-04 Thread Florian Obser 
On Sat, Apr 04, 2015 at 10:48:15AM -0300, Gleydson Soares wrote:
 
 mg(1) segfault.
 it is triggered as follows:
 
 1- echo (start-kbd-macro)  $HOME/.mg
 2- open mg and type twice C-x (
 
 find below the backtrace and a patch to fix.
 OK?
 
 Program received signal SIGBUS, Bus error.
 definemacro (f=Variable f is not available.
 ) at macro.c:43
 43  lp2 = lp1-l_fp;
 (gdb) backtrace
 #0  definemacro (f=Variable f is not available.
 ) at macro.c:43
 #1  0x038cecf15606 in doin () at kbd.c:158
 #2  0x038cecf16d4b in main (argc=Variable argc is not available.
 ) at main.c:188
 (gdb)
 

 ? mg
 ? mg_segfault.diff
 Index: macro.c
 ===
 RCS file: /cvs/src/usr.bin/mg/macro.c,v
 retrieving revision 1.16
 diff -u -p -r1.16 macro.c
 --- macro.c   19 Mar 2015 21:22:15 -  1.16
 +++ macro.c   4 Apr 2015 13:45:15 -
 @@ -38,7 +38,7 @@ definemacro(int f, int n)
   }
  
   /* free lines allocated for string arguments */
 - if (maclhead != NULL) {
 + if (macrodef  maclhead != NULL) {
   for (lp1 = maclhead-l_fp; lp1 != maclhead; lp1 = lp2) {
   lp2 = lp1-l_fp;
   free(lp1);

No, this is obviously not correct. macrodef can't be true here, ever.
Some lines above this we have:
if (macrodef) {
ewprintf(already defining macro);
return (macrodef = FALSE);
}

-- 
I'm not entirely sure you are real.

mg(1) segfault

2015-04-04 Thread Gleydson Soares 

mg(1) segfault.
it is triggered as follows:

1- echo (start-kbd-macro)  $HOME/.mg
2- open mg and type twice C-x (

find below the backtrace and a patch to fix.
OK?

Program received signal SIGBUS, Bus error.
definemacro (f=Variable f is not available.
) at macro.c:43
43  lp2 = lp1-l_fp;
(gdb) backtrace
#0  definemacro (f=Variable f is not available.
) at macro.c:43
#1  0x038cecf15606 in doin () at kbd.c:158
#2  0x038cecf16d4b in main (argc=Variable argc is not available.
) at main.c:188
(gdb)

? mg
? mg_segfault.diff
Index: macro.c
===
RCS file: /cvs/src/usr.bin/mg/macro.c,v
retrieving revision 1.16
diff -u -p -r1.16 macro.c
--- macro.c 19 Mar 2015 21:22:15 -  1.16
+++ macro.c 4 Apr 2015 13:45:15 -
@@ -38,7 +38,7 @@ definemacro(int f, int n)
}
 
/* free lines allocated for string arguments */
-   if (maclhead != NULL) {
+   if (macrodef  maclhead != NULL) {
for (lp1 = maclhead-l_fp; lp1 != maclhead; lp1 = lp2) {
lp2 = lp1-l_fp;
free(lp1);

Re: mg(1) segfault

2015-04-04 Thread Gleydson Soares 
 return (macrodef = FALSE);

but we shouldn't change macrodef here.

? mg
? mg_segfault.diff
? v2_mg_segfault.diff
Index: macro.c
===
RCS file: /cvs/src/usr.bin/mg/macro.c,v
retrieving revision 1.16
diff -u -p -u -p -r1.16 macro.c
--- macro.c 19 Mar 2015 21:22:15 -  1.16
+++ macro.c 4 Apr 2015 16:09:38 -
@@ -34,11 +34,11 @@ definemacro(int f, int n)
 
if (macrodef) {
ewprintf(already defining macro);
-   return (macrodef = FALSE);
+   return (FALSE);
}
 
/* free lines allocated for string arguments */
-   if (maclhead != NULL) {
+   if (macrodef  maclhead != NULL) {
for (lp1 = maclhead-l_fp; lp1 != maclhead; lp1 = lp2) {
lp2 = lp1-l_fp;
free(lp1);

Re: mg(1) segfault

2015-04-04 Thread Steven McDonald 
On Sat, 4 Apr 2015 15:23:45 -0300
Gleydson Soares gsoa...@gmail.com wrote:

 but we shouldn't change macrodef here.

Regardless of whether macrodef is being changed, you're still returning
if macrodef is true, so there is no way for it to be true at the point
where you're adding a test for it.

Re: mg(1) segfault

2015-04-04 Thread Florian Obser 
On Sat, Apr 04, 2015 at 03:23:45PM -0300, Gleydson Soares wrote:
  return (macrodef = FALSE);
 
 but we shouldn't change macrodef here.
 

I hate the startup file.
Look, this is a use after free, but I can't find it...

#0  0x1b9de0b1b77f in definemacro (f=0, n=1)
at /usr/src/usr.bin/mg/macro.c:43
43  lp2 = lp1-l_fp;
(gdb) p *maclhead
$1 = {l_fp = 0xdfdfdfdfdfdfdfdf, l_bp = 0xdfdfdfdfdfdfdfdf, 
  l_size = -538976289, l_used = -538976289, 
  l_text = 0xdfdfdfdfdfdfdfdf Address 0xdfdfdfdfdfdfdfdf out of bounds}

also: what Steven McDonald says

 ? mg
 ? mg_segfault.diff
 ? v2_mg_segfault.diff
 Index: macro.c
 ===
 RCS file: /cvs/src/usr.bin/mg/macro.c,v
 retrieving revision 1.16
 diff -u -p -u -p -r1.16 macro.c
 --- macro.c   19 Mar 2015 21:22:15 -  1.16
 +++ macro.c   4 Apr 2015 16:09:38 -
 @@ -34,11 +34,11 @@ definemacro(int f, int n)
  
   if (macrodef) {
   ewprintf(already defining macro);
 - return (macrodef = FALSE);
 + return (FALSE);
   }
  
   /* free lines allocated for string arguments */
 - if (maclhead != NULL) {
 + if (macrodef  maclhead != NULL) {
   for (lp1 = maclhead-l_fp; lp1 != maclhead; lp1 = lp2) {
   lp2 = lp1-l_fp;
   free(lp1);


-- 
I'm not entirely sure you are real.