Re: [PATCH] objtool: Fix infinite loop in for_offset_range()

From: Randy Dunlap
Date: Sat Apr 25 2020 - 16:08:10 EST


On 4/25/20 12:19 PM, Josh Poimboeuf wrote:
> Randy reported that objtool got stuck in an infinite loop when
> processing drivers/i2c/busses/i2c-parport.o. It was caused by the
> following code:
>
> 00000000000001fd <line_set>:
> 1fd: 48 b8 00 00 00 00 00 movabs $0x0,%rax
> 204: 00 00 00
> 1ff: R_X86_64_64 .rodata-0x8
> 207: 41 55 push %r13
> 209: 41 89 f5 mov %esi,%r13d
> 20c: 41 54 push %r12
> 20e: 49 89 fc mov %rdi,%r12
> 211: 55 push %rbp
> 212: 48 89 d5 mov %rdx,%rbp
> 215: 53 push %rbx
> 216: 0f b6 5a 01 movzbl 0x1(%rdx),%ebx
> 21a: 48 8d 34 dd 00 00 00 lea 0x0(,%rbx,8),%rsi
> 221: 00
> 21e: R_X86_64_32S .rodata
> 222: 48 89 f1 mov %rsi,%rcx
> 225: 48 29 c1 sub %rax,%rcx
>
> find_jump_table() saw the .rodata reference and tried to find a jump
> table associated with it (though there wasn't one). The -0x8 rela
> addend is unusual. It caused find_jump_table() to send a negative
> table_offset (unsigned 0xfffffffffffffff8) to find_rela_by_dest().
>
> The negative offset should have been harmless, but it actually threw
> for_offset_range() for a loop... literally. When the mask value got
> incremented past the end value, it also wrapped to zero, causing the
> loop exit condition to remain true forever.
>
> Prevent this scenario from happening by ensuring the incremented value
> is always >= the starting value.
>
> Fixes: 74b873e49d92 ("objtool: Optimize find_rela_by_dest_range()")
> Reported-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
> Signed-off-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>

Hi Josh,

When applied to linux-next 20200414 (where it was reported) and using
config-r2092, objtool still loops (I killed it after 6 minutes of CPU time).

When applied to linux-next 20200424 and using the same config-r2092 file,
objtool appears to terminate normally and the entire build does also.

Acked-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
Tested-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>

thanks.

> ---
> tools/objtool/elf.h | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h
> index 5e76ac38cf99..f753148f5dac 100644
> --- a/tools/objtool/elf.h
> +++ b/tools/objtool/elf.h
> @@ -89,9 +89,10 @@ struct elf {
> #define OFFSET_STRIDE (1UL << OFFSET_STRIDE_BITS)
> #define OFFSET_STRIDE_MASK (~(OFFSET_STRIDE - 1))
>
> -#define for_offset_range(_offset, _start, _end) \
> - for (_offset = ((_start) & OFFSET_STRIDE_MASK); \
> - _offset <= ((_end) & OFFSET_STRIDE_MASK); \
> +#define for_offset_range(_offset, _start, _end) \
> + for (_offset = ((_start) & OFFSET_STRIDE_MASK); \
> + _offset >= ((_start) & OFFSET_STRIDE_MASK) && \
> + _offset <= ((_end) & OFFSET_STRIDE_MASK); \
> _offset += OFFSET_STRIDE)
>
> static inline u32 sec_offset_hash(struct section *sec, unsigned long offset)
>


--
~Randy