Re: [Nasm-devel] [PATCH] asm: Always check the size of address is sitting inside bounds

[Cyrill G. <gor...@gm...>]

On Mon, Aug 28, 2017 at 02:50:58PM -0700, H. Peter Anvin wrote:
> On 08/28/17 14:46, H. Peter Anvin wrote:
> > On 08/28/17 13:55, Cyrill Gorcunov wrote:
> >> On Sun, Aug 27, 2017 at 04:46:32PM -0700, hp...@zy... wrote:
> >>> On August 27, 2017 2:16:18 PM PDT, Cyrill Gorcunov <gor...@gm...> wrote:
> >>>> In particular
> >>>>
> >>>> | call 0xdeadbeef12345678
> >>>>
> >>>> should warn a user about address cutoff.
> >>>>
> >>>> https://bugzilla.nasm.us/show_bug.cgi?id=3392422
> >>>>
> >>>> Signed-off-by: Cyrill Gorcunov <gor...@gm...>
> >>>> ---
> >>>>
> >>>> Peter, I don't get why we check for address not being overflowed
> >>>> under _if_ condition only, looks like a bug for me, no?
> >>>>
> >>>> asm/assemble.c | 2 +-
> >>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/asm/assemble.c b/asm/assemble.c
> >>>> index 6d3e25e..e81354f 100644
> >>>> --- a/asm/assemble.c
> >>>> +++ b/asm/assemble.c
> >>>> @@ -374,8 +374,8 @@ static void out(struct out_data *data)
> >>>>         nasm_assert(data->size <= 8);
> >>>>         asize = data->size;
> >>>>         amax = ofmt->maxbits >> 3; /* Maximum address size in bytes */
> >>>> +        warn_overflow_out(addrval, asize, data->sign);
> >>>>         if (data->tsegment == fixseg && data->twrt == NO_SEG) {
> >>>> -            warn_overflow_out(addrval, asize, data->sign);
> >>>>             xdata.q = cpu_to_le64(addrval);
> >>>>             data->data = xdata.b;
> >>>>             data->type = OUT_RAWDATA;
> >>>
> >>> Because it is the linker's job.  We have to check for overflow if we are
> >>> doing a conversation to raw bytes (doing the linker's job for it).
> >>
> >> But length of immediate is known on source code level only, when
> >> matcher finds suitable instruction template we choose the size of address
> >> to be passed into out() engine, and only we know which value a user
> >> has defined. And if value is too big (just like as in example above)
> >> we should warn him.
> >>
> > 
> > Consider:
> > 
> > 	jmp 0x0012_3456_789a_bcde
> > 
> > This is totally legitimate if the code is linked to address
> > 0x0012_3456_0000_0000!
> > 
> > Only the linker can know that.
> > 
> 
> Similarly:
> 
> 	add rax,[rel 0x0012_3456_789a_bcde]
> 
> ... only the linker can know if that is legal.
> 
> However, neither one of:
> 
> 	add rax,0x0012_3456_789a_bcde
> 
> 	add rax,[abs 0x0012_3456_789a_bcde]
> 
> ... can ever be valid, and we will warn.
> 
> 	mov rax,[abs qword 0x0012_3456_789a_bcde]
> 
> ... is a special case, and is legal, however.  It has to be specifically
> a mov al/ax/eax/rax.

I see, indeed. Somehow missed it in first place. Thanks for explanation!

	Cyrill