| Message ID | 1472792932-26187-1-git-send-email-fgao@ikuai8.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
fgao@ikuai8.com <fgao@ikuai8.com> wrote: > From: Gao Feng <fgao@ikuai8.com> > > When memory is exhausted, nfct_seqadj_ext_add may fail to add the seqadj > extension. But the function nf_ct_seqadj_init doesn't check if get valid > seqadj pointer by the nfct_seqadj, while other functions perform the > sanity check. > > So the system would be panic when nfct_seqadj_ext_add failed. > > Signed-off-by: Gao Feng <fgao@ikuai8.com> > diff --git a/net/netfilter/nf_conntrack_seqadj.c b/net/netfilter/nf_conntrack_seqadj.c > index dff0f0c..2c8e201 100644 > --- a/net/netfilter/nf_conntrack_seqadj.c > +++ b/net/netfilter/nf_conntrack_seqadj.c > @@ -16,9 +16,14 @@ int nf_ct_seqadj_init(struct nf_conn *ct, enum ip_conntrack_info ctinfo, > if (off == 0) > return 0; > > + seqadj = nfct_seqadj(ct); > + if (unlikely(!seqadj)) { > + WARN_ONCE(1, "Missing nfct_seqadj_ext_add() setup call\n"); > + return 0; > + } > + Not sure this WARN() is really needed, I would remove it (since its most likely only missing due to memory shortage). Other than that, this looks good. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Florian, On Fri, Sep 2, 2016 at 2:59 PM, Florian Westphal <fw@strlen.de> wrote: > fgao@ikuai8.com <fgao@ikuai8.com> wrote: >> From: Gao Feng <fgao@ikuai8.com> >> >> When memory is exhausted, nfct_seqadj_ext_add may fail to add the seqadj >> extension. But the function nf_ct_seqadj_init doesn't check if get valid >> seqadj pointer by the nfct_seqadj, while other functions perform the >> sanity check. >> >> So the system would be panic when nfct_seqadj_ext_add failed. >> >> Signed-off-by: Gao Feng <fgao@ikuai8.com> > >> diff --git a/net/netfilter/nf_conntrack_seqadj.c b/net/netfilter/nf_conntrack_seqadj.c >> index dff0f0c..2c8e201 100644 >> --- a/net/netfilter/nf_conntrack_seqadj.c >> +++ b/net/netfilter/nf_conntrack_seqadj.c >> @@ -16,9 +16,14 @@ int nf_ct_seqadj_init(struct nf_conn *ct, enum ip_conntrack_info ctinfo, >> if (off == 0) >> return 0; >> >> + seqadj = nfct_seqadj(ct); >> + if (unlikely(!seqadj)) { >> + WARN_ONCE(1, "Missing nfct_seqadj_ext_add() setup call\n"); >> + return 0; >> + } >> + > > Not sure this WARN() is really needed, I would remove it (since its most > likely only missing due to memory shortage). > > Other than that, this looks good. > I prefer to keep the warning, although it only happens caused by mem shortage now. But it would give the accurate description, if the nfct_seqadj_ext_add was lost when new features were depending on synadj. Best Regards Feng -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/netfilter/nf_conntrack_seqadj.c b/net/netfilter/nf_conntrack_seqadj.c index dff0f0c..2c8e201 100644 --- a/net/netfilter/nf_conntrack_seqadj.c +++ b/net/netfilter/nf_conntrack_seqadj.c @@ -16,9 +16,14 @@ int nf_ct_seqadj_init(struct nf_conn *ct, enum ip_conntrack_info ctinfo, if (off == 0) return 0; + seqadj = nfct_seqadj(ct); + if (unlikely(!seqadj)) { + WARN_ONCE(1, "Missing nfct_seqadj_ext_add() setup call\n"); + return 0; + } + set_bit(IPS_SEQ_ADJUST_BIT, &ct->status); - seqadj = nfct_seqadj(ct); this_way = &seqadj->seq[dir]; this_way->offset_before = off; this_way->offset_after = off;