Jeroen Demeyer on Wed, 18 Jan 2017 15:33:00 +0100


[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]

Re: Use PROT_NONE for unused virtual stack memory


The problem was a bug in gp_main_loop() where parivstack_reset() was called *without* resetting avma. So you end up in a situation where avma was below bot. Before my patch, accessing memory below bot worked fine. With my patch, this might give a segmentation fault.

The fix is easy: reset avma when calling parivstack_reset(). I also made it an error to call parivstack_reset() when avma is below the new bottom of the stack. This will help to catch similar errors.

Patch attached...


Cheers,
Jeroen.
>From dc1d20081d09280f18820b565b80509f6e6c326e Mon Sep 17 00:00:00 2001
From: Jeroen Demeyer <jdemeyer@cage.ugent.be>
Date: Wed, 18 Jan 2017 13:45:39 +0000
Subject: [PATCH] Reset avma before calling parivstack_reset

---
 doc/usersch5.tex    | 9 ++++++---
 src/gp/gp.c         | 2 +-
 src/language/init.c | 2 ++
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/doc/usersch5.tex b/doc/usersch5.tex
index 58c7d04..6dc868c 100644
--- a/doc/usersch5.tex
+++ b/doc/usersch5.tex
@@ -193,9 +193,12 @@ at most \kbd{parisizemax}. The stack content is not affected
 by this operation.
 
 \fun{void}{parivstack_reset}{void}
-resets the current stack to its default size \kbd{parisize},
-destroying its content. Used to recover memory after a
-computation that enlarged the stack.
+resets the current stack to its default size \kbd{parisize}. This is
+used to recover memory after a computation that enlarged the stack.
+This function destroys the content of the enlarged stack (between
+the old and the new bottom of the stack).
+Before calling this function, you must ensure that \kbd{avma} lies
+within the new smaller stack.
 
 \fun{void}{paristack_newrsize}{ulong newsize}
 \emph{(does not return)}. Library version of
diff --git a/src/gp/gp.c b/src/gp/gp.c
index deaeec7..3463a2d 100644
--- a/src/gp/gp.c
+++ b/src/gp/gp.c
@@ -360,7 +360,6 @@ gp_main_loop(long ismain)
       if (ismain) continue;
       pop_buffer(); return z;
     }
-    avma = av;
     if (ismain)
     {
       reset_ctrlc();
@@ -384,6 +383,7 @@ gp_main_loop(long ismain)
     if (GP_DATA->simplify) z = simplify_shallow(z);
     pari_add_hist(z, t);
     if (z != gnil && ! is_silent(b->buf) ) gp_output(z);
+    avma = av;
     parivstack_reset();
   }
 }
diff --git a/src/language/init.c b/src/language/init.c
index 64783be..6c69c8e 100644
--- a/src/language/init.c
+++ b/src/language/init.c
@@ -786,6 +786,8 @@ void
 parivstack_reset(void)
 {
   pari_mainstack_setsize(pari_mainstack, pari_mainstack->rsize);
+  if (avma < pari_mainstack->bot)
+    pari_err_BUG("parivstack_reset [avma < bot]");
 }
 
 /* Enlarge the stack if needed such that the unused portion of the stack
-- 
2.7.3