[PATCH v5 04/19] net: ipv6: Add Neighbor Discovery Protocol (NDP)

[Vyacheslav Mitrofanov V]

On Tue, 2022-12-06 at 03:14 +0100, Daniel Schwierzeck wrote:
> «Внимание! Данное письмо от внешнего адресата!»
> 
> On 12/2/22 10:18, Viacheslav Mitrofanov wrote:
> > Implement basic of NDP. It doesn't include such things as Router
> > Solicitation, Router Advertisement and Redirect. It just has
> > Neighbor
> > Solicitation and Neighbor Advertisement. Only these two features
> > are used
> > in u-boot IPv6. Implementation of some NDP functions uses API that
> > was
> > exposed in "net: ipv6: Add IPv6 basic primitives".
> > 
> > Also this patch inlcudes update in Makefile to build NDP.
> > 
> > Series-changes: 3
> > - Added structures and functions descriptions
> > - Fixed style problems
> > 
> > Series-changes: 4
> > - Fixed structures and functions description style
> > 
> > Signed-off-by: Viacheslav Mitrofanov <v.v.mitrofanov at yadro.com>
> > Reviewed-by: Ramon Fried <rfried.dev at gmail.com>
> > Reviewed-by: Simon Glass <sjg at chromium.org>
> > ---
> >   include/ndisc.h | 102 +++++++++++++++++
> >   net/Makefile    |   1 +
> >   net/ndisc.c     | 289
> > ++++++++++++++++++++++++++++++++++++++++++++++++
> >   3 files changed, 392 insertions(+)
> >   create mode 100644 include/ndisc.h
> >   create mode 100644 net/ndisc.c
> > 
> 
> ...
> 
> > +
> > +int ndisc_receive(struct ethernet_hdr *et, struct ip6_hdr *ip6,
> > int len)
> > +{
> > +     struct icmp6hdr *icmp =
> > +         (struct icmp6hdr *)(((uchar *)ip6) + IP6_HDR_SIZE);
> > +     struct nd_msg *ndisc = (struct nd_msg *)icmp;
> > +     uchar neigh_eth_addr[6];
> > +
> > +     switch (icmp->icmp6_type) {
> > +     case IPV6_NDISC_NEIGHBOUR_SOLICITATION:
> > +             debug("received neighbor solicitation for %pI6c from
> > %pI6c\n",
> > +                   &ndisc->target, &ip6->saddr);
> > +             if (ip6_is_our_addr(&ndisc->target) &&
> > +                 ndisc_has_option(ip6, ND_OPT_SOURCE_LL_ADDR)) {
> > +                     ndisc_extract_enetaddr(ndisc,
> > neigh_eth_addr);
> > +                     ip6_send_na(neigh_eth_addr, &ip6->saddr,
> > +                                 &ndisc->target);
> > +             }
> > +             break;
> > +
> > +     case IPV6_NDISC_NEIGHBOUR_ADVERTISEMENT:
> > +             /* are we waiting for a reply ? */
> > +             if (ip6_is_unspecified_addr(&net_nd_sol_packet_ip6))
> > +                     break;
> > +
> > +             if ((memcmp(&ndisc->target, &net_nd_rep_packet_ip6,
> > +                         sizeof(struct in6_addr)) == 0) &&
> > +                 ndisc_has_option(ip6, ND_OPT_TARGET_LL_ADDR)) {
> > +                     ndisc_extract_enetaddr(ndisc,
> > neigh_eth_addr);
> > +
> > +                     /* save address for later use */
> > +                     if (!net_nd_packet_mac)
> > +                             memcpy(net_nd_packet_mac,
> > neigh_eth_addr, 7);
> 
> Coverity reports the following:
> 
> CID 430977:  Null pointer dereferences  (FORWARD_NULL)
> Passing null pointer "net_nd_packet_mac" to "memcpy", which
> dereferences
> it. [Note: The source code implementation of the function has been
> overridden by a builtin model.]
> 
> CID 430974:  Memory - corruptions  (OVERRUN)
> Overrunning array "neigh_eth_addr" of 6 bytes by passing it to a
> function which accesses it at byte offset 6 using argument "7UL".
> [Note:
> The source code implementation of the function has been overridden by
> a
> builtin model.]
> 
> 
> Did you mean to write the following which would make more sense?
> 
> if (net_nd_packet_mac)
>      memcpy(net_nd_packet_mac, neigh_eth_addr, 7);
> 
> 
> > +
> > +                     /* modify header, and transmit it */
> > +                     memcpy(((struct ethernet_hdr
> > *)net_nd_tx_packet)->et_dest,
> > +                            neigh_eth_addr, 6);
> > +
> > +                     net_send_packet(net_nd_tx_packet,
> > +                                     net_nd_tx_packet_size);
> > +
> > +                     /* no ND request pending now */
> > +                     net_nd_sol_packet_ip6 = net_null_addr_ip6;
> > +                     net_nd_tx_packet_size = 0;
> > +                     net_nd_packet_mac = NULL;
> > +             }
> > +             break;
> > +     default:
> > +             debug("Unexpected ICMPv6 type 0x%x\n", icmp-
> > >icmp6_type);
> > +             return -1;
> > +     }
> > +
> > +     return 0;
> > +}
> 
> --
> - Daniel
> 
Hello Daniel! I do agree with your comments.
Will be fixed.Thank you!