Re: [PATCH 0/4] readelf: Fixes for IN_RANGE

[Nick Clifton <nickc at redhat dot com>]

Hi Christian,

> Various small fixes for the IN_RANGE macro.

Given the size of these patches, it would have been simpler 
just to submit them as one single patch....


> diff --git a/binutils/ChangeLog b/binutils/ChangeLog
> index 36b75c1f3c..16ed7ed5b0 100644
> --- a/binutils/ChangeLog
> +++ b/binutils/ChangeLog
> @@ -1,3 +1,7 @@
> +2019-11-03  Christian Eggers  <ceggers@gmx.de>
> +
> +	* readelf.c (IN_RANGE): Rename parameter OFF to SIZE.
> +

Including ChangeLog entries is very nice, but please could you
provide them as plain text, and not a context diff.  The diff
almost never applies cleanly because the ChangeLogs are being
constantly updated.


> +	* readelf.c (IN_RANGE): Fix off by one error
> +	whilst checking reloc location against section size.

>  #define IN_RANGE(START,END,ADDR,SIZE)		\
> -  (((ADDR) >=3D (START)) && ((ADDR) < (END)) && ((ADDR) + (SIZE) < (END)))
> +  (((ADDR) >=3D (START)) && ((ADDR) < (END)) && ((ADDR) + (SIZE) <=3D (END=
> )))

This is wrong.  The test deliberately uses "< END" rather than "<= END".
Ie the END value is not the last valid address in a region but rather the first
invalid address beyond the end of the region.

Also note - your mailer has corrupted the context diff, adding "3D" after
the = sign, and then wrapping the lines at 80 characters. :-(  If you cannot
turn this behaviour off, then could you provide the patches as attachments,
rather than inline please.


> 
>  	  rloc =3D start + rp->r_offset;
> -	  if (rloc >=3D end || (rloc + reloc_size) > end || (rloc < start))
> +	  if (!IN_RANGE(start, end, rloc, reloc_size))
>  	    {
>  	      warn (_("skipping invalid relocation offset 0x%lx in section %s\n"),
>  		    (unsigned long) rp->r_offset,

Note - this patch is wrong if fix 3/4 is applied as the test needs to check
for rloc >= end, whereas IN_RANGE would check for rloc > end.  If however 3/4
is not applied (as suggested above) then this patch is OK.

Please could you address these issues and resubmit your patch(es).  Thanks.

Cheers
  Nick