[PATCH] net: ipv6: Fix link-partner MAC address assignment

[Daniel Schwierzeck]

On 12/7/22 12:25, Vyacheslav Mitrofanov V wrote:
> On Tue, 2022-12-06 at 13:22 +0100, Daniel Schwierzeck wrote:
>> «Внимание! Данное письмо от внешнего адресата!»
>>
>> On 12/6/22 08:08, Viacheslav Mitrofanov wrote:
>>> MAC address of a link-partner is not saved for future use because
>>> of
>>> bad condition of if statement. Moreover it can potentially cause to
>>> NULL-pointer dereference.
>>>
>>> Signed-off-by: Viacheslav Mitrofanov <v.v.mitrofanov at yadro.com>
>>> ---
>>>    net/ndisc.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>
>> Reviewed-by: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>
>>
>>> diff --git a/net/ndisc.c b/net/ndisc.c
>>> index 3c0eeeaea3..56fc6390bc 100644
>>> --- a/net/ndisc.c
>>> +++ b/net/ndisc.c
>>> @@ -264,7 +264,7 @@ int ndisc_receive(struct ethernet_hdr *et,
>>> struct ip6_hdr *ip6, int len)
>>>                        ndisc_extract_enetaddr(ndisc,
>>> neigh_eth_addr);
>>>
>>>                        /* save address for later use */
>>> -                     if (!net_nd_packet_mac)
>>> +                     if (net_nd_packet_mac)
>>>                                memcpy(net_nd_packet_mac,
>>> neigh_eth_addr, 7);
>>>
>>>                        /* modify header, and transmit it */
>>
>> --
>> - Daniel
>>
> This patch is not appropriate!net_nd_packet_mac is just a pointer,
> moreover there is no memory allocation. It has just keep a pointer to
> neigh_eth_addr.
> So the solution must be sth. like net_nd_packet_mac = neigh_eth_addr;

that wouldn't make much sense. You would assign a global pointer to an 
array defined in local function scope. After ndisc_receive() has 
finished, the pointer will have an invalid address. The same problem 
exists in ping6_send(). Also the pointer is assigned in 
net_send_udp_packet6() to an array so the memcpy in ndisc_receive() 
would work.

BTW: the same approach is used with IPv4 and the arp_wait_packet_ethaddr 
pointer. The pointer is assigned in net_send_ip_packet() to an array and 
arp_receive() does a memcpy to that pointer if it is not NULL.

I think this patch is correct but you should revisit the assignment of 
net_nd_packet_mac in ping6_send(). You copy net_null_ethaddr to the 
local mac array and assign net_nd_packet_mac to the mac array. Either 
the assignment is useless or it should be a memcpy too.

-- 
- Daniel