Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 8 Jul 2018 12:03:49 -0400
From:      Mark Johnston <markjdb@gmail.com>
To:        Mark Millard <marklmi@yahoo.com>
Cc:        FreeBSD Toolchain <freebsd-toolchain@freebsd.org>
Subject:   Re: src/contrib/elftoolchain/elfcopy/sections.c underallocates for Elf64_Rela and Elf32_Rela?
Message-ID:  <20180708160349.GC18193@pesky>
In-Reply-To: <79954D9E-0A93-4148-A2C6-B5113E59AE28@yahoo.com>
References:  <79954D9E-0A93-4148-A2C6-B5113E59AE28@yahoo.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Jul 08, 2018 at 08:47:38AM -0700, Mark Millard via freebsd-toolchain wrote:
> src/contrib/elftoolchain/elfcopy/sections.c has and uses the macro:
> 
> 716	#define	COPYREL(REL, SZ) do {					\
> 717		if (nrels == 0) {					\
> 718			if ((REL##SZ = malloc(cap *			\
> 719			    sizeof(Elf##SZ##_Rel))) == NULL)		\
> 720				err(EXIT_FAILURE, "malloc failed");	\
> 721		}							\
> 722		if (nrels >= cap) {					\
> 723			cap *= 2;					\
> 724			if ((REL##SZ = realloc(REL##SZ, cap *		\
> 725			    sizeof(Elf##SZ##_Rel))) == NULL)		\
> 726				err(EXIT_FAILURE, "realloc failed");	\
> 727		}							\
> 728		REL##SZ[nrels].r_offset = REL.r_offset;			\
> 729		REL##SZ[nrels].r_info	= REL.r_info;			\
> 730		if (s->type == SHT_RELA)				\
> 731			rela##SZ[nrels].r_addend = rela.r_addend;	\
> 732		nrels++;						\
> 733	} while (0)
> 
> The context has:
> 
> 687		Elf32_Rel	*rel32;
> 688		Elf64_Rel	*rel64;
> 689		Elf32_Rela	*rela32;
> 690		Elf64_Rela	*rela64;
> 
> So for, say, COPYREL(rela,64), the macro uses sizeof(Elf64_Rel) instead
> of sizeof(ELF64_Rela) in malloc and realloc but Elf64_Rela is the
> larger structure of the two ELF64_ types (by also having .r_addend).
> 
> The scan build on ci.freebsd.org complains about this:
> 
> Result of 'realloc' is converted to a pointer of type 'Elf64_Rela', which is incompatible with sizeof operand type 'Elf64_Rel'
> 
> So far it does not look like a false-positive to me.

Looks like a valid bug to me.  I think the following patch is needed:

diff --git a/contrib/elftoolchain/elfcopy/sections.c b/contrib/elftoolchain/elfcopy/sections.c
index b292d18693b4..92265289c7d1 100644
--- a/contrib/elftoolchain/elfcopy/sections.c
+++ b/contrib/elftoolchain/elfcopy/sections.c
@@ -716,13 +716,13 @@ filter_reloc(struct elfcopy *ecp, struct section *s)
 #define	COPYREL(REL, SZ) do {					\
 	if (nrels == 0) {					\
 		if ((REL##SZ = malloc(cap *			\
-		    sizeof(Elf##SZ##_Rel))) == NULL)		\
+		    sizeof(*REL##SZ))) == NULL)			\
 			err(EXIT_FAILURE, "malloc failed");	\
 	}							\
 	if (nrels >= cap) {					\
 		cap *= 2;					\
 		if ((REL##SZ = realloc(REL##SZ, cap *		\
-		    sizeof(Elf##SZ##_Rel))) == NULL)		\
+		    sizeof(*REL##SZ))) == NULL)			\
 			err(EXIT_FAILURE, "realloc failed");	\
 	}							\
 	REL##SZ[nrels].r_offset = REL.r_offset;			\



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180708160349.GC18193>