[CRIU] [PATCH 1/3] compel: fix sign-extension in get_strings_section

Dmitry Safonov dsafonov at virtuozzo.com
Wed May 25 05:52:57 PDT 2016


Well, I hope, I will not make integer promotion mistakes anymore:
> 6.3.1.1
>   If an int can represent all values of the original type, the value
> is converted to an int; otherwise, it is converted to an unsigned int.
> These are called the integer promotions.48) All other types are
> unchanged by the integer promotions.

>>> CID 161317:    (SIGN_EXTENSION)
>>> Suspicious implicit sign extension: "hdr->e_shentsize" with type
    "unsigned short" (16 bits, unsigned) is promoted in
    "hdr->e_shentsize * hdr->e_shnum" to type "int" (32 bits, signed),
    then sign-extended to type "unsigned long" (64 bits, unsigned).
    If "hdr->e_shentsize * hdr->e_shnum" is greater than 0x7FFFFFFF,
    the upper bits of the result will all be 1.
96      size_t sec_table_size = hdr->e_shentsize * hdr->e_shnum;

>>> CID 161317:    (SIGN_EXTENSION)
>>> Suspicious implicit sign extension: "hdr->e_shentsize" with type
    "unsigned short" (16 bits, unsigned) is promoted in
    "hdr->e_shentsize * hdr->e_shstrndx" to type "int" (32 bits, signed),
    then sign-extended to type "unsigned long" (64 bits, unsigned).
    If "hdr->e_shentsize * hdr->e_shstrndx" is greater than 0x7FFFFFFF,
    the upper bits of the result will all be 1.
111             addr = sec_table + hdr->e_shentsize * hdr->e_shstrndx;

Fixes: #157
Fixes: commit 36664a3cabec ("compel: separate get_strings_section from
__handle_elf").

Reported-by: Coverity
Reported-by: Andrew Vagin <avagin at virtuozzo.com>
Cc: Andrew Vagin <avagin at virtuozzo.com>
Cc: Cyrill Gorcunov <gorcunov at openvz.org>
Signed-off-by: Dmitry Safonov <dsafonov at virtuozzo.com>
---
 compel/handle-elf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/compel/handle-elf.c b/compel/handle-elf.c
index eabff7c4f067..22a7f0f5d4c2 100644
--- a/compel/handle-elf.c
+++ b/compel/handle-elf.c
@@ -93,7 +93,7 @@ static bool is_header_supported(Ehdr_t *hdr)
 
 static const char *get_strings_section(Ehdr_t *hdr, uintptr_t mem, size_t size)
 {
-	size_t sec_table_size = hdr->e_shentsize * hdr->e_shnum;
+	size_t sec_table_size = ((size_t) hdr->e_shentsize) * hdr->e_shnum;
 	uintptr_t sec_table = mem + hdr->e_shoff;
 	Shdr_t *secstrings_hdr;
 	uintptr_t addr;
@@ -108,7 +108,7 @@ static const char *get_strings_section(Ehdr_t *hdr, uintptr_t mem, size_t size)
 	 * strings section header's offset in section headers table is
 	 * (size of section header * index of string section header)
 	 */
-	addr = sec_table + hdr->e_shentsize * hdr->e_shstrndx;
+	addr = sec_table + ((size_t) hdr->e_shentsize) * hdr->e_shstrndx;
 	if (__ptr_struct_oob(addr, sizeof(Shdr_t),
 			sec_table, sec_table + sec_table_size)) {
 		pr_err("String section header @%#zx is out of [%#zx, %#zx)\n",
-- 
2.8.2



More information about the CRIU mailing list