> -----Original Message-----
> From: Alexandr Nedvedicky <alexandr.nedvedi...@oracle.com>
> Sent: Mittwoch, 24. März 2021 23:09
> To: Schreilechner, Dominik (RC-AT DI FA DH-GRAZ ICO)
> <dominik.schreilech...@siemens.com>
> Cc: tech@openbsd.org
> Subject: Re: [External] : [ICMP] IP options lead to malformed reply
>
> Hello,
>
> </snip>
>
> > We really need to fix ip_send() such the output task will handle IP
> options
> > properly.
>
>     took a look at bug reported by Dominik earlier today. Looks like
>     there are two issues:
>
>       1) ip_insertoptions() does not update length of IP header (ip_hl)
>
>       2) ip_hl is being overridden anyway later in ip_output() called
>          by ip_send_dispatch() to send ICMP error packet out. Looks
>          like ip_send_dispatch() should use IP_RAWOUTPUT flag so
>          ip_hl won't get overridden.

Maybe I have overlooked something, but I see two problems with this.
When the IP_RAWOUTPUT flag is set, all packets send via ip_send() are
no longer included in the ips_localout statistic. Also, all callers of
ip_send() need to fill out the IP header themselves (as it would be
done in the beginning of ip_output() without the IP_RAWOUTPUT flag). As
far as I can tell this is not done for packets in a gif tunnel (e.g.
the ip_id is not calculated/added).

>
>     Diff below fixes both issues. We still have no good story when
>     ip_insertoptions() fails. I'll send another diff later this week.
>
>
>     diff below makes 'nping --ip-options T ...' to work. OK?
>
> thanks and
> regards
> sashan

Regards
Dominik

>
> --------8<---------------8<---------------8<------------------8<--------
> diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c
> index 0ec3f723be4..234c798e7d4 100644
> --- a/sys/netinet/ip_input.c
> +++ b/sys/netinet/ip_input.c
> @@ -1789,7 +1789,7 @@ ip_send_dispatch(void *xmq)
>
>       NET_LOCK();
>       while ((m = ml_dequeue(&ml)) != NULL) {
> -             ip_output(m, NULL, NULL, 0, NULL, NULL, 0);
> +             ip_output(m, NULL, NULL, IP_RAWOUTPUT, NULL, NULL,
> 0);
>       }
>       NET_UNLOCK();
>  }
> diff --git a/sys/netinet/ip_output.c b/sys/netinet/ip_output.c
> index c01a3e7803c..ea803077304 100644
> --- a/sys/netinet/ip_output.c
> +++ b/sys/netinet/ip_output.c
> @@ -765,6 +765,11 @@ ip_insertoptions(struct mbuf *m, struct mbuf
> *opt, int *phlen)
>       optlen = opt->m_len - sizeof(p->ipopt_dst);
>       if (optlen + ntohs(ip->ip_len) > IP_MAXPACKET)
>               return (m);             /* XXX should fail */
> +
> +     /* check if options will fit to IP header */
> +     if ((optlen + (ip->ip_hl << 2)) > (0x0f << 2))
> +             return (m);
> +
>       if (p->ipopt_dst.s_addr)
>               ip->ip_dst = p->ipopt_dst;
>       if (m->m_flags & M_EXT || m->m_data - optlen < m->m_pktdat)
> {
> @@ -790,6 +795,7 @@ ip_insertoptions(struct mbuf *m, struct mbuf
> *opt, int *phlen)
>       memcpy(ip + 1, p->ipopt_list, optlen);
>       *phlen = sizeof(struct ip) + optlen;
>       ip->ip_len = htons(ntohs(ip->ip_len) + optlen);
> +     ip->ip_hl += (optlen >> 2);
>       return (m);
>  }
>

Reply via email to