From a1ce4da11ca0710a166e5048251aa1e128a83695 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Tue, 26 Nov 2013 20:25:53 +0100 Subject: erts: Fix invalid read in bitstring comparison Valgrind complained that erts_cmp_bits sometimes read one byte too much from bitstrings. The byte was never used and the invalid read is safe in opt-VM due to padding in binary allocation (CHICKED_PAD). --- erts/emulator/beam/erl_bits.c | 79 +++++++++++++++++++++++++++---------------- 1 file changed, 49 insertions(+), 30 deletions(-) (limited to 'erts') diff --git a/erts/emulator/beam/erl_bits.c b/erts/emulator/beam/erl_bits.c index 43eb691338..d5b0e341a1 100644 --- a/erts/emulator/beam/erl_bits.c +++ b/erts/emulator/beam/erl_bits.c @@ -1,7 +1,7 @@ /* * %CopyrightBegin% * - * Copyright Ericsson AB 1999-2012. All Rights Reserved. + * Copyright Ericsson AB 1999-2013. All Rights Reserved. * * The contents of this file are subject to the Erlang Public License, * Version 1.1, (the "License"); you may not use this file except in @@ -1810,6 +1810,11 @@ erts_cmp_bits(byte* a_ptr, size_t a_offs, byte* b_ptr, size_t b_offs, size_t siz Uint rshift; int cmp; + ASSERT(a_offs < 8 && b_offs < 8); + + if (size == 0) + return 0; + if (((a_offs | b_offs | size) & 7) == 0) { int byte_size = size >> 3; return sys_memcmp(a_ptr, b_ptr, byte_size); @@ -1818,13 +1823,15 @@ erts_cmp_bits(byte* a_ptr, size_t a_offs, byte* b_ptr, size_t b_offs, size_t siz /* Compare bit by bit until a_ptr is aligned on byte boundary */ a = *a_ptr++; b = *b_ptr++; - while (size > 0) { + for (;;) { a_bit = get_bit(a, a_offs); b_bit = get_bit(b, b_offs); if ((cmp = (a_bit-b_bit)) != 0) { return cmp; } - size--; + if (--size == 0) + return 0; + b_offs++; if (b_offs == 8) { b_offs = 0; @@ -1839,37 +1846,49 @@ erts_cmp_bits(byte* a_ptr, size_t a_offs, byte* b_ptr, size_t b_offs, size_t siz } /* Compare byte by byte as long as at least 8 bits remain */ - lshift = b_offs; - rshift = 8 - lshift; - while (size >= 8) { - byte b_cmp = (b << lshift); - b = *b_ptr++; - b_cmp |= b >> rshift; - if ((cmp = (a - b_cmp)) != 0) { - return cmp; - } + if (size >= 8) { + lshift = b_offs; + rshift = 8 - lshift; + for (;;) { + byte b_cmp = (b << lshift); + b = *b_ptr++; + b_cmp |= b >> rshift; + if ((cmp = (a - b_cmp)) != 0) { + return cmp; + } + size -= 8; + if (size < 8) + break; + a = *a_ptr++; + } + + if (size == 0) + return 0; a = *a_ptr++; - size -= 8; } /* Compare the remaining bits bit by bit */ - while (size > 0) { - a_bit = get_bit(a, a_offs); - b_bit = get_bit(b, b_offs); - if ((cmp = (a_bit-b_bit)) != 0) { - return cmp; - } - a_offs++; - if (a_offs == 8) { - a_offs = 0; - a = *a_ptr++; - } - b_offs++; - if (b_offs == 8) { - b_offs = 0; - b = *b_ptr++; - } - size--; + if (size > 0) { + for (;;) { + a_bit = get_bit(a, a_offs); + b_bit = get_bit(b, b_offs); + if ((cmp = (a_bit-b_bit)) != 0) { + return cmp; + } + if (--size == 0) + return 0; + + a_offs++; + if (a_offs == 8) { + a_offs = 0; + a = *a_ptr++; + } + b_offs++; + if (b_offs == 8) { + b_offs = 0; + b = *b_ptr++; + } + } } return 0; -- cgit v1.2.3 From f3589a79de29f1a4e6a2d03518739af214ae9600 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Tue, 26 Nov 2013 20:31:43 +0100 Subject: erts: Optimize comparison for bitstrings with byte aligned start --- erts/emulator/beam/erl_bits.c | 44 +++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) (limited to 'erts') diff --git a/erts/emulator/beam/erl_bits.c b/erts/emulator/beam/erl_bits.c index d5b0e341a1..872ded4be7 100644 --- a/erts/emulator/beam/erl_bits.c +++ b/erts/emulator/beam/erl_bits.c @@ -1823,25 +1823,27 @@ erts_cmp_bits(byte* a_ptr, size_t a_offs, byte* b_ptr, size_t b_offs, size_t siz /* Compare bit by bit until a_ptr is aligned on byte boundary */ a = *a_ptr++; b = *b_ptr++; - for (;;) { - a_bit = get_bit(a, a_offs); - b_bit = get_bit(b, b_offs); - if ((cmp = (a_bit-b_bit)) != 0) { - return cmp; - } - if (--size == 0) - return 0; + if (a_offs) { + for (;;) { + a_bit = get_bit(a, a_offs); + b_bit = get_bit(b, b_offs); + if ((cmp = (a_bit-b_bit)) != 0) { + return cmp; + } + if (--size == 0) + return 0; - b_offs++; - if (b_offs == 8) { - b_offs = 0; - b = *b_ptr++; - } - a_offs++; - if (a_offs == 8) { - a_offs = 0; - a = *a_ptr++; - break; + b_offs++; + if (b_offs == 8) { + b_offs = 0; + b = *b_ptr++; + } + a_offs++; + if (a_offs == 8) { + a_offs = 0; + a = *a_ptr++; + break; + } } } @@ -1879,10 +1881,8 @@ erts_cmp_bits(byte* a_ptr, size_t a_offs, byte* b_ptr, size_t b_offs, size_t siz return 0; a_offs++; - if (a_offs == 8) { - a_offs = 0; - a = *a_ptr++; - } + ASSERT(a_offs < 8); + b_offs++; if (b_offs == 8) { b_offs = 0; -- cgit v1.2.3 From 35b4fd5d91cbeb1b9c8540996624856134e091e9 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Wed, 27 Nov 2013 18:58:14 +0100 Subject: erts: Fix invalid read when appending binaries during call trace Found by valgrind. Probably safe on opt-VM due to CHICKEN_PAD. --- erts/emulator/beam/erl_bits.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'erts') diff --git a/erts/emulator/beam/erl_bits.c b/erts/emulator/beam/erl_bits.c index 872ded4be7..73765772c8 100644 --- a/erts/emulator/beam/erl_bits.c +++ b/erts/emulator/beam/erl_bits.c @@ -1491,7 +1491,7 @@ erts_bs_private_append(Process* p, Eterm bin, Eterm build_size_term, Uint unit) bptr->flags = 0; bptr->orig_size = new_size; erts_refc_init(&bptr->refc, 1); - sys_memcpy(bptr->orig_bytes, binp->orig_bytes, pb->size); + sys_memcpy(bptr->orig_bytes, binp->orig_bytes, binp->orig_size); pb->flags |= PB_IS_WRITABLE | PB_ACTIVE_WRITER; pb->val = bptr; pb->bytes = (byte *) bptr->orig_bytes; -- cgit v1.2.3