Subject: Re: [PATCH RESEND nf-next] netfilter: add support for matching IPv4 options

From: Stephen Suryaputra <[email-masked]>
Date: Tue, 18 Jun 2019 10:13:55 -0400

On Tue, Jun 18, 2019 at 05:31:12PM +0200, Pablo Neira Ayuso wrote:
> > +{
> > +	unsigned char optbuf[sizeof(struct ip_options) + 41];
> 
> In other parts of the kernel this is + 40:
> 
> net/ipv4/cipso_ipv4.c:  unsigned char optbuf[sizeof(struct ip_options) + 40];
> 
> here it is + 41.
>
> ...
>
> > +	/* Copy the options since __ip_options_compile() modifies
> > +	 * the options. Get one byte beyond the option for target < 0
> 
> How does this "one byte beyond the option" trick works?

I used ipv6_find_hdr() as a reference. There if target is set to less
than 0, then the offset points to the byte beyond the extension header.
In this function, it points to the byte beyond the option. I wanted to
be as close as a working code as possible. Also, why +41 instead of +40.

> > +		if (opt->end) {
> > +			*offset = opt->end + start;
> > +			target = IPOPT_END;
> 
> May I ask, what's the purpose of IPOPT_END? :-)

My understanding is that in ipv6_find_hdr() if the nexthdr is
NEXTHDR_NONE, then that's the one being returned. The same here: target
is the return value.

> Apart from the above, this looks good to me.

AOK for other comments. I can spin another version.

Thank you,

Stephen.