Re: Segmentation fault with mpz_inp_raw on gcc45
Ciao, Il 2021-09-18 10:07 Torbjörn Granlund ha scritto: I suppose that, in this case, as my fix has problems with nails, this Your fix also aborts, instead of returning 0. might be a better patch. An UNLIKELY(...) might have a place here, though. I commited the patch, adding UNLIKELY to all error branches. I took a very quick lock at other uses of BITS_TO_LIMBS and think we might have one more place where scalar overflow might occur: mpz/import.c. There might be more places. I also changed mpz/import.c but I did not correct this possible overflow. Ĝis, m ___ gmp-bugs mailing list gmp-bugs@gmplib.org https://gmplib.org/mailman/listinfo/gmp-bugs
Re: Segmentation fault with mpz_inp_raw on gcc45
Ciao,
Il 2021-09-16 23:27 Torbjörn Granlund ha scritto:
Going from byte count to bit count to limb count is non-trivial without
risking overflow, even if the end result will typically be smaller than
the incoming byte count (assuming we're not using gmp/asl.h with tiny
Yes, but, should we really consider it only a temporary overflow?
Does GMP support mpz numbers with a bit-lenght that overflows?
Perhaps, instead of the change below which triggers an errir in the
allocation statement later in the program flow, we could make an
explicit check of the byte count vs the max supported sizes, and
generate the overflow error locally.
I agree!
We do not define BITCNT_MAX anywhere, do we?
Then, maybe something like the following could do?
--- a/mpz/inp_raw.c Tue Sep 14 19:05:51 2021 +0200
+++ b/mpz/inp_raw.c Fri Sep 17 18:49:14 2021 +0200
@@ -87,9 +87,11 @@
csize = size - 0x8000u - 0x8000u;
abs_csize = ABS (csize);
+ if (abs_csize > ~(mp_bitcnt_t) 0 / 8)
+return 0; /* Bit size overflows */
/* round up to a multiple of limbs */
- abs_xsize = BITS_TO_LIMBS (abs_csize*8);
+ abs_xsize = BITS_TO_LIMBS ((mp_bitcnt_t) abs_csize * 8);
if (abs_xsize != 0)
{
Ĝis,
m
___
gmp-bugs mailing list
gmp-bugs@gmplib.org
https://gmplib.org/mailman/listinfo/gmp-bugs
Re: Segmentation fault with mpz_inp_raw on gcc45
Hi Torbjörn, I confirm your fix works on gcc45: zimmerma@gcc45:~/ecm$ gcc -I$HOME/include test.c $HOME/lib/libgmp.a zimmerma@gcc45:~/ecm$ ./a.out XXX 2d65200d 0b594804 gmp: overflow in mpz type Aborted Thanks, Paul ___ gmp-bugs mailing list gmp-bugs@gmplib.org https://gmplib.org/mailman/listinfo/gmp-bugs
Re: Segmentation fault with mpz_inp_raw on gcc45
Vincent Lefevre writes:
In mpz/inp_raw.c, I think that abs_csize*8 yields an integer overflow
on large sizes.
Indeed.
Going from byte count to bit count to limb count is non-trivial without
risking overflow, even if the end result will typically be smaller than
the incoming byte count (assuming we're not using gmp/asl.h with tiny
limbs...).
Below is a fix which works unless GMP_NUMB_BITS mod 8 != 0. I think we
have this problem in more places in the library. I spotted
mpz/import.c, but there might be more.
Perhaps, instead of the change below which triggers an errir in the
allocation statement later in the program flow, we could make an
explicit check of the byte count vs the max supported sizes, and
generate the overflow error locally.
*** /tmp/extdiff.b8_0au15/gmp.09a7d62c4438/mpz/inp_raw.cTue Sep 14
19:05:51 2021
--- /home/tege/prec/gmp/mpz/inp_raw.c Thu Sep 16 22:35:46 2021
***
*** 90,94
/* round up to a multiple of limbs */
! abs_xsize = BITS_TO_LIMBS (abs_csize*8);
if (abs_xsize != 0)
--- 90,99
/* round up to a multiple of limbs */
! if (GMP_NUMB_BITS % 8 != 0)
! abs_xsize = BITS_TO_LIMBS (abs_csize*8); /* FIXME: may overflow */
! else
! abs_xsize = (abs_csize + (GMP_NUMB_BITS / 8) - 1) / (GMP_NUMB_BITS / 8);
!
! printf ("XXX %08lx %08lx\n", csize, abs_xsize);
if (abs_xsize != 0)
--
Torbjörn
Please encrypt, key id 0xC8601622
___
gmp-bugs mailing list
gmp-bugs@gmplib.org
https://gmplib.org/mailman/listinfo/gmp-bugs
Re: Segmentation fault with mpz_inp_raw on gcc45
On 2021-09-15 17:34:01 +0200, Vincent Lefevre wrote:
> On 2021-09-15 17:05:42 +0200, Paul Zimmermann wrote:
> > sorry the test_dummy2.save is attached. It was generated by (under /bin/sh,
> > not /bin/bash):
> >
> > echo -e "\n\r\n\r# this is a comment line and should be ignored" >
> > test_dummy2.save
>
> I can reproduce the segfault only with a 32-bit ABI.
>
> read(3, "-e \n\r\n\r# this is a comment line "..., 4096) = 54
> mmap2(NULL, 224735232, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1,
> 0) = 0xea604000
> --- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_MAPERR, si_addr=0xca604012} ---
>
> If I understand correctly, the read system call comes from the initial
> fread() to get the size, and the mmap2 comes from the allocation.
In mpz/inp_raw.c, I think that abs_csize*8 yields an integer overflow
on large sizes.
--
Vincent Lefèvre - Web:
Re: Segmentation fault with mpz_inp_raw on gcc45
On 2021-09-15 17:05:42 +0200, Paul Zimmermann wrote:
> sorry the test_dummy2.save is attached. It was generated by (under /bin/sh,
> not /bin/bash):
>
> echo -e "\n\r\n\r# this is a comment line and should be ignored" >
> test_dummy2.save
I can reproduce the segfault only with a 32-bit ABI.
read(3, "-e \n\r\n\r# this is a comment line "..., 4096) = 54
mmap2(NULL, 224735232, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0)
= 0xea604000
--- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_MAPERR, si_addr=0xca604012} ---
If I understand correctly, the read system call comes from the initial
fread() to get the size, and the mmap2 comes from the allocation.
--
Vincent Lefèvre - Web:
Re: Segmentation fault with mpz_inp_raw on gcc45
sorry the test_dummy2.save is attached. It was generated by (under /bin/sh, not /bin/bash): echo -e "\n\r\n\r# this is a comment line and should be ignored" > test_dummy2.save Paul test_dummy2.save Description: Binary data ___ gmp-bugs mailing list gmp-bugs@gmplib.org https://gmplib.org/mailman/listinfo/gmp-bugs
Re: Segmentation fault with mpz_inp_raw on gcc45
On 2021-09-15 16:13:55 +0200, Torbjorn Granlund wrote:
> I tried to reproduce this on several systems. I failed; mpz_inp_raw
> returns 0 for me as it should.
>
> I cannot be sure I got test_dummy2.save right. I did include the line
> that asked to be ignored. Should I have ignored it? I added LF at each
> appparent line end, including the ignore line.
You may need a bigger file, as shown by strace output:
openat(AT_FDCWD, "test_dummy2.save", O_RDONLY) = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=51, ...}) = 0
read(3, "-e\n\n# this is a comment line and"..., 4096) = 51
mmap(NULL, 761597952, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0x7f267cd4a000
read(3, "", 761593856) = 0
i.e. if there is any issue in GMP, the test_dummy2.save file is too
small as nothing is read from read(3, "", 761593856).
Paul should give more details.
--
Vincent Lefèvre - Web:
Re: Segmentation fault with mpz_inp_raw on gcc45
I tried to reproduce this on several systems. I failed; mpz_inp_raw returns 0 for me as it should. I cannot be sure I got test_dummy2.save right. I did include the line that asked to be ignored. Should I have ignored it? I added LF at each appparent line end, including the ignore line. -- Torbjörn Please encrypt, key id 0xC8601622 ___ gmp-bugs mailing list gmp-bugs@gmplib.org https://gmplib.org/mailman/listinfo/gmp-bugs
Re: Segmentation fault with mpz_inp_raw on gcc45
> OK, so you deliberately sen d junk to mpz_inp_raw. That is fine, but it > was not clear from your report. it was not completely deliberate. The long story is that I tested "make check" of GMP-ECM on gcc45 with some recent merge request, and with /bin/sh the command echo -e "..." > xxx did put the '-e' into the file xxx, which produced the reported Seg fault. Paul ___ gmp-bugs mailing list gmp-bugs@gmplib.org https://gmplib.org/mailman/listinfo/gmp-bugs
Re: Segmentation fault with mpz_inp_raw on gcc45
Paul Zimmermann writes: I was thus expecting it to return 0 in case of an invalid file. OK, so you deliberately sen d junk to mpz_inp_raw. That is fine, but it was not clear from your report. -- Torbjörn Please encrypt, key id 0xC8601622 ___ gmp-bugs mailing list gmp-bugs@gmplib.org https://gmplib.org/mailman/listinfo/gmp-bugs
Re: Segmentation fault with mpz_inp_raw on gcc45
Dear Torbjörn, > $ cat test_dummy2.save > -e > > # this is a comment line and should be ignored > > You do understand that mpz_inp_raw expects a binary file with a size > field followed by that many byytes of data, don't you? > > The file contents above make no sense. the documentation says: -- Function: size_t mpz_inp_raw (mpz_t ROP, FILE *STREAM) Input from stdio stream STREAM in the format written by 'mpz_out_raw', and put the result in ROP. Return the number of bytes read, or if an error occurred, return 0. I was thus expecting it to return 0 in case of an invalid file. Paul ___ gmp-bugs mailing list gmp-bugs@gmplib.org https://gmplib.org/mailman/listinfo/gmp-bugs
Re: Segmentation fault with mpz_inp_raw on gcc45
Paul Zimmermann writes: $ cat test_dummy2.save -e # this is a comment line and should be ignored You do understand that mpz_inp_raw expects a binary file with a size field followed by that many byytes of data, don't you? The file contents above make no sense. -- Torbjörn Please encrypt, key id 0xC8601622 ___ gmp-bugs mailing list gmp-bugs@gmplib.org https://gmplib.org/mailman/listinfo/gmp-bugs
