[CRIU] [PATCH 3/5] pie: piegen -- Don't dereference section data too early

[Laurent Dufour]

On 09/06/2015 15:18, Cyrill Gorcunov wrote:
> Also use already computed @sec_hdrs and add
> ptr_func_exit helpers calls.
> 
> Also a few style nitfix.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
>  pie/piegen/elf.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/pie/piegen/elf.c b/pie/piegen/elf.c
> index a908624135eb..51f3edf9d513 100644
> --- a/pie/piegen/elf.c
> +++ b/pie/piegen/elf.c
> @@ -465,18 +465,21 @@ int handle_elf(const piegen_opt_t *opts, void *mem, size_t size)
> 
>  	pr_out("static __maybe_unused const char %s[] = {\n\t", opts->stream_name);
> 
> -	for (i=0, k=0; i < hdr->e_shnum; i++) {
> +	for (i = 0, k = 0; i < hdr->e_shnum; i++) {
> +		Shdr_t *sh = sec_hdrs[i];
> +		unsigned char *shdata;
>  		size_t j;
> -		Shdr_t *sh = mem + hdr->e_shoff + hdr->e_shentsize * i;
> -		unsigned char *shdata = mem + sh->sh_offset;
> 
>  		if (!(sh->sh_flags & SHF_ALLOC) || !sh->sh_size)
>  			continue;
> 
> -		pr_debug("Copying section '%s'\n" \
> +		shdata =  mem + sh->sh_offset;
> +		ptr_func_exit(&secstrings[sh->sh_name]);
> +
> +		pr_debug("Copying section '%s'\n"
>  			 "\tstart:0x%lx (gap:0x%lx) size:0x%lx\n",
>  			 &secstrings[sh->sh_name], (unsigned long) sh->sh_addr,
> -			 (unsigned long)(sh->sh_addr - k), sh->sh_size);
> +			 (unsigned long)(sh->sh_addr - k), (unsigned long)sh->sh_size);
> 
>  		/* write 0 in the gap between the 2 sections */
>  		for (;k < sh->sh_addr; k++) {
> @@ -485,9 +488,10 @@ int handle_elf(const piegen_opt_t *opts, void *mem, size_t size)
>  			pr_out("0x00,");
>  		}
> 
> -		for (j=0; j < sh->sh_size; j++, k++) {
> +		for (j = 0; j < sh->sh_size; j++, k++) {
>  			if (k && (k % 8) == 0)
>  				pr_out("\n\t");
> +			ptr_func_exit(&shdata[j]);

I don't think this is the right place to put such a check. If we want to
ensure that the pointer is in the object file (which should be
guaranteed by the compiler), we should check for the end pointer of the
shdata prior to this loop. Here we will check each byte of each section...

This being said, I can't see the need for such a check. Could you please
elaborate on this ?

>  			pr_out("0x%02x,", shdata[j]);
>  		}
>  	}
>