Re: Fix for segfault in find(1)

2015-07-14 Thread Todd C. Miller 
On Tue, 14 Jul 2015 17:52:52 +0200, Gregor Best wrote:

Comments inline:

 Index: misc.c
 ===
 RCS file: /mnt/media/cvs/src/usr.bin/find/misc.c,v
 retrieving revision 1.12
 diff -u -p -u -r1.12 misc.c
 --- misc.c18 May 2014 08:10:00 -  1.12
 +++ misc.c14 Jul 2015 15:41:20 -
 @@ -66,6 +66,7 @@ brace_subst(char *orig, char **store, ch
   if (!(newstore = realloc(*store, newlen)))
   err(1, NULL);
   *store = newstore;
 + p = newstore;

Shouldn't this be:

p = (p - *store) + newstore;

   len = newlen;
   }
   memmove(p, path, plen);

Re: Fix for segfault in find(1)

2015-07-14 Thread Gregor Best 
On Tue, Jul 14, 2015 at 09:57:45AM -0600, Todd C. Miller wrote:
 [...]
 Shouldn't this be:
 
   p = (p - *store) + newstore;
 [...]

Of course, that makes way more sense. An amended patch is attached.

-- 
Gregor

Index: misc.c
===
RCS file: /mnt/media/cvs/src/usr.bin/find/misc.c,v
retrieving revision 1.12
diff -u -p -u -r1.12 misc.c
--- misc.c  18 May 2014 08:10:00 -  1.12
+++ misc.c  14 Jul 2015 16:07:28 -
@@ -65,6 +65,7 @@ brace_subst(char *orig, char **store, ch
 
if (!(newstore = realloc(*store, newlen)))
err(1, NULL);
+   p = (p - *store) + newstore;
*store = newstore;
len = newlen;
}

Re: Fix for segfault in find(1)

2015-07-14 Thread Todd C. Miller 
On Tue, 14 Jul 2015 12:55:35 -0400, Ted Unangst wrote:

 so technically i believe this is still undefined since you're not supposed to
 look at freed pointers. an even more better fix would be to save the offset
 before the realloc.

Yeah, I forgot we had to deref store.

 - todd

Index: misc.c
===
RCS file: /cvs/src/usr.bin/find/misc.c,v
retrieving revision 1.14
diff -u -p -u -r1.14 misc.c
--- misc.c  14 Jul 2015 16:58:22 -  1.14
+++ misc.c  14 Jul 2015 17:05:19 -
@@ -39,6 +39,7 @@
 #include err.h
 #include errno.h
 #include fts.h
+#include stddef.h
 #include stdio.h
 #include stdlib.h
 #include string.h
@@ -60,12 +61,14 @@ brace_subst(char *orig, char **store, ch
for (p = *store; (ch = *orig); ++orig)
if (ch == '{'  orig[1] == '}') {
while ((p - *store) + plen  len) {
+   ptrdiff_t p_off;
char *newstore;
 
+   p_off = (p - *store);
newstore = reallocarray(*store, len, 2);
if (newstore == NULL)
err(1, NULL);
-   p = (p - *store) + newstore;
+   p = newstore + p_off;
*store = newstore;
len *= 2;
}

Re: Fix for segfault in find(1)

2015-07-14 Thread Ted Unangst 
Gregor Best wrote:
 On Tue, Jul 14, 2015 at 09:57:45AM -0600, Todd C. Miller wrote:
  [...]
  Shouldn't this be:
  
  p = (p - *store) + newstore;
  [...]
 
 Of course, that makes way more sense. An amended patch is attached.

so technically i believe this is still undefined since you're not supposed to
look at freed pointers. an even more better fix would be to save the offset
before the realloc.

Re: Fix for segfault in find(1)

2015-07-14 Thread Ted Unangst 
Todd C. Miller wrote:
 On Tue, 14 Jul 2015 12:55:35 -0400, Ted Unangst wrote:
 
  so technically i believe this is still undefined since you're not supposed 
  to
  look at freed pointers. an even more better fix would be to save the offset
  before the realloc.
 
 Yeah, I forgot we had to deref store.


looks good.

 
  - todd
 
 Index: misc.c
 ===
 RCS file: /cvs/src/usr.bin/find/misc.c,v
 retrieving revision 1.14
 diff -u -p -u -r1.14 misc.c
 --- misc.c14 Jul 2015 16:58:22 -  1.14
 +++ misc.c14 Jul 2015 17:05:19 -
 @@ -39,6 +39,7 @@
  #include err.h
  #include errno.h
  #include fts.h
 +#include stddef.h
  #include stdio.h
  #include stdlib.h
  #include string.h
 @@ -60,12 +61,14 @@ brace_subst(char *orig, char **store, ch
   for (p = *store; (ch = *orig); ++orig)
   if (ch == '{'  orig[1] == '}') {
   while ((p - *store) + plen  len) {
 + ptrdiff_t p_off;
   char *newstore;
  
 + p_off = (p - *store);
   newstore = reallocarray(*store, len, 2);
   if (newstore == NULL)
   err(1, NULL);
 - p = (p - *store) + newstore;
 + p = newstore + p_off;
   *store = newstore;
   len *= 2;
   }