Skip to content

Commit

Permalink
netfilter: remove BUG_ON() after skb_header_pointer()
Browse files Browse the repository at this point in the history
Several conntrack helpers and the TCP tracker assume that
skb_header_pointer() never fails based on upfront header validation.
Even if this should not ever happen, BUG_ON() is a too drastic measure,
remove them.

Signed-off-by: Pablo Neira Ayuso <[email protected]>
  • Loading branch information
ummakynes committed May 5, 2021
1 parent 5e024c3 commit 198ad97
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 7 deletions.
5 changes: 4 additions & 1 deletion net/netfilter/nf_conntrack_ftp.c
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,10 @@ static int help(struct sk_buff *skb,

spin_lock_bh(&nf_ftp_lock);
fb_ptr = skb_header_pointer(skb, dataoff, datalen, ftp_buffer);
BUG_ON(fb_ptr == NULL);
if (!fb_ptr) {
spin_unlock_bh(&nf_ftp_lock);
return NF_ACCEPT;
}

ends_in_nl = (fb_ptr[datalen - 1] == '\n');
seq = ntohl(th->seq) + datalen;
Expand Down
3 changes: 2 additions & 1 deletion net/netfilter/nf_conntrack_h323_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ static int get_tpkt_data(struct sk_buff *skb, unsigned int protoff,
/* Get first TPKT pointer */
tpkt = skb_header_pointer(skb, tcpdataoff, tcpdatalen,
h323_buffer);
BUG_ON(tpkt == NULL);
if (!tpkt)
goto clear_out;

/* Validate TPKT identifier */
if (tcpdatalen < 4 || tpkt[0] != 0x03 || tpkt[1] != 0) {
Expand Down
5 changes: 4 additions & 1 deletion net/netfilter/nf_conntrack_irc.c
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,10 @@ static int help(struct sk_buff *skb, unsigned int protoff,
spin_lock_bh(&irc_buffer_lock);
ib_ptr = skb_header_pointer(skb, dataoff, skb->len - dataoff,
irc_buffer);
BUG_ON(ib_ptr == NULL);
if (!ib_ptr) {
spin_unlock_bh(&irc_buffer_lock);
return NF_ACCEPT;
}

data = ib_ptr;
data_limit = ib_ptr + skb->len - dataoff;
Expand Down
4 changes: 3 additions & 1 deletion net/netfilter/nf_conntrack_pptp.c
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,9 @@ conntrack_pptp_help(struct sk_buff *skb, unsigned int protoff,

nexthdr_off = protoff;
tcph = skb_header_pointer(skb, nexthdr_off, sizeof(_tcph), &_tcph);
BUG_ON(!tcph);
if (!tcph)
return NF_ACCEPT;

nexthdr_off += tcph->doff * 4;
datalen = tcplen - tcph->doff * 4;

Expand Down
6 changes: 4 additions & 2 deletions net/netfilter/nf_conntrack_proto_tcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,8 @@ static void tcp_options(const struct sk_buff *skb,

ptr = skb_header_pointer(skb, dataoff + sizeof(struct tcphdr),
length, buff);
BUG_ON(ptr == NULL);
if (!ptr)
return;

state->td_scale =
state->flags = 0;
Expand Down Expand Up @@ -394,7 +395,8 @@ static void tcp_sack(const struct sk_buff *skb, unsigned int dataoff,

ptr = skb_header_pointer(skb, dataoff + sizeof(struct tcphdr),
length, buff);
BUG_ON(ptr == NULL);
if (!ptr)
return;

/* Fast path for timestamp-only option */
if (length == TCPOLEN_TSTAMP_ALIGNED
Expand Down
5 changes: 4 additions & 1 deletion net/netfilter/nf_conntrack_sane.c
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,10 @@ static int help(struct sk_buff *skb,

spin_lock_bh(&nf_sane_lock);
sb_ptr = skb_header_pointer(skb, dataoff, datalen, sane_buffer);
BUG_ON(sb_ptr == NULL);
if (!sb_ptr) {
spin_unlock_bh(&nf_sane_lock);
return NF_ACCEPT;
}

if (dir == IP_CT_DIR_ORIGINAL) {
if (datalen != sizeof(struct sane_request))
Expand Down

0 comments on commit 198ad97

Please sign in to comment.