On Thu, Oct 04, 2018 at 02:17:53PM +0200, Fernando Fernandez Mancera wrote:
> On 10/4/18 2:03 PM, Pablo Neira Ayuso wrote:
> > On Thu, Oct 04, 2018 at 01:57:17PM +0200, Fernando Fernandez Mancera wrote:
> > [...]
> > > diff --git a/net/netfilter/nfnetlink_osf.c b/net/netfilter/nfnetlink_osf.c
> > > index 00db27dfd2ff..e0fe1b8429ac 100644
> > > --- a/net/netfilter/nfnetlink_osf.c
> > > +++ b/net/netfilter/nfnetlink_osf.c
> > > @@ -32,9 +32,7 @@ static inline int nf_osf_ttl(const struct sk_buff *skb,
> > > {
> > > const struct iphdr *ip = ip_hdr(skb);
> > > - if (ttl_check != -1) {
> > > - if (ttl_check == NF_OSF_TTL_TRUE)
> > > - return ip->ttl == f_ttl;
> > > + if (ttl_check != 0) {
> >
> > May ttl_check now ever be -1 now that we do not need it in nft_osf?
> >
> > If xt_osf never does it, we can probably remove this branch, ie.
> >
> > - if (ttl_check != -1) {
> >
> > and save one level of indentation.
> >
> > This would need a careful look at current xt_osf.c - as well as its
> > previous one code - to make sure we do not break anything if we remove
> > this ttl_check != -1 branch.
> >
>
> Currently in xt_osf.c if the option "--ttl" is not found by default, after
> the v3 iteration, it sets ttl_check to 0 as you can see here:
>
> @@ -213,7 +211,7 @@ nf_osf_match(const struct sk_buff *skb, u_int8_t family,
> if (!tcp)
> return false;
>
> - ttl_check = (info->flags & NF_OSF_TTL) ? info->ttl : -1;
> + ttl_check = (info->flags & NF_OSF_TTL) ? info->ttl : 0;
>
> I think we can remove the "ttl_check != -1" branch and keep the if statement
> that checks "ttl_check == NF_OSF_TTL_TRUE". I can do it and test if it
> breaks something in xt_osf.c but I think this change should break nothing.
Then, go ahead remove it. Thanks.
