Re: [Chicken-hackers] fix a bug/typo in srfi-4

[Evan Hanson]

Hi Kristian,

This was a good catch, thanks for the patch!

Pushed to master and chicken-5.

Best regards,

Evan

___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] fix a bug/typo in srfi-4

[Peter Bex]

On Thu, Apr 13, 2017 at 02:25:41PM +0200, Peter Bex wrote:
> On Thu, Apr 13, 2017 at 02:19:00PM +0200, Kristian Lein-Mathisen wrote:
> > Hi Chickeners,
> > 
> > As the commit says, this fixes a typo in s8vector-ref's setter.
> 
> Hi Kristian,
> 
> Well spotted, and a good fix, but could you also update the tests to
> include a regression tests for this and add a NEWS entry for 4.12.1?

I went ahead and added this to your patch (blame my impatience :P).
Attached are patches for master and chicken-5.

Cheers,
Peter
From 1fc800d85f2ecd18bbdaadc98c50c4336ce6af92 Mon Sep 17 00:00:00 2001
From: Kristian Lein-Mathisen 
Date: Thu, 13 Apr 2017 14:15:07 +0200
Subject: [PATCH] Fix a bug in srfi-4

this correctly uses s8vector-set! for the s8vector-ref's setter

Signed-off-by: Peter Bex 
---
 NEWS   | 2 ++
 srfi-4.scm | 2 +-
 tests/srfi-4-tests.scm | 7 ++-
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index b43e851..4713c83 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,8 @@
 interrupted by a signal, we now retry (thanks to Joerg Wittenberger).
   - char-ready? on string ports now also returns #t at EOF, as per R5RS;
 in other words, it always returns #t (thanks to Moritz Heidkamp)
+  - Unit srfi-4: Fixed typo that broke SRFI-17 generalised set! syntax
+on s8vectors (thanks to Kristian Lein-Mathisen).
 
 - Build system
   - Fixed broken compilation on NetBSD, due to missing _NETBSD_SOURCE.
diff --git a/srfi-4.scm b/srfi-4.scm
index 69f58ba..89e62ae 100644
--- a/srfi-4.scm
+++ b/srfi-4.scm
@@ -189,7 +189,7 @@ EOF
  (let ((len (##core#inline "C_u_i_s8vector_length" x)))
(check-range i 0 len 's8vector-ref)
(##core#inline "C_u_i_s8vector_ref" x i)))
-   u8vector-set!
+   s8vector-set!
"(s8vector-ref v i)"))
 
 (define u16vector-ref
diff --git a/tests/srfi-4-tests.scm b/tests/srfi-4-tests.scm
index 9daaa78..9af6c25 100644
--- a/tests/srfi-4-tests.scm
+++ b/tests/srfi-4-tests.scm
@@ -16,12 +16,17 @@
 	  (assert (= 100 (,(conc "vector-ref") x 0)))
   (assert (,(conc "vector?") x))
   (assert (number-vector? x))
+	  ;; Test direct setter and ref
 	  (,(conc "vector-set!") x 1 99)
 	  (assert (= 99 (,(conc "vector-ref") x 1)))
+	  ;; Test SRFI-17 generalised set! and ref
+	  (set! (,(conc "vector-ref") x 0) 127)
+	  (assert (= 127 (,(conc "vector-ref") x 0)))
+	  ;; Ensure length is okay
 	  (assert (= 2 (,(conc "vector-length") x)))
 	  (assert
 	   (every =
-		  '(100 99)
+		  '(127 99)
 		  (,(conc "vector->list") x
 
 (test1 u8)
-- 
2.1.4

From 16051070722bc433fad92a83c7522837257cf071 Mon Sep 17 00:00:00 2001
From: Kristian Lein-Mathisen 
Date: Thu, 13 Apr 2017 14:15:07 +0200
Subject: [PATCH] Fix a bug in srfi-4

this correctly uses s8vector-set! for the s8vector-ref's setter

Signed-off-by: Peter Bex 

Conflicts:
	tests/srfi-4-tests.scm
---
 NEWS   | 2 ++
 srfi-4.scm | 2 +-
 tests/srfi-4-tests.scm | 7 ++-
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index a107ebb..c61cf58 100644
--- a/NEWS
+++ b/NEWS
@@ -73,6 +73,8 @@
 interrupted by a signal, we now retry (thanks to Joerg Wittenberger).
   - char-ready? on string ports now also returns #t at EOF, as per R5RS;
 in other words, it always returns #t (thanks to Moritz Heidkamp)
+  - Unit srfi-4: Fixed typo that broke SRFI-17 generalised set! syntax
+on s8vectors (thanks to Kristian Lein-Mathisen).
 
 - Build system
   - Fixed broken compilation on NetBSD, due to missing _NETBSD_SOURCE.
diff --git a/srfi-4.scm b/srfi-4.scm
index 0371b9e..3d532d8 100644
--- a/srfi-4.scm
+++ b/srfi-4.scm
@@ -257,7 +257,7 @@ EOF
  (let ((len (##core#inline "C_u_i_s8vector_length" x)))
(check-range i 0 len 's8vector-ref)
(##core#inline "C_u_i_s8vector_ref" x i)))
-   u8vector-set!
+   s8vector-set!
"(s8vector-ref v i)"))
 
 (define u16vector-ref
diff --git a/tests/srfi-4-tests.scm b/tests/srfi-4-tests.scm
index b6dcebc..5f02ae5 100644
--- a/tests/srfi-4-tests.scm
+++ b/tests/srfi-4-tests.scm
@@ -17,12 +17,17 @@
 	  (assert (eqv? 100 (,(conc "vector-ref") x 0)))
 	  (assert (,(conc "vector?") x))
 	  (assert (number-vector? x))
+	  ;; Test direct setter and ref
 	  (,(conc "vector-set!") x 1 99)
 	  (assert (eqv? 99 (,(conc "vector-ref") x 1)))
+	  ;; Test SRFI-17 generalised set! and ref
+	  (set! (,(conc "vector-ref") x 0) 127)
+	  (assert (eqv? 127 (,(conc "vector-ref") x 0)))
+	  ;; Ensure length is okay
 	  (assert (= 2 (,(conc "vector-length") x)))
 	  (assert
 	   (let ((result (,(conc "vector->list") x)))
-	 (and (eqv? 100 (car result))
+	 (and (eqv? 127 (car result))
 		  (eqv? 99 (cadr result))
 
 (test1 u8 0 255)
-- 
2.1.4



signature.asc
Description: Digital signature
___
Chicken-hackers mailing list

Re: [Chicken-hackers] Fix

[Peter Bex]

On Fri, Apr 14, 2017 at 04:40:36PM +0200, Moritz Heidkamp wrote:
> Hi everyone,
> 
> char-ready? on string input ports would return #f when they've reached
> the end of their underlying string. However, char-ready? is supposed to
> return #t in this case. The attached patch fixes this and adds a
> corresponding regression test. It applies to both master and chicken-5.

Nice find, thanks!  I've pushed it to both branches and added a NEWS
entry, because this is a pretty important fix.

We should keep an eye on Salmonella to see if any eggs inadvertently rely
on this broken behaviour.

Cheers,
Peter


signature.asc
Description: Digital signature
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] Fix

[John Cowan]

On Fri, Apr 14, 2017 at 10:40 AM, Moritz Heidkamp <
moritz.heidk...@bevuta.com> wrote:

char-ready? on string input ports would return #f when they've reached
> the end of their underlying string. However, char-ready? is supposed to
> return #t in this case.
>

Yes.  The semantics of char-ready? is that it returns #f if there are no
characters available now, but there might be some available in future.  In
particular, at EOF there is always an EOF object available.

-- 
John Cowan  http://vrici.lojban.org/~cowanco...@ccil.org
Schlingt dreifach einen Kreis vom dies!
Schliesst euer Aug vor heiliger Schau,
Denn er genoss vom Honig-Tau,
Und trank die Milch vom Paradies.
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

[Chicken-hackers] Fix

[Moritz Heidkamp]

Hi everyone,

char-ready? on string input ports would return #f when they've reached
the end of their underlying string. However, char-ready? is supposed to
return #t in this case. The attached patch fixes this and adds a
corresponding regression test. It applies to both master and chicken-5.

Cheers
Moritz
>From 0fb9dd6eb4f9380e6ef44cf4ee2030e1f11ef412 Mon Sep 17 00:00:00 2001
From: Moritz Heidkamp 
Date: Fri, 14 Apr 2017 16:22:39 +0200
Subject: [PATCH] Fix char-ready? on EOF for string input ports

char-ready? on string input ports would return #f when they've reached
the end of their underlying string. However, char-ready? is supposed
to return #t in this case.
---
 library.scm  | 3 +--
 tests/port-tests.scm | 2 ++
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/library.scm b/library.scm
index 3caba429..51d6793b 100644
--- a/library.scm
+++ b/library.scm
@@ -4255,8 +4255,7 @@ EOF
 	   (##sys#setislot p 10 (fx+ position len)) ) ) )
  void ; close
  (lambda (p) #f)			; flush-output
- (lambda (p)			; char-ready?
-   (fx< (##sys#slot p 10) (##sys#slot p 11)) )
+ (lambda (p) #t)			; char-ready?
  (lambda (p n dest start)		; read-string!
(let* ((pos (##sys#slot p 10))
 	  (n2 (fx- (##sys#slot p 11) pos) ) )
diff --git a/tests/port-tests.scm b/tests/port-tests.scm
index ec6a323b..463e31e6 100644
--- a/tests/port-tests.scm
+++ b/tests/port-tests.scm
@@ -42,6 +42,8 @@ EOF
   (read-line p)))
 (assert (= 20 (length (read-lines (open-input-string *text*)
 
+(assert (char-ready? (open-input-string "")))
+
 (let ((out (open-output-string)))
   (test-equal "Initially, output string is empty"
   (get-output-string out) "")
-- 
2.12.0

___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] fix a bug/typo in srfi-4

[Peter Bex]

On Thu, Apr 13, 2017 at 02:19:00PM +0200, Kristian Lein-Mathisen wrote:
> Hi Chickeners,
> 
> As the commit says, this fixes a typo in s8vector-ref's setter.

Hi Kristian,

Well spotted, and a good fix, but could you also update the tests to
include a regression tests for this and add a NEWS entry for 4.12.1?

Cheers,
Peter


signature.asc
Description: Digital signature
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

[Chicken-hackers] fix a bug/typo in srfi-4

[Kristian Lein-Mathisen]

Hi Chickeners,

As the commit says, this fixes a typo in s8vector-ref's setter.

It can be applied in both chicken-5 and master.

K.
From 9a3f1b789bd937d9442aa24473e528fc33eb7d10 Mon Sep 17 00:00:00 2001
From: Kristian Lein-Mathisen 
Date: Thu, 13 Apr 2017 14:15:07 +0200
Subject: [PATCH] Fix a bug in srfi-4

this correctly uses s8vector-set! for the s8vector-ref's setter
---
 srfi-4.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/srfi-4.scm b/srfi-4.scm
index 0371b9e6..3d532d80 100644
--- a/srfi-4.scm
+++ b/srfi-4.scm
@@ -257,7 +257,7 @@ EOF
  (let ((len (##core#inline "C_u_i_s8vector_length" x)))
(check-range i 0 len 's8vector-ref)
(##core#inline "C_u_i_s8vector_ref" x i)))
-   u8vector-set!
+   s8vector-set!
"(s8vector-ref v i)"))
 
 (define u16vector-ref
-- 
2.12.2

___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] Fix buffer overflow found by #1308

[Peter Bex]

On Fri, Aug 12, 2016 at 06:23:16PM +1200, Evan Hanson wrote:
> Hi folks,
> 
> Looks good, thanks for digging into this issue. I've pushed these. I
> also restored the return value of process-spawn, which was changed by
> the patch (unintentionally, I think) but must preserve the result of
> spawnvp[e] in order to be useful.

Good catch, I didn't notice that!

Cheers,
Peter


signature.asc
Description: Digital signature
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] Fix buffer overflow found by #1308

[Evan Hanson]

Hi folks,

Looks good, thanks for digging into this issue. I've pushed these. I
also restored the return value of process-spawn, which was changed by
the patch (unintentionally, I think) but must preserve the result of
spawnvp[e] in order to be useful.

Cheers,

Evan


signature.asc
Description: Digital signature
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] Fix buffer overflow found by #1308

[Peter Bex]

On Thu, Jul 28, 2016 at 09:19:14PM +0200, Peter Bex wrote:
> We've discussed this some more on IRC, and I think that we all agree
> that it's better to refactor this whole mess.  The attached patch is
> a complete overhaul of process-execute and process-spawn (which was
> equally affected by the ARG_MAX/ENV_MAX sized buffer overrun).

The previous mailed contained only a patch for master.  Here's the
chicken-5 patch.

Cheers,
Peter
From c7568d565fabd1d961c76e71c5716de0216330cc Mon Sep 17 00:00:00 2001
From: Christian Kellermann 
Date: Sat, 23 Jul 2016 21:23:50 +0200
Subject: [PATCH] Fix buffer overflow in posix execvp/execve wrapper

This fixes bug #1308 found by wasamasa.  It turns out that we don't
check the number of arguments or the number of env entries before trying
to write them to the target string.

Instead of checking the argument count, this patch replaces the static
buffer with a dynamically allocated string and relies on errno being
set to E2BIG if the argument vector is too large.

Furthermore, this merges the process-execute and process-spawn
code from Windows and Unix some more to use more common code.
This should make it easier to tweak this code in the future.

This new version also fixes a memory leak which would be
triggered when the arg or env list contained non-string objects
or embedded NULs, or when the exec itself would fail.

Most C code in these procedures was rewritten to Scheme.

Signed-off-by: Peter Bex 

Conflicts:
	manual/Acknowledgements
	posix-common.scm
	posixunix.scm
	posixwin.scm
---
 NEWS|   6 ++
 manual/Acknowledgements |  62 +--
 posix-common.scm|  62 +++
 posixunix.scm   |  84 -
 posixwin.scm| 159 
 rules.make  |   2 +
 6 files changed, 165 insertions(+), 210 deletions(-)

diff --git a/NEWS b/NEWS
index d4eb49d..bc03768 100644
--- a/NEWS
+++ b/NEWS
@@ -53,6 +53,12 @@
 
 4.11.1
 
+- Security fixes
+  - Fix buffer overrun due to excessively long argument or
+environment lists in process-execute and process-spawn (#1308).
+This also removes unnecessary limitations on the length of
+these lists (thanks to Vasilij Schneidermann).
+
 - Compiler:
   - define-constant now correctly keeps symbol values quoted.
   - Warnings are now emitted when using vector-{ref,set!} or one
diff --git a/manual/Acknowledgements b/manual/Acknowledgements
index 7cfb6c6..7b6fe2f 100644
--- a/manual/Acknowledgements
+++ b/manual/Acknowledgements
@@ -6,15 +6,15 @@ Many thanks to Jules Altfas, Nico Amtsberg, Alonso Andres, William
 Annis, Jason E. Aten, Marc Baily, Peter Barabas, Andrei Barbu, Jonah
 Beckford, Arto Bendiken, Andy Bennett, Kevin Beranek, Peter Bex,
 Jean-Francois Bignolles, Oivind Binde, Alaric Blagrave Snell-Pym, Dave
-Bodenstab, Fabian Böhlke, T. Kurt Bond, Ashley Bone, Dominique Boucher,
-Terence Brannon, Roy Bryant, Adam Buchbinder, Hans Bulfone, "Category
-5", Taylor Campbell, Naruto Canada, Mark Carter, Esteban U. Caamano
-Castro, Semih Cemiloglu, Alex Charlton, Franklin Chen, Joo ChurlSoo,
-Thomas Chust, Gian Paolo Ciceri, Fulvio Ciriaco, Paul Colby, Tobia
-Conforto, John Cowan, Grzegorz Chrupala, James Crippen, Evan Hanson,
-Adhi Hargo, Moritz Heidkamp, Tollef Fog Heen, Drew Hess, Alejandro
-Forero Cuervo, Peter Danenberg, Linh Dang, Brian Denheyer, Sean
-D'Epagnier, "dgym", "Don", Chris Double, "Brown Dragon", David
+Bodenstab, Fabian Böhlke, T. Kurt Bond, Ashley Bone, Dominique
+Boucher, Terence Brannon, Roy Bryant, Adam Buchbinder, Hans Bulfone,
+"Category 5", Taylor Campbell, Naruto Canada, Mark Carter, Esteban
+U. Caamano Castro, Semih Cemiloglu, Alex Charlton, Franklin Chen, Joo
+ChurlSoo, Thomas Chust, Gian Paolo Ciceri, Fulvio Ciriaco, Paul Colby,
+Tobia Conforto, John Cowan, Grzegorz Chrupala, James Crippen, Evan
+Hanson, Adhi Hargo, Moritz Heidkamp, Tollef Fog Heen, Drew Hess,
+Alejandro Forero Cuervo, Peter Danenberg, Linh Dang, Brian Denheyer,
+Sean D'Epagnier, "dgym", "Don", Chris Double, "Brown Dragon", David
 Dreisigmeyer, Jarod Eells, Petter Egesund, Stephen Eilert, Steve
 Elkins, Daniel B. Faken, Erik Falor, Will Farr, Graham Fawcett, Marc
 Feeley, "Fizzie", Matthew Flatt, Kimura Fuyuki, Tony Garnock-Jones,
@@ -28,33 +28,33 @@ Jäger, Matt Jones, Dale Jordan, Valentin Kamyshenko, Daishi Kato,
 Peter Keller, Christian Kellermann, Brad Kind, Ron Kneusel, Matthias
 Köppe, Krysztof Kowalczyk, Andre Kühne, Todd R. Kueny Sr, Goran
 Krampe, David Krentzlin, Ben Kurtz, Michele La Monaca, Micky
-Latowicki, Kristian Lein-Mathisen, "LemonBoy", John Lenz,
-Kirill Lisovsky, Jürgen Lorenz, Kon Lovett, Lam Luu, Arthur Maciel,
-Vitaly Magerya, Leonardo Valeri Manera, Claude Marinier, Dennis Marti,
+Latowicki, Kristian Lein-Mathisen, "LemonBoy", John Lenz, Kirill
+Lisovsky, Jürgen Lorenz, Kon Lovett, Lam Luu, Arthur Maciel, Vitaly
+Magerya, Leonardo Valeri 

Re: [Chicken-hackers] Fix buffer overflow found by #1308

[Peter Bex]

On Sat, Jul 23, 2016 at 10:26:49PM +0200, Christian Kellermann wrote:
> Hi!
> 
> The attached patch adds range checks avoiding a buffer overflow for
> the environment and argument buffers used in the posix execvp/execve
> wrappers on linux and windows.
> 
> Does that look OK?

We've discussed this some more on IRC, and I think that we all agree
that it's better to refactor this whole mess.  The attached patch is
a complete overhaul of process-execute and process-spawn (which was
equally affected by the ARG_MAX/ENV_MAX sized buffer overrun).

Why I think this substantial refactoring is a good idea:

- ARG_MAX does not represent the _number_ of arguments, but the total
   size of the memory slab holding both the arguments PLUS the
   environment (so this includes the argument / environment strings
   themselves, not just the vector).  ENV_MAX doesn't seem to exist (?!)

- In modern glibc-based Linuxen, ARG_MAX is no longer being defined
   by limits.h.  Instead, you can query it using sysconf(3).
   Our fallback code defining it to be 256 is conservative to the point
   of being useless (especially if you take into account the preceding
   bullet point).

- There's no need to check the buffer length in advance, the kernel will
   guard against too large sizes, so the syscall will fail, and we can
   simply check errno when the function returns (but see below).

- There's too much C in the old implementation.  This code doesn't have
   to be blazingly fast, and it should really be safe.  So it makes more
   sense to do the array creation and indexing in Scheme too.  The
   patch uses a pointer-vector.

- Case in point: the original code also had a nasty memory leak: if the
   arguments or environment list contained non-string values, it would
   raise an exception, but it didn't free the already-allocated strings
   in the vectors.  The current call-with-exec-args takes care to do
   this correctly.

- This code is kind of long, and tricky.  I decided to put as much shared
   and reusable code as possible into posix-common.scm.  The shell
   quoting stuff is simply a "conversion" procedure you can pass to
   call-with-exec-args, which in the unix case is the IDENTITY procedure
   (but not by that name, because we don't load data-structures here).


The one disadvantage of this approach, which DeeEff on IRC found out, is
that on Windows, the errno is _always_ set to ENOFILE, even if you pass
too many arguments.  This is misleading, because the error message is
"no such file", which is the same error you get if the command is unknown.

Cheers,
Peter
From 3b0bcb9598d53a2f796203339679e7f51ae1 Mon Sep 17 00:00:00 2001
From: Christian Kellermann 
Date: Sat, 23 Jul 2016 21:23:50 +0200
Subject: [PATCH] Fix buffer overflow in posix execvp/execve wrapper

This fixes bug #1308 found by wasamasa.  It turns out that we don't
check the number of arguments or the number of env entries before trying
to write them to the target string.

Instead of checking the argument count, this patch replaces the static
buffer with a dynamically allocated string and relies on errno being
set to E2BIG if the argument vector is too large.

Furthermore, this merges the process-execute and process-spawn
code from Windows and Unix some more to use more common code.
This should make it easier to tweak this code in the future.

This new version also fixes a memory leak which would be
triggered when the arg or env list contained non-string objects
or embedded NULs, or when the exec itself would fail.

Most C code in these procedures was rewritten to Scheme.

Signed-off-by: Peter Bex 
---
 NEWS|   6 ++
 manual/Acknowledgements |  44 +++---
 posix-common.scm|  65 +++-
 posixunix.scm   |  83 -
 posixwin.scm| 158 +++-
 5 files changed, 154 insertions(+), 202 deletions(-)

diff --git a/NEWS b/NEWS
index c557b6a..b1a110d 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,11 @@
 4.11.1
 
+- Security fixes
+  - Fix buffer overrun due to excessively long argument or
+environment lists in process-execute and process-spawn (#1308).
+This also removes unnecessary limitations on the length of
+these lists (thanks to Vasilij Schneidermann).
+
 - Compiler:
   - define-constant now correctly keeps symbol values quoted.
   - Warnings are now emitted when using vector-{ref,set!} or one
diff --git a/manual/Acknowledgements b/manual/Acknowledgements
index 8169b72..8f34616 100644
--- a/manual/Acknowledgements
+++ b/manual/Acknowledgements
@@ -6,15 +6,15 @@ Many thanks to Jules Altfas, Nico Amtsberg, Alonso Andres, William
 Annis, Jason E. Aten, Marc Baily, Peter Barabas, Andrei Barbu, Jonah
 Beckford, Arto Bendiken, Andy Bennett, Kevin Beranek, Peter Bex,
 Jean-Francois Bignolles, Oivind Binde, Alaric Blagrave Snell-Pym, Dave
-Bodenstab, Fabian Böhlke, T. Kurt Bond, Ashley Bone, 

[Chicken-hackers] Fix buffer overflow found by #1308

[Christian Kellermann]

Hi!

The attached patch adds range checks avoiding a buffer overflow for
the environment and argument buffers used in the posix execvp/execve
wrappers on linux and windows.

Does that look OK?

Cheers,

Christian

--
May you be peaceful, may you live in safety, may you be free from
suffering, and may you live with ease.
>From 643848bbad2cb8858030507ca1c26bb0817b3633 Mon Sep 17 00:00:00 2001
From: Christian Kellermann 
Date: Sat, 23 Jul 2016 21:23:50 +0200
Subject: [PATCH] Fix buffer overflow in posix execvp/execve wrapper

This fixes bug #1308 found by wasamasa.  It turns out that we don't
check the number of arguments or the number of env entries before trying
to write them to the target string. That target string is only ARG_MAX
or ENV_MAX bytes long.

This patch adds a ##sys#check-range for both arguments.

The unix implementation and the windows implementation are both affected
and fixed with this patch.
---
 posixunix.scm | 6 +-
 posixwin.scm  | 9 ++---
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/posixunix.scm b/posixunix.scm
index a21d0b0..cc9c947 100644
--- a/posixunix.scm
+++ b/posixunix.scm
@@ -1599,10 +1599,13 @@ EOF
 [freeargs (foreign-lambda void "C_free_exec_args")]
 [setenv (foreign-lambda void "C_set_exec_env" int c-string int)]
 [freeenv (foreign-lambda void "C_free_exec_env")]
-[pathname-strip-directory pathname-strip-directory] )
+[pathname-strip-directory pathname-strip-directory]
+[arg-max (foreign-value "ARG_MAX" size_t)]
+[env-max (foreign-value "ENV_MAX" size_t)] )
 (lambda (filename #!optional (arglist '()) envlist)
   (##sys#check-string filename 'process-execute)
   (##sys#check-list arglist 'process-execute)
+  (##sys#check-range (##sys#length arglist) 0 arg-max 'process-execute)
   (let ([s (pathname-strip-directory filename)])
 (setarg 0 s (##sys#size s)) )
   (do ([al arglist (cdr al)]
@@ -1611,6 +1614,7 @@ EOF
(setarg i #f 0)
(when envlist
  (##sys#check-list envlist 'process-execute)
+ (##sys#check-range (##sys#length envlist) 0  env-max 
'process-execute)
  (do ([el envlist (cdr el)]
   [i 0 (fx+ i 1)] )
  ((null? el) (setenv i #f 0))
diff --git a/posixwin.scm b/posixwin.scm
index 2f46aaf..cc9a583 100644
--- a/posixwin.scm
+++ b/posixwin.scm
@@ -1191,11 +1191,14 @@ EOF
   ;; At least it's secure, we can worry about performance later, if at all
   (let ([setarg (foreign-lambda void "C_set_exec_arg" int c-string int)]
[setenv (foreign-lambda void "C_set_exec_env" int c-string int)]
+   [arg-max (foreign-value "ARG_MAX" size_t)]
+   [env-max (foreign-value "ENV_MAX" size_t)]
[build-exec-argvec
- (lambda (loc lst argvec-setter idx)
+ (lambda (loc lst argvec-setter idx max)
(if lst
  (begin
(##sys#check-list lst loc)
+   (##sys-check-range (##sys#length lst) 0 max loc)
(do ([l lst (cdr l)]
 [i idx (fx+ i 1)] )
((null? l) (argvec-setter i #f 0))
@@ -1207,8 +1210,8 @@ EOF
   (##sys#check-string filename loc)
   (let ([s (pathname-strip-directory filename)])
(setarg 0 s (##sys#size s)) )
-  (build-exec-argvec loc (and arglst ($quote-args-list arglst exactf)) 
setarg 1)
-  (build-exec-argvec loc envlst setenv 0)
+  (build-exec-argvec loc (and arglst ($quote-args-list arglst exactf)) 
setarg 1 arg-max)
+  (build-exec-argvec loc envlst setenv 0 env-max)
   (##core#inline "C_flushall")
   (##sys#make-c-string filename loc) ) ) )
 
-- 
2.9.0

___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] Fix attempt for bug #1269 "Port or reader state corruption"

[Peter Bex]

On Sun, Apr 03, 2016 at 06:36:16PM +1200, Evan Hanson wrote:
> Hi all,
> 
> Signoff attached.
> 
> As Kooda says, this applies and tests fine on the chicken-5 branch as
> well.
> 
> Thanks very much for pursuing this, and thanks also to lemonboy for the
> initial report and subsequent testing and investigation.

Thanks everyone for your perseverence and hard work to find the cause
and fix the bug.  I've pushed it to both branches.

Cheers,
Peter


signature.asc
Description: Digital signature
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] Fix attempt for bug #1269 "Port or reader state corruption"

[Evan Hanson]

Hi all,

Signoff attached.

As Kooda says, this applies and tests fine on the chicken-5 branch as
well.

Thanks very much for pursuing this, and thanks also to lemonboy for the
initial report and subsequent testing and investigation.

Cheers,

Evan
>From 7d7fd72b9738a359d619d277d43b80dc07a8aefa Mon Sep 17 00:00:00 2001
From: Kooda 
Date: Sat, 19 Mar 2016 13:21:43 +0100
Subject: [PATCH] Clean up process exit and flush output streams

Flush standard output streams before both forking and exiting, and use
C_exit_runtime() uniformly to exit the process.

This fixes bug #1269.

See this mail thread for details:

  http://lists.nongnu.org/archive/html/chicken-hackers/2016-03/msg00022.html

Signed-off-by: Evan Hanson 
---
 chicken.h |  3 ++-
 dbg-stub.c|  2 +-
 posixunix.scm |  4 +++-
 runtime.c | 14 +++---
 4 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/chicken.h b/chicken.h
index be5f308..e64d0bb 100644
--- a/chicken.h
+++ b/chicken.h
@@ -907,6 +907,7 @@ typedef void (C_ccall *C_proc)(C_word, C_word *) C_noret;
 # define C_fflush   fflush
 # define C_getchar  getchar
 # define C_exit exit
+# define C__exit_exit
 # define C_dlopen   dlopen
 # define C_dlclose  dlclose
 # define C_dlsymdlsym
@@ -1751,7 +1752,7 @@ C_fctexport C_word C_message(C_word msg);
 C_fctexport C_word C_fcall C_equalp(C_word x, C_word y) C_regparm;
 C_fctexport C_word C_fcall C_set_gc_report(C_word flag) C_regparm;
 C_fctexport C_word C_fcall C_start_timer(void) C_regparm;
-C_fctexport C_word C_exit_runtime(C_word code);
+C_fctexport C_word C_exit_runtime(C_word code) C_noret;
 C_fctexport C_word C_fcall C_set_print_precision(C_word n) C_regparm;
 C_fctexport C_word C_fcall C_get_print_precision(void) C_regparm;
 C_fctexport C_word C_fcall C_read_char(C_word port) C_regparm;
diff --git a/dbg-stub.c b/dbg-stub.c
index 39dee2a..e9cdd14 100644
--- a/dbg-stub.c
+++ b/dbg-stub.c
@@ -201,7 +201,7 @@ terminate(char *msg)
 {
   fprintf(stderr, "%s\n", msg);
   socket_close();
-  _exit(1);
+  C_exit_runtime(C_fix(1));
 }
 
 
diff --git a/posixunix.scm b/posixunix.scm
index ede6877..a21d0b0 100644
--- a/posixunix.scm
+++ b/posixunix.scm
@@ -1577,6 +1577,8 @@ EOF
 (define process-fork
   (let ((fork (foreign-lambda int "C_fork")))
 (lambda (#!optional thunk killothers)
+  ;; flush all stdio streams before fork
+  ((foreign-lambda int "C_fflush" c-pointer) #f)
   (let ((pid (fork)))
 	(when (fx= -1 pid) 
 	  (posix-error #:process-error 'process-fork "cannot create child process"))
@@ -1586,7 +1588,7 @@ EOF
 		 (lambda (thunk) (thunk)))
 	 (lambda ()
 	   (thunk)
-	   ((foreign-lambda void "_exit" int) 0) ))
+	   (exit 0)))
 	pid)
 
 (define process-execute
diff --git a/runtime.c b/runtime.c
index f11789a..8f1e011 100644
--- a/runtime.c
+++ b/runtime.c
@@ -1311,7 +1311,7 @@ void CHICKEN_parse_command_line(int argc, char *argv[], C_word *heap, C_word *st
 		 " -:S  do not handle segfaults or other serious conditions\n"
 		 "\n  SIZE may have a `k' (`K'), `m' (`M') or `g' (`G') suffix, meaning size\n"
 		 "  times 1024, 1048576, and 1073741824, respectively.\n\n");
-	  exit(0);
+	  C_exit_runtime(C_fix(0));
 
 	case 'h':
 	  switch(*ptr) {
@@ -1527,7 +1527,7 @@ void C_ccall termination_continuation(C_word c, C_word *av)
 C_dbg(C_text("debug"), C_text("application terminated normally\n"));
   }
 
-  exit(0);
+  C_exit_runtime(C_fix(0));
 }
 
 
@@ -1556,7 +1556,7 @@ void usual_panic(C_char *msg)
   } /* fall through if not WIN32 GUI app */
 
   C_dbg("panic", C_text("%s - execution terminated\n\n%s"), msg, dmp);
-  C_exit(1);
+  C_exit_runtime(C_fix(1));
 }
 
 
@@ -1573,7 +1573,7 @@ void horror(C_char *msg)
   } /* fall through */
 
   C_dbg("horror", C_text("\n%s - execution terminated"), msg);  
-  C_exit(1);
+  C_exit_runtime(C_fix(1));
 }
 
 
@@ -4152,7 +4152,7 @@ C_word C_halt(C_word msg)
   if(dmp != NULL) 
 C_dbg("", C_text("\n%s"), dmp);
   
-  C_exit(EX_SOFTWARE);
+  C_exit_runtime(C_fix(EX_SOFTWARE));
   return 0;
 }
 
@@ -4274,8 +4274,8 @@ void C_ccall C_stop_timer(C_word c, C_word *av)
 
 C_word C_exit_runtime(C_word code)
 {
-  exit(C_unfix(code));
-  return 0;			/* to please the compiler... */
+  C_fflush(NULL);
+  C__exit(C_unfix(code));
 }
 
 
-- 
2.7.0

___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] Fix attempt for bug #1269 "Port or reader state corruption"

[Kooda]

Thanks for your comment. :)

Here is a new version of the patch.

It seems to apply to the chicken-5 branch without problem as well.

I hope this one is right. :)

>From 88a0753bc4abf7bd25c1d90449fa927d0420ddc5 Mon Sep 17 00:00:00 2001
From: Kooda 
Date: Sat, 19 Mar 2016 13:21:43 +0100
Subject: [PATCH] Cleanup process exit

This fixes bug #1269

See this mail thread for details:
http://lists.nongnu.org/archive/html/chicken-hackers/2016-03/msg00022.html
---
 chicken.h |  3 ++-
 dbg-stub.c|  2 +-
 posixunix.scm |  4 +++-
 runtime.c | 14 +++---
 4 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/chicken.h b/chicken.h
index be5f308..e64d0bb 100644
--- a/chicken.h
+++ b/chicken.h
@@ -907,6 +907,7 @@ typedef void (C_ccall *C_proc)(C_word, C_word *) C_noret;
 # define C_fflush   fflush
 # define C_getchar  getchar
 # define C_exit exit
+# define C__exit_exit
 # define C_dlopen   dlopen
 # define C_dlclose  dlclose
 # define C_dlsymdlsym
@@ -1751,7 +1752,7 @@ C_fctexport C_word C_message(C_word msg);
 C_fctexport C_word C_fcall C_equalp(C_word x, C_word y) C_regparm;
 C_fctexport C_word C_fcall C_set_gc_report(C_word flag) C_regparm;
 C_fctexport C_word C_fcall C_start_timer(void) C_regparm;
-C_fctexport C_word C_exit_runtime(C_word code);
+C_fctexport C_word C_exit_runtime(C_word code) C_noret;
 C_fctexport C_word C_fcall C_set_print_precision(C_word n) C_regparm;
 C_fctexport C_word C_fcall C_get_print_precision(void) C_regparm;
 C_fctexport C_word C_fcall C_read_char(C_word port) C_regparm;
diff --git a/dbg-stub.c b/dbg-stub.c
index 39dee2a..e9cdd14 100644
--- a/dbg-stub.c
+++ b/dbg-stub.c
@@ -201,7 +201,7 @@ terminate(char *msg)
 {
   fprintf(stderr, "%s\n", msg);
   socket_close();
-  _exit(1);
+  C_exit_runtime(C_fix(1));
 }
 
 
diff --git a/posixunix.scm b/posixunix.scm
index f56960d..5f2e20b 100644
--- a/posixunix.scm
+++ b/posixunix.scm
@@ -1573,6 +1573,8 @@ EOF
 (define process-fork
   (let ((fork (foreign-lambda int "C_fork")))
 (lambda (#!optional thunk killothers)
+  ;; flush all stdio streams before fork
+  ((foreign-lambda int "C_fflush" c-pointer) #f)
   (let ((pid (fork)))
(when (fx= -1 pid) 
  (posix-error #:process-error 'process-fork "cannot create child 
process"))
@@ -1582,7 +1584,7 @@ EOF
 (lambda (thunk) (thunk)))
 (lambda ()
   (thunk)
-  ((foreign-lambda void "_exit" int) 0) ))
+  (exit 0)))
pid)
 
 (define process-execute
diff --git a/runtime.c b/runtime.c
index 8e89ea3..613375e 100644
--- a/runtime.c
+++ b/runtime.c
@@ -1311,7 +1311,7 @@ void CHICKEN_parse_command_line(int argc, char *argv[], 
C_word *heap, C_word *st
 " -:S  do not handle segfaults or other serious 
conditions\n"
 "\n  SIZE may have a `k' (`K'), `m' (`M') or `g' (`G') suffix, 
meaning size\n"
 "  times 1024, 1048576, and 1073741824, respectively.\n\n");
- exit(0);
+ C_exit_runtime(C_fix(0));
 
case 'h':
  switch(*ptr) {
@@ -1527,7 +1527,7 @@ void C_ccall termination_continuation(C_word c, C_word 
*av)
 C_dbg(C_text("debug"), C_text("application terminated normally\n"));
   }
 
-  exit(0);
+  C_exit_runtime(C_fix(0));
 }
 
 
@@ -1556,7 +1556,7 @@ void usual_panic(C_char *msg)
   } /* fall through if not WIN32 GUI app */
 
   C_dbg("panic", C_text("%s - execution terminated\n\n%s"), msg, dmp);
-  C_exit(1);
+  C_exit_runtime(C_fix(1));
 }
 
 
@@ -1573,7 +1573,7 @@ void horror(C_char *msg)
   } /* fall through */
 
   C_dbg("horror", C_text("\n%s - execution terminated"), msg);  
-  C_exit(1);
+  C_exit_runtime(C_fix(1));
 }
 
 
@@ -4152,7 +4152,7 @@ C_word C_halt(C_word msg)
   if(dmp != NULL) 
 C_dbg("", C_text("\n%s"), dmp);
   
-  C_exit(EX_SOFTWARE);
+  C_exit_runtime(C_fix(EX_SOFTWARE));
   return 0;
 }
 
@@ -4274,8 +4274,8 @@ void C_ccall C_stop_timer(C_word c, C_word *av)
 
 C_word C_exit_runtime(C_word code)
 {
-  exit(C_unfix(code));
-  return 0;/* to please the compiler... */
+  C_fflush(NULL);
+  C__exit(C_unfix(code));
 }
 
 
-- 
2.1.4

___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] Fix attempt for bug #1269 "Port or reader state corruption"

[Evan Hanson]

On 2016-03-27 21:30, Kooda wrote:
> I went for the first solution for now, I suppose it??s a good enough??
> fix for now. A better one should go to CHICKEN 5.

Agreed, though I don't have any idea what that will look like, now. I do
think this one should be applied to both, though, to avoid divergence in
the meantime.

> diff --git a/chicken.h b/chicken.h
> index be5f308..0c985cf 100644
> --- a/chicken.h
> +++ b/chicken.h
> @@ -906,7 +906,7 @@ typedef void (C_ccall *C_proc)(C_word, C_word *) C_noret;
>  # define C_vfprintf vfprintf
>  # define C_fflush   fflush
>  # define C_getchar  getchar
> -# define C_exit exit
> +# define C_exit(x)  { C_fflush(NULL); _exit(x); }

I'd prefer to leave C_exit as it is and introduce C__exit instead, or
perhaps put the fflush() call into C_exit_runtime and use that directly
wherever an _exit() is needed. Otherwise, C_exit will be the one C_foo
that isn't just an alias for foo, which seems like asking for trouble.

> diff --git a/posixunix.scm b/posixunix.scm
> index f56960d..ae1b9b0 100644
> --- a/posixunix.scm
> +++ b/posixunix.scm
> @@ -1573,6 +1573,8 @@ EOF
>  (define process-fork
>(let ((fork (foreign-lambda int "C_fork")))
>  (lambda (#!optional thunk killothers)
> +  ;; flush all stdio streams before fork
> +  ((foreign-lambda int "fflush" c-pointer) #f)

"C_fflush".

Cheers,

Evan

___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] Fix attempt for bug #1269 "Port or reader state corruption"

[Evan Hanson]

Hi Kooda,

On 2016-03-25 17:30, Kooda wrote:
> [... various POSIX absurdities...]

What a mess.

> From this, I only see a handful ways of working around the bug:
> 
> - Calling _exit() instead of exit() in the default exit-handler. This
>   would prevent the streams from synchronizing with their FDs but
>   would completely throw away functions registered with atexit() or
>   on_exit()

This seems like an acceptable solution to me. We already use _exit() to
avoid similar shenanigans when a thunked child process exits implicitly,
and we currently make no guarantees about atexit() handlers so at least
we won't be breaking any declared functionality. There's also `on-exit`,
which *is* correctly inherited.

> - Calling fflush() on every input file owned by CHICKEN before calling
>   fork(). This would require a way to get all FILE* streams opened by
>   CHICKEN, I don?t know if we have that. 

We don't have that, and the overhead of global FILE* tracking is too
high a price to pay for such a stupid issue.

>   It would not cause the atexit() problem mentioned earlier, but it
>   wouldn?t be completely general, as we wouldn?t be able to call
>   fflush() on streams not owned by CHICKEN.

Also this. And again, I don't really see the atexit() problem as much of
one.

> Anyhow, we should at least add a call to fflush(NULL) in the
> process-fork procedure, as it takes care of the output streams.

Agreed. If only that were enough.

Anyway, your patch seems like it's on the right track to me. Thanks for
investigating this, Kooda.

Evan

___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] Fix attempt for bug #1269 "Port or reader state corruption"

[Kooda]

Here is a new patch for this issue.

I went for the first solution for now, I suppose it’s a good enough™
fix for now. A better one should go to CHICKEN 5.

>From c66052c91e1622e0b3dc10cd01fc0dacf7d9827c Mon Sep 17 00:00:00 2001
From: Kooda 
Date: Sat, 19 Mar 2016 13:21:43 +0100
Subject: [PATCH] Cleanup process exit

This fixes bug #1269

See this mail thread for details:
http://lists.nongnu.org/archive/html/chicken-hackers/2016-03/msg00022.html
---
 chicken.h | 2 +-
 posixunix.scm | 4 +++-
 runtime.c | 6 +++---
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/chicken.h b/chicken.h
index be5f308..0c985cf 100644
--- a/chicken.h
+++ b/chicken.h
@@ -906,7 +906,7 @@ typedef void (C_ccall *C_proc)(C_word, C_word *) C_noret;
 # define C_vfprintf vfprintf
 # define C_fflush   fflush
 # define C_getchar  getchar
-# define C_exit exit
+# define C_exit(x)  { C_fflush(NULL); _exit(x); }
 # define C_dlopen   dlopen
 # define C_dlclose  dlclose
 # define C_dlsymdlsym
diff --git a/posixunix.scm b/posixunix.scm
index f56960d..ae1b9b0 100644
--- a/posixunix.scm
+++ b/posixunix.scm
@@ -1573,6 +1573,8 @@ EOF
 (define process-fork
   (let ((fork (foreign-lambda int "C_fork")))
 (lambda (#!optional thunk killothers)
+  ;; flush all stdio streams before fork
+  ((foreign-lambda int "fflush" c-pointer) #f)
   (let ((pid (fork)))
(when (fx= -1 pid) 
  (posix-error #:process-error 'process-fork "cannot create child 
process"))
@@ -1582,7 +1584,7 @@ EOF
 (lambda (thunk) (thunk)))
 (lambda ()
   (thunk)
-  ((foreign-lambda void "_exit" int) 0) ))
+  ((foreign-lambda void "C_exit" int) 0) ))
pid)
 
 (define process-execute
diff --git a/runtime.c b/runtime.c
index 8e89ea3..2ac8c2f 100644
--- a/runtime.c
+++ b/runtime.c
@@ -1311,7 +1311,7 @@ void CHICKEN_parse_command_line(int argc, char *argv[], 
C_word *heap, C_word *st
 " -:S  do not handle segfaults or other serious 
conditions\n"
 "\n  SIZE may have a `k' (`K'), `m' (`M') or `g' (`G') suffix, 
meaning size\n"
 "  times 1024, 1048576, and 1073741824, respectively.\n\n");
- exit(0);
+ C_exit(0);
 
case 'h':
  switch(*ptr) {
@@ -1527,7 +1527,7 @@ void C_ccall termination_continuation(C_word c, C_word 
*av)
 C_dbg(C_text("debug"), C_text("application terminated normally\n"));
   }
 
-  exit(0);
+  C_exit(0);
 }
 
 
@@ -4274,7 +4274,7 @@ void C_ccall C_stop_timer(C_word c, C_word *av)
 
 C_word C_exit_runtime(C_word code)
 {
-  exit(C_unfix(code));
+  C_exit(C_unfix(code));
   return 0;/* to please the compiler... */
 }
 
-- 
2.1.4

___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] Fix attempt for bug #1269 "Port or reader state corruption"

[Kooda]

Sorry, I forgot to post a link that explains what should be done with
FILE streams in case of fork.

Here it is:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_05_01

___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] Fix attempt for bug #1269 "Port or reader state corruption"

[Kooda]

So!

I’ve been spending some time on the musl libc IRC channel today, and
discussed the issue with them.

The conclusion is that the bug is indeed caused by the exit() call,
which is required to synchronize stdio FILE* streams with their
associated file descriptor.

Unfortunately, in evhan’s test case as well as the core and
scsh-process test suites (and probably most other cases), the call to
fork() happens while the FILE* buffer is in use. So at exit() time,
the file descriptor associated with the source file will get lseek-ed
to match the position in the FILE* buffer.

This happens for all file descriptors associated to stdio FILE*
streams.

The obvious fix for this is to call fflush(NULL) before we fork. This
call should synchronize all FILE* streams with their file descriptors.

Unfortunately, POSIX only documents the behaviour of fflush(NULL) for
*output and update streams*. Input streams stay untouched. See here:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/fflush.html


From this, I only see a handful ways of working around the bug:

- Calling _exit() instead of exit() in the default exit-handler. This
  would prevent the streams from synchronizing with their FDs but
  would completely throw away functions registered with atexit() or
  on_exit()

- Calling fflush() on every input file owned by CHICKEN before calling
  fork(). This would require a way to get all FILE* streams opened by
  CHICKEN, I don’t know if we have that. It would not cause the
  atexit() problem mentioned earlier, but it wouldn’t be completely
  general, as we wouldn’t be able to call fflush() on streams not
  owned by CHICKEN.

- Getting rid of every stdio use in core and roll our own buffered i/o
  system. This is quite involved but may also be a push in the right
  direction for the ports system redesign of CHICKEN 5.

Anyhow, we should at least add a call to fflush(NULL) in the
process-fork procedure, as it takes care of the output streams.

Some musl people will get it touch with the POSIX work group to see if
there is a way to fix this.

I hope this explains clearly what happens. If you find another way to
fix this bug, please go ahead and tell us!

___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] Fix attempt for bug #1269 "Port or reader state corruption"

[Peter Bex]

On Sat, Mar 19, 2016 at 01:51:01PM +0100, Kooda wrote:
> I may have found a fix for the reader bug that occured recently.
> http://bugs.call-cc.org/ticket/1269
> 
> It seems to be caused by an implementation-dependent behaviour of the
> exit(3) procedure.

Hi Kooda,

Can you elaborate some more on this?  As I understand it, exit() will
flush standard I/O buffers and call any atexit() handlers, whereas
_exit() will skip all those things.

So if I'm not mistaken, in the absence of any atexit() handlers, your
code should perform exactly the same actions as exit() should.

I thought a better fix would be to call fflush _before_ forking.  That
way, the child will always start with empty buffers and we'll avoid
flushing the same data buffers twice (preventing the bug from being
triggered), and we still allow any atexit() handlers to be invoked,
and we won't override a potentially custom user-supplied exit-handler.

Unfortunately, that fix didn't work, so maybe I'm misunderstanding how
and why the current version is breaking.

In any case, I would be hesitant to just hard override the user's
##sys#exit-handler.

Cheers,
Peter


signature.asc
Description: Digital signature
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

[Chicken-hackers] Fix attempt for bug #1269 "Port or reader state corruption"

[Kooda]

I may have found a fix for the reader bug that occured recently.
http://bugs.call-cc.org/ticket/1269

It seems to be caused by an implementation-dependent behaviour of the
exit(3) procedure.

The bug occurs reliably on musl 1.1.12 and glibc 2.23

Here is a patch that make `make check` pass, as well as the
scsh-process’ tests for both incriminated libc


>From 5e4195073e46794e0127435bad53a0c4947e7b91 Mon Sep 17 00:00:00 2001
From: Kooda 
Date: Sat, 19 Mar 2016 13:21:43 +0100
Subject: [PATCH] Fix bug #1269

---
 posixunix.scm | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/posixunix.scm b/posixunix.scm
index f56960d..bae028d 100644
--- a/posixunix.scm
+++ b/posixunix.scm
@@ -1576,6 +1576,12 @@ EOF
   (let ((pid (fork)))
(when (fx= -1 pid) 
  (posix-error #:process-error 'process-fork "cannot create child 
process"))
+   ;; exit the child process gracefully when (exit) is called
+   (##sys#exit-handler
+(lambda (#!optional (code 0))
+  ((foreign-lambda int "fflush" c-pointer) #f) ;; flush all output 
buffers
+  (##sys#cleanup-before-exit)
+  ((foreign-lambda void "_exit" int) code)))
(if (and thunk (zero? pid))
((if killothers
 ##sys#kill-other-threads
-- 
2.1.4

___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] Fix #1227 and simplify "parameterize" macro a bit

[Evan Hanson]

Thanks Peter and Joo ChurlSoo, subtle stuff. This is certainly more
robust, and the new expansion is still sane enough that I think the
change is worth it. The second patch certainly helps. I've pushed both.

Evan


signature.asc
Description: PGP signature
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] Fix #1227 and simplify "parameterize" macro a bit

[Peter Bex]

On Sat, Jan 23, 2016 at 05:35:54PM +0100, Peter Bex wrote:
> The fix is as I mentioned in the ticket: We change the semantics of
> parameter object procedures a bit.  Instead of accepting a "hidden"
> extra argument which indicates the guard procedure should be skipped,
> we add another argument which indicates that the parameter should not
> be set.

About this: I didn't do it as in the ticket for compatibility reasons:
the compiler and runtime library itself uses "parameterize", and if we
change the way parameter procedures work, that will result in a compiler
that won't work against the new library.

This is unfortunate, because the current approach makes user errors more
likely: if you accidentally pass more than one argument to a parameter
object's procedure, it will do the wrong thing.  I don't really know how
we can change this without causing bootstrap breakage, though.  Ideas on
how to do this are welcome!

Cheers,
Peter


signature.asc
Description: Digital signature
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

[Chicken-hackers] Fix #1227 and simplify "parameterize" macro a bit

[Peter Bex]

Hi hackers,

Attached is a fix for #1227: when (parameterize) is used with parameters
that have a "guard" procedure associated with them, and one of the guards
raises an exception, the parameters that had already been converted by
their respective guards would not get reset properly.

The fix is as I mentioned in the ticket: We change the semantics of
parameter object procedures a bit.  Instead of accepting a "hidden"
extra argument which indicates the guard procedure should be skipped,
we add another argument which indicates that the parameter should not
be set.

This allows us to call just the guards, one by one, and use "let" to
bind these to temporaries.  After having called all the guards, we
then the parameter object procedures again, this time bypassing the
guards and asking them to set the values to the temporaries.

In between, we ask the parameters for their current value, so that we
can remember them.  This can then be restored when leaving the dynamic
extent of the parameterized body, and flipped back again when entering,
at which time we skip application of the guards.

I didn't expect this, but it actually fixes the "ordering bug" as well,
because we simply set the parameters to their original values we saved
earlier!

This code is a bit tricky; I found it difficult to work out in my head
how "swap!" works, so I decided to simplify the code by using different
procedures for dynamic-wind's "pre" and "post" arguments.  The "post"
one is simple, and it becomes clearer how the "mode" stuff works by
pulling it apart like this.  Even though the two procedures differ,
the code is actually less LOCs than the original because we can drop a
renamed procedure name and the code goes a bit to the left, which allows
us to put some things on a single line that arguably belong on one line.

This simplification is in the second patch.  It is purely optional: the
first patch completely fixes the bug.  I'll leave it to the reviewer to
decide whether the second patch really does simplify things.

These patches apply to both master and chicken-5.

Cheers,
Peter
From 55dfec9461b38f6d17257f8d8e31383092cb50dc Mon Sep 17 00:00:00 2001
From: Peter Bex 
Date: Sat, 23 Jan 2016 15:10:26 +0100
Subject: [PATCH 1/2] Fix nonlocal exit handling in parameterize.

When a parameter's "guard" procedure raises an exception, the values
should be consistent.  Before, the values were assigned one by one, and
if the exception got raised halfway through, the already-assigned
parameters would not be restored to their original values.

Add a clarifying comment as to why we must alias all the parameters:
they don't need to be identifiers but can be complex expressions
evaluating to parameters.  This is not immediately obvious from the
code.  It's only clear when you remember that parameterize can be used
like this when reading the code.
---
 NEWS|  3 +++
 chicken-syntax.scm  | 33 +-
 library.scm | 62 ++---
 tests/library-tests.scm | 21 +
 4 files changed, 79 insertions(+), 40 deletions(-)

diff --git a/NEWS b/NEWS
index 786c107..5fdfd6e 100644
--- a/NEWS
+++ b/NEWS
@@ -27,6 +27,9 @@
 - Core libraries
- SRFI-18: thread-join! no longer gives an error when passed a
  thread in the "sleeping" state (thanks to Joerg Wittenberger)
+   - SRFI-39: When a parameter's "guard" procedure raises an exception,
+ "parameterize" now correctly resets the original values of all
+ parameters (fixes #1227, thanks to Joo ChurlSoo).
- Irregex has been updated to 0.9.4, which fixes severe performance
  problems with {n,m} repeating patterns (thanks to Caolan McMahon).
- Unit "posix": The following posix procedures now work on port
diff --git a/chicken-syntax.scm b/chicken-syntax.scm
index 11ecda2..f3ee9c8 100644
--- a/chicken-syntax.scm
+++ b/chicken-syntax.scm
@@ -280,25 +280,36 @@
 (let* ((bindings (cadr form))
 	   (body (cddr form))
 	   (swap (r 'swap))
-	   (mode (r 'mode))
+	   (convert? (r 'convert?))
 	   (params (##sys#map car bindings))
 	   (vals (##sys#map cadr bindings))
-	   (aliases (##sys#map (lambda (z) (r (pname z))) params))
-	   (aliases2 (##sys#map (lambda (z) (r (gensym))) params)) )
+	   (param-aliases (##sys#map (lambda (z) (r (pname z))) params))
+	   (saveds (##sys#map (lambda (z) (r (gensym 'saved))) params))
+	   (temps (##sys#map (lambda (z) (r (gensym 'tmp))) params)) )
   `(##core#let
-	,(map ##sys#list aliases params)
+	,(map ##sys#list param-aliases params) ; These may be expressions
 	(##core#let
-	 ,(map ##sys#list aliases2 vals)
+	 ,(map ##sys#list saveds vals)
 	 (##core#let
-	  ((,mode #f))
+	  ((,convert? #t))
 	  (##core#let
 	   ((,swap (##core#lambda
 		()
-		,@(map (lambda (a a2)
-			 `(##core#let ((t (,a))) (,a ,a2 ,mode)
-	  (##core#set! ,a2 t)))
-			   aliases aliases2)
-		(##core#set! ,mode #t
+		(##core#let
+		

Re: [Chicken-hackers] Fix solaris9 build for missing trunc, round, isinf math functions

[Jim Ursetto]

On Apr 20, 2013, at 4:45 PM, Felix wrote:

 From: Christian Kellermann ck...@pestilenz.org

 No, published patches are better not changed as they would require
 either a merge by everyone that already has the old patch, or you
 would force everyone to throw away that patch and take the other.
 
 I think git commit --amend --signoff should work.

Only if you haven't pushed already (which turned out to be the case here).

Jim

___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] Fix solaris9 build for missing trunc, round, isinf math functions

[Christian Kellermann]

* felix winkelmann fe...@call-with-current-continuation.org [130420 16:56]:
 From: Michele La Monaca mikele.chic...@lamonaca.net
 Subject: [Chicken-hackers] Fix solaris9 build for missing trunc, round, isinf 
 math functions
 Date: Wed, 17 Apr 2013 23:48:19 +0200
 
  This patch fixes Solaris 9 compilation (suncc) which fails due to
  missing C99 math functions.
 
 Thanks - signed off and pushed.
 
 (I have pushed before realizing that this needs another signoff. 
 On the other hand, this patch is straightforward, so to follow
 THE RULES,  perhaps someone can do another signoff?)

No, published patches are better not changed as they would require
either a merge by everyone that already has the old patch, or you
would force everyone to throw away that patch and take the other.

That's the tooling folks.

And your pushing the patch is OK, Michele is the only person we
know that can possibly test this, so who else should sign it off?

Cheers,

Christian

-- 
In the world, there is nothing more submissive and weak than
water. Yet for attacking that which is hard and strong, nothing can
surpass it. --- Lao Tzu

___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] Fix solaris9 build for missing trunc, round, isinf math functions

[Christian Kellermann]

* felix winkelmann fe...@call-with-current-continuation.org [130420 16:56]:
 From: Michele La Monaca mikele.chic...@lamonaca.net
 Subject: [Chicken-hackers] Fix solaris9 build for missing trunc, round, isinf 
 math functions
 Date: Wed, 17 Apr 2013 23:48:19 +0200
 
  This patch fixes Solaris 9 compilation (suncc) which fails due to
  missing C99 math functions.
 
 Thanks - signed off and pushed.

On a second thought, neither Peter nor I can find this in the usual
place. Did you really push it?

Sorry for the lecturing...

Christian

-- 
In the world, there is nothing more submissive and weak than
water. Yet for attacking that which is hard and strong, nothing can
surpass it. --- Lao Tzu

___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] Fix solaris9 build for missing trunc, round, isinf math functions

[Michele La Monaca]

I've spotted an error in the patch.

static inline double trunc(double x) { return (x  0 ? floor(x) :
floor(x) + 1); }

should be:

static inline double trunc(double x) { return (x = 0 ? floor(x) :
floor(x) + 1); }

please correct it before pushing.

Thanks!

Michele


On Sat, Apr 20, 2013 at 5:12 PM, Christian Kellermann
ck...@pestilenz.org wrote:
 * felix winkelmann fe...@call-with-current-continuation.org [130420 16:56]:
 From: Michele La Monaca mikele.chic...@lamonaca.net
 Subject: [Chicken-hackers] Fix solaris9 build for missing trunc, round, 
 isinf math functions
 Date: Wed, 17 Apr 2013 23:48:19 +0200

  This patch fixes Solaris 9 compilation (suncc) which fails due to
  missing C99 math functions.

 Thanks - signed off and pushed.

 On a second thought, neither Peter nor I can find this in the usual
 place. Did you really push it?

 Sorry for the lecturing...

 Christian

 --
 In the world, there is nothing more submissive and weak than
 water. Yet for attacking that which is hard and strong, nothing can
 surpass it. --- Lao Tzu

___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] Fix solaris9 build for missing trunc, round, isinf math functions

[Felix]

From: Michele La Monaca mikele.chic...@lamonaca.net
Subject: Re: [Chicken-hackers] Fix solaris9 build for missing trunc, round, 
isinf math functions
Date: Sat, 20 Apr 2013 17:18:07 +0200

 I've spotted an error in the patch.
 
 static inline double trunc(double x) { return (x  0 ? floor(x) :
 floor(x) + 1); }
 
 should be:
 
 static inline double trunc(double x) { return (x = 0 ? floor(x) :
 floor(x) + 1); }
 
 please correct it before pushing.

Attached is the patch with the applied fix. I pushed from the wrong
branch, therefore it didn't show up in master (I'm still too thick to
use git correctly...).

Contrary to most others on this list I think it is generally a good
idea to support different compilers, if the necessary changes are not
too involved (as compared to, say, MSVC). 


cheers,
felix
From 3ea4c865da7b07f548cdd9208bc596bca7e99e9a Mon Sep 17 00:00:00 2001
From: Michele La Monaca mikele.chic...@lamonaca.net
Date: Wed, 17 Apr 2013 23:21:14 +0200
Subject: [PATCH] fixed solaris9 build for missing trunc,round,isinf

With fix added as reported by Michele La Monaca (who also provided the original patch).

Signed-off-by: felix fe...@call-with-current-continuation.org
---
 chicken.h |5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/chicken.h b/chicken.h
index 2b9030a..0e6b735 100644
--- a/chicken.h
+++ b/chicken.h
@@ -89,7 +89,7 @@
 # define C_NONUNIX
 #endif
 
-#if defined(__sun__)  defined(__svr4__)
+#if defined(__sun__)  defined(__svr4__) || defined(__SUNPRO_C)
 # define C_SOLARIS
 #endif
 
@@ -976,6 +976,9 @@ DECL_C_PROC_p0 (128,  1,0,0,0,0,0,0,0)
 # ifdef __linux__
 extern double round(double);
 extern double trunc(double);
+# elif defined (__SunOS_5_9)
+static inline double trunc(double x) { return (x = 0 ? floor(x) : floor(x) + 1); }
+static inline double round(double x) { return (x - floor(x) = 0.5 ? floor(x) + 1 : floor(x)); }
 # endif
 #else
 /* provide this file and define C_PROVIDE_LIBC_STUBS if you want to use
-- 
1.7.9.5

___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

[Chicken-hackers] Fix solaris9 build for missing trunc, round, isinf math functions

[Michele La Monaca]

This patch fixes Solaris 9 compilation (suncc) which fails due to
missing C99 math functions.

Regards,
Michele


0001-fixed-solaris9-build-for-missing-trunc-round-isinf.patch
Description: Binary data
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] fix for blob read syntax manual entry

[Peter Bex]

On Fri, Apr 27, 2012 at 02:37:29PM -0700, Kon Lovett wrote:
 Hi,
 
 This was just bugging me.

Thanks, pushed.

-- 
http://sjamaan.ath.cx
--
The process of preparing programs for a digital computer
 is especially attractive, not only because it can be economically
 and scientifically rewarding, but also because it can be an aesthetic
 experience much like composing poetry or music.
-- Donald Knuth

___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

[Chicken-hackers] fix for blob read syntax manual entry

[Kon Lovett]

Hi,

This was just bugging me.



fix_blob_read_syntax.patch
Description: Binary data
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers