Hello, On Mon, Mar 29, 2021 at 11:56:40AM +0000, Schreilechner, Dominik wrote: > </snip> > > > > I think the changes in ip_insertoptions() can be dropped completely, > > > because the if-statement uses ip-ip_hl, which might not be initialized. > > > > yes, you are right, I think we should rather go for this tweak: > > > > --------8<---------------8<---------------8<------------------8<-------- > > > > diff --git a/sys/netinet/ip_output.c b/sys/netinet/ip_output.c > > index e19b744fdf3..5c1222c0c86 100644 > > --- a/sys/netinet/ip_output.c > > +++ b/sys/netinet/ip_output.c > > @@ -767,7 +767,7 @@ ip_insertoptions(struct mbuf *m, struct mbuf *opt, int > > *phlen) > > return (m); /* XXX should fail */ > > > > /* check if options will fit to IP header */ > > - if ((optlen + (ip->ip_hl << 2)) > (0x0f << 2)) { > > + if ((optlen + (sizeof (struct ip))) > (0x0f << 2)) { > > *phlen = sizeof (struct ip); > > return (m); > > } > > --------8<---------------8<---------------8<------------------8<-------- > > Looks good for me. Just one more remark. In all other failure-cases in > ip_insertoptions() the old mbuf is returned without setting phlen. > Maybe, for the sake of consistence, always or never set phlen in a > failure-case? > ip_output() and icmp_send() do initialize (p)hlen before calling > ip_insertoptions(). So it is not strictly necessary to set it within > ip_insertoptions(). >
I plan to look at failure cases over Easter break (hopefully). I think we should just drop the response packet if we fail to add options. I'd like to have a better story for 'XXX should fail'. I'll keep your not on my mind, when I'll get to it. thanks and regards sashan