9:30 a.m.
On Mon, Oct 29, 2018 at 9:22 PM Daniel Gryniewicz <dang(a)redhat.com> wrote:
On 10/27/2018 08:41 AM, William Allen Simpson wrote:
> On 10/25/18 12:17 AM, Suhrud Patankar wrote:
>> if (addr->len < sizeof(cu->cu_raddr))
>> The addr is IPv4 but cu->cu_raddr is IPv6.
>> We want to copy the addr->buf to cu->cu_raddr but it is failing even
>> if addr->len is less than sizeof(cu->cu_raddr).
>> The if() above is buggy.
>>
> You are correct. That < should be >, to prevent copying too large an
> address in the memcpy(). That line hasn't been touched since the fork,
> and this bug is still in tirpc 1.0.2.
>
>
>> If I disable PORTMAP then tirpc uses TCP to connect to statd and does
>> not set the addr->len as per IPV4. So the code goes through correctly.
>>
> This is the underlying problem. This version of the code is so old
> that it never gets tested, because nobody requires UDP instead of TCP.
> That's left over from 20 years ago. Better to just remove all the
> PORTMAP code entirely. IMnsHO.
>
Good catch, I completely missed that.
Suhrud, can you test with this:
https://github.com/nfs-ganesha/ntirpc/pull/157
The bug I was chasing is fixed in 1.7.1. It was related to
clnt_dg_control(CLSET_VERS) .
With 1.7.1, it is not required to fix the addr->len comparison. It
works as expected.
Thanks for looking into it!
>
> Daniel