GNU bug report logs - #13827
faulty range check in bytevector accessor

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 13827 in the body.
You can then email your comments to 13827 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ian Price <ianprice90 <at> googlemail.com>
To: bug-guile <at> gnu.org
Subject: faulty range check in bytevector accessor
Date: Wed, 27 Feb 2013 02:02:06 +0000

Branch: master
Commit: 9b977c836bf147d386944c401113aba32776fa68
System: 32 bit x86 Fedora 16

(use-modules (rnrs bytevectors))
(define not-32-bit (expt 2 32))
(define bv (make-bytevector 4))
(bytevector-u32-set! bv 0 not-32-bit (endianness big))
(pk bv)

Running this gives me a core dump. It happens for a wide range of values
that don't fit in 32 bits.

After some talk on #guile, Mark and I believe it comes down to the range
check in INTEGER_ACCESSOR_PROLOGUE in bytevectors.c

-- 
Ian Price -- shift-reset.com

"Programming is like pinball. The reward for doing it well is
the opportunity to do it again" - from "The Wizardy Compiled"

Message #8 received at 13827 <at> debbugs.gnu.org (full text, mbox):

From: Mark H Weaver <mhw <at> netris.org>
To: Ian Price <ianprice90 <at> googlemail.com>
Cc: 13827 <at> debbugs.gnu.org
Subject: Re: bug#13827: faulty range check in bytevector accessor
Date: Tue, 26 Feb 2013 21:30:15 -0500

Ian Price <ianprice90 <at> googlemail.com> writes:
> After some talk on #guile, Mark and I believe it comes down to the range
> check in INTEGER_ACCESSOR_PROLOGUE in bytevectors.c

Going a bit further: INTEGER_ACCESSOR_PROLOGUE uses 'scm_to_uint', which
I believe should fail for 2^32 on a 32-bit machine.  According to
numbers.h:430, 'scm_to_uint' should be an alias for 'scm_to_uint32',
which is defined in numbers.c:9277 and conv-uinteger.i.c:27.

It seems to me that it ought to be getting to conv-uinteger.i.c:50,
which calls 'mpz_fits_ulong_p'.  So maybe it's a bug in the version of
libgmp on Ian's machine, or perhaps I'm missing something.

I don't know whether it's possible to step through the code in
'conv-uinteger.i.c' using gdb.  If so, I'd like to see what happens.  If
not, I suspect the next step is to write some test programs in C and try
them on Ian's machine: first test 'scm_to_uint32', which should raise an
exception for 2^32.  If it doesn't then try testing 'mpz_fits_ulong_p'
directly and see if it's broken.

    Thanks,
      Mark

Message #11 received at 13827 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Ian Price <ianprice90 <at> googlemail.com>
Cc: 13827 <at> debbugs.gnu.org
Subject: Re: bug#13827: faulty range check in bytevector accessor
Date: Wed, 27 Feb 2013 12:42:50 +0100

Ian Price <ianprice90 <at> googlemail.com> skribis:

> Branch: master
> Commit: 9b977c836bf147d386944c401113aba32776fa68
> System: 32 bit x86 Fedora 16
>
> (use-modules (rnrs bytevectors))
> (define not-32-bit (expt 2 32))
> (define bv (make-bytevector 4))
> (bytevector-u32-set! bv 0 not-32-bit (endianness big))
> (pk bv)

FWIW, with 2.0.7+ on x86_64-linux-gnu, with GMP 5.1.0:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> (use-modules (rnrs bytevectors))
scheme@(guile-user)> (define not-32-bit (expt 2 32))
scheme@(guile-user)> (define bv (make-bytevector 4))
scheme@(guile-user)> (bytevector-u32-set! bv 0 not-32-bit (endianness big))
<unnamed port>:4:0: In procedure #<procedure 1b1bf00 at <current input>:4:0 ()>:
<unnamed port>:4:0: In procedure bytevector-u32-set!: Value out of range: 4294967296

Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
--8<---------------cut here---------------end--------------->8---

As Mark pointed out, it could be due to the GMP version you’re using.

Could you check that?

TIA,
Ludo’.

Message #14 received at 13827 <at> debbugs.gnu.org (full text, mbox):

From: Mark H Weaver <mhw <at> netris.org>
To: ludo <at> gnu.org (Ludovic Courtès)
Cc: Ian Price <ianprice90 <at> googlemail.com>, 13827 <at> debbugs.gnu.org
Subject: Re: bug#13827: faulty range check in bytevector accessor
Date: Wed, 27 Feb 2013 20:38:17 -0500

ludo <at> gnu.org (Ludovic Courtès) writes:

> Ian Price <ianprice90 <at> googlemail.com> skribis:
>
>> Branch: master
>> Commit: 9b977c836bf147d386944c401113aba32776fa68
>> System: 32 bit x86 Fedora 16
>>
>> (use-modules (rnrs bytevectors))
>> (define not-32-bit (expt 2 32))
>> (define bv (make-bytevector 4))
>> (bytevector-u32-set! bv 0 not-32-bit (endianness big))
>> (pk bv)
>
> FWIW, with 2.0.7+ on x86_64-linux-gnu, with GMP 5.1.0:
>
> scheme@(guile-user)> (use-modules (rnrs bytevectors))
> scheme@(guile-user)> (define not-32-bit (expt 2 32))
> scheme@(guile-user)> (define bv (make-bytevector 4))
> scheme@(guile-user)> (bytevector-u32-set! bv 0 not-32-bit (endianness big))
> <unnamed port>:4:0: In procedure #<procedure 1b1bf00 at <current input>:4:0 ()>:
> <unnamed port>:4:0: In procedure bytevector-u32-set!: Value out of range: 4294967296

That's because you're on a 64-bit system, where 2^32 is no difficulty.

     Mark

Message #17 received at 13827 <at> debbugs.gnu.org (full text, mbox):

From: Ian Price <ianprice90 <at> googlemail.com>
To: ludo <at> gnu.org (Ludovic Courtès)
Cc: 13827 <at> debbugs.gnu.org
Subject: Re: bug#13827: faulty range check in bytevector accessor
Date: Thu, 28 Feb 2013 20:20:07 +0000

ludo <at> gnu.org (Ludovic Court$(D+2(Bs) writes:

> As Mark pointed out, it could be due to the GMP version you$B!G(Bre using.
>
> Could you check that?

Well, I wouldn't rule it out until I've had a proper look. I believe I
am running 4.3.2, but do you have a specific test in mind?

I'm going away for the weekend, though, so I won't really be able to
debug this properly till Monday.

-- 
Ian Price -- shift-reset.com

"Programming is like pinball. The reward for doing it well is
the opportunity to do it again" - from "The Wizardy Compiled"

Message #20 received at 13827 <at> debbugs.gnu.org (full text, mbox):

From: Andy Wingo <wingo <at> pobox.com>
To: Ian Price <ianprice90 <at> googlemail.com>
Cc: 13827 <at> debbugs.gnu.org
Subject: Re: bug#13827: faulty range check in bytevector accessor
Date: Wed, 13 Mar 2013 13:55:14 +0100

On Wed 27 Feb 2013 03:02, Ian Price <ianprice90 <at> googlemail.com> writes:

> Branch: master
> Commit: 9b977c836bf147d386944c401113aba32776fa68
> System: 32 bit x86 Fedora 16
>
> (use-modules (rnrs bytevectors))
> (define not-32-bit (expt 2 32))
> (define bv (make-bytevector 4))
> (bytevector-u32-set! bv 0 not-32-bit (endianness big))
> (pk bv)
>
> Running this gives me a core dump. It happens for a wide range of values
> that don't fit in 32 bits.
>
> After some talk on #guile, Mark and I believe it comes down to the range
> check in INTEGER_ACCESSOR_PROLOGUE in bytevectors.c

Something like this look right to you?

--- a/libguile/bytevectors.c
+++ b/libguile/bytevectors.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 2009, 2010, 2011, 2012 Free Software Foundation, Inc.
+/* Copyright (C) 2009, 2010, 2011, 2012, 2013 Free Software Foundation, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public License
@@ -82,12 +82,12 @@
   _sign char *c_bv;						\
 								\
   SCM_VALIDATE_BYTEVECTOR (1, bv);				\
-  c_index = scm_to_uint (index);				\
+  c_index = scm_to_size_t (index);				\
 								\
   c_len = SCM_BYTEVECTOR_LENGTH (bv);				\
   c_bv = (_sign char *) SCM_BYTEVECTOR_CONTENTS (bv);		\
 								\
-  if (SCM_UNLIKELY (c_index + ((_len) >> 3UL) - 1 >= c_len))	\
+  if (SCM_UNLIKELY (c_index >= c_len))                          \
     scm_out_of_range (FUNC_NAME, index);
 
 /* Template for fixed-size integer access (only 8, 16 or 32-bit).  */



-- 
http://wingolog.org/

Message #23 received at 13827 <at> debbugs.gnu.org (full text, mbox):

From: Andy Wingo <wingo <at> pobox.com>
To: Ian Price <ianprice90 <at> googlemail.com>
Cc: 13827 <at> debbugs.gnu.org
Subject: Re: bug#13827: faulty range check in bytevector accessor
Date: Wed, 13 Mar 2013 15:37:56 +0100

On Wed 13 Mar 2013 13:55, Andy Wingo <wingo <at> pobox.com> writes:

> -  if (SCM_UNLIKELY (c_index + ((_len) >> 3UL) - 1 >= c_len))	\
> +  if (SCM_UNLIKELY (c_index >= c_len))                          \
>      scm_out_of_range (FUNC_NAME, index);

I see the intention was to take into account the size of the access
(e.g. 32 bits).  Confusing with len, _len, and c_len...

-- 
http://wingolog.org/

Message #28 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Ben Rocer" <fleabyte <at> mail.com>
To: bug-guile <at> gnu.org
Subject: bug#13827: faulty range check in bytevector accessor
Date: Mon, 28 Jul 2014 16:35:15 +0200

[resubmitting to bug-guile <at> gnu.org as debbugs seems to have eaten my
 first mail]

When I tried to reproduce this bug on a 32-bit x86 system, I got an
abort in the function bytevector_large_set(); I think this is also
where the bug is.

Specifically, there are two bugs in these two consecutive lines in
bytevector_large_set():

value_size = (mpz_sizeinbase (c_mpz, 2) + (8 * c_size)) / (8 * c_size);
if (SCM_UNLIKELY (value_size > c_size))

In the first line, there is an off-by-one error in the calculation of
value_size; it gives the wrong answer if mpz_sizeinbase() is a
multiple of (8 * c_size) (see
https://gmplib.org/manual/Integer-Import-and-Export.html).

Secondly, this calculation gives the number of (c_size-byte) *words*
required to hold c_mpz, not the number of bytes. So the check in the
next line should be (c_size * value_size > c_size), or equivalently
(value_size > 1).

Since bytevector-u64-set! also calls bytevector_large_set, it
may be possible to reproduce this bug on 64 bit systems too; e.g
(bytevector-u64-set! (make-bytevector 8) 0 (expt 2 64) (endianness big))
[untested]


--- a/libguile/bytevectors.c
+++ b/libguile/bytevectors.c
@@ -867,10 +867,10 @@ bytevector_large_set (char *c_bv, size_t c_size, int signed_p,
     memset (c_bv, 0, c_size);
   else
     {
-      size_t word_count, value_size;
+      size_t word_count, value_words;
 
-      value_size = (mpz_sizeinbase (c_mpz, 2) + (8 * c_size)) / (8 * c_size);
-      if (SCM_UNLIKELY (value_size > c_size))
+      value_words = (mpz_sizeinbase (c_mpz, 2) + (8 * c_size) - 1) / (8 * c_size);
+      if (SCM_UNLIKELY (value_words > 1))
 	{
 	  err = -2;
 	  goto finish;

Message #33 received at 13827-done <at> debbugs.gnu.org (full text, mbox):

From: Andy Wingo <wingo <at> pobox.com>
To: "Ben Rocer" <fleabyte <at> mail.com>
Cc: 13827-done <at> debbugs.gnu.org
Subject: Re: bug#13827: faulty range check in bytevector accessor
Date: Mon, 20 Jun 2016 17:16:05 +0200

Hi!

Thank you very much for the bug report and fix!  Applied to master, will
be part of 2.1.4.

Cheers,

Andy

On Mon 28 Jul 2014 16:35, "Ben Rocer" <fleabyte <at> mail.com> writes:

> [resubmitting to bug-guile <at> gnu.org as debbugs seems to have eaten my
>  first mail]
>
> When I tried to reproduce this bug on a 32-bit x86 system, I got an
> abort in the function bytevector_large_set(); I think this is also
> where the bug is.
>
> Specifically, there are two bugs in these two consecutive lines in
> bytevector_large_set():
>
> value_size = (mpz_sizeinbase (c_mpz, 2) + (8 * c_size)) / (8 * c_size);
> if (SCM_UNLIKELY (value_size > c_size))
>
> In the first line, there is an off-by-one error in the calculation of
> value_size; it gives the wrong answer if mpz_sizeinbase() is a
> multiple of (8 * c_size) (see
> https://gmplib.org/manual/Integer-Import-and-Export.html).
>
> Secondly, this calculation gives the number of (c_size-byte) *words*
> required to hold c_mpz, not the number of bytes. So the check in the
> next line should be (c_size * value_size > c_size), or equivalently
> (value_size > 1).
>
> Since bytevector-u64-set! also calls bytevector_large_set, it
> may be possible to reproduce this bug on 64 bit systems too; e.g
> (bytevector-u64-set! (make-bytevector 8) 0 (expt 2 64) (endianness big))
> [untested]
>
>
> --- a/libguile/bytevectors.c
> +++ b/libguile/bytevectors.c
> @@ -867,10 +867,10 @@ bytevector_large_set (char *c_bv, size_t c_size, int signed_p,
>      memset (c_bv, 0, c_size);
>    else
>      {
> -      size_t word_count, value_size;
> +      size_t word_count, value_words;
>  
> -      value_size = (mpz_sizeinbase (c_mpz, 2) + (8 * c_size)) / (8 * c_size);
> -      if (SCM_UNLIKELY (value_size > c_size))
> +      value_words = (mpz_sizeinbase (c_mpz, 2) + (8 * c_size) - 1) / (8 * c_size);
> +      if (SCM_UNLIKELY (value_words > 1))
>  	{
>  	  err = -2;
>  	  goto finish;

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.