Skip to content

Commit d02f51c

Browse files
daxtensborkmann
authored andcommitted
bpf: fix bpf_skb_adjust_net/bpf_skb_proto_xlat to deal with gso sctp skbs
SCTP GSO skbs have a gso_size of GSO_BY_FRAGS, so any sort of unconditionally mangling of that will result in nonsense value and would corrupt the skb later on. Therefore, i) add two helpers skb_increase_gso_size() and skb_decrease_gso_size() that would throw a one time warning and bail out for such skbs and ii) refuse and return early with an error in those BPF helpers that are affected. We do need to bail out as early as possible from there before any changes on the skb have been performed. Fixes: 6578171 ("bpf: add bpf_skb_change_proto helper") Co-authored-by: Daniel Borkmann <daniel@iogearbox.net> Signed-off-by: Daniel Axtens <dja@axtens.net> Cc: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Acked-by: Alexei Starovoitov <ast@kernel.org> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
1 parent 4a0c719 commit d02f51c

File tree

3 files changed

+73
-20
lines changed

3 files changed

+73
-20
lines changed

Documentation/networking/segmentation-offloads.txt

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,15 @@ To signal this, gso_size is set to the special value GSO_BY_FRAGS.
153153

154154
Therefore, any code in the core networking stack must be aware of the
155155
possibility that gso_size will be GSO_BY_FRAGS and handle that case
156-
appropriately. (For size checks, the skb_gso_validate_*_len family of
157-
helpers do this automatically.)
156+
appropriately.
157+
158+
There are a couple of helpers to make this easier:
159+
160+
- For size checks, the skb_gso_validate_*_len family of helpers correctly
161+
considers GSO_BY_FRAGS.
162+
163+
- For manipulating packets, skb_increase_gso_size and skb_decrease_gso_size
164+
will check for GSO_BY_FRAGS and WARN if asked to manipulate these skbs.
158165

159166
This also affects drivers with the NETIF_F_FRAGLIST & NETIF_F_GSO_SCTP bits
160167
set. Note also that NETIF_F_GSO_SCTP is included in NETIF_F_GSO_SOFTWARE.

include/linux/skbuff.h

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4038,13 +4038,35 @@ static inline bool skb_is_gso_v6(const struct sk_buff *skb)
40384038
return skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6;
40394039
}
40404040

4041+
/* Note: Should be called only if skb_is_gso(skb) is true */
4042+
static inline bool skb_is_gso_sctp(const struct sk_buff *skb)
4043+
{
4044+
return skb_shinfo(skb)->gso_type & SKB_GSO_SCTP;
4045+
}
4046+
40414047
static inline void skb_gso_reset(struct sk_buff *skb)
40424048
{
40434049
skb_shinfo(skb)->gso_size = 0;
40444050
skb_shinfo(skb)->gso_segs = 0;
40454051
skb_shinfo(skb)->gso_type = 0;
40464052
}
40474053

4054+
static inline void skb_increase_gso_size(struct skb_shared_info *shinfo,
4055+
u16 increment)
4056+
{
4057+
if (WARN_ON_ONCE(shinfo->gso_size == GSO_BY_FRAGS))
4058+
return;
4059+
shinfo->gso_size += increment;
4060+
}
4061+
4062+
static inline void skb_decrease_gso_size(struct skb_shared_info *shinfo,
4063+
u16 decrement)
4064+
{
4065+
if (WARN_ON_ONCE(shinfo->gso_size == GSO_BY_FRAGS))
4066+
return;
4067+
shinfo->gso_size -= decrement;
4068+
}
4069+
40484070
void __skb_warn_lro_forwarding(const struct sk_buff *skb);
40494071

40504072
static inline bool skb_warn_if_lro(const struct sk_buff *skb)

net/core/filter.c

Lines changed: 42 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2087,6 +2087,10 @@ static int bpf_skb_proto_4_to_6(struct sk_buff *skb)
20872087
u32 off = skb_mac_header_len(skb);
20882088
int ret;
20892089

2090+
/* SCTP uses GSO_BY_FRAGS, thus cannot adjust it. */
2091+
if (skb_is_gso(skb) && unlikely(skb_is_gso_sctp(skb)))
2092+
return -ENOTSUPP;
2093+
20902094
ret = skb_cow(skb, len_diff);
20912095
if (unlikely(ret < 0))
20922096
return ret;
@@ -2096,19 +2100,21 @@ static int bpf_skb_proto_4_to_6(struct sk_buff *skb)
20962100
return ret;
20972101

20982102
if (skb_is_gso(skb)) {
2103+
struct skb_shared_info *shinfo = skb_shinfo(skb);
2104+
20992105
/* SKB_GSO_TCPV4 needs to be changed into
21002106
* SKB_GSO_TCPV6.
21012107
*/
2102-
if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4) {
2103-
skb_shinfo(skb)->gso_type &= ~SKB_GSO_TCPV4;
2104-
skb_shinfo(skb)->gso_type |= SKB_GSO_TCPV6;
2108+
if (shinfo->gso_type & SKB_GSO_TCPV4) {
2109+
shinfo->gso_type &= ~SKB_GSO_TCPV4;
2110+
shinfo->gso_type |= SKB_GSO_TCPV6;
21052111
}
21062112

21072113
/* Due to IPv6 header, MSS needs to be downgraded. */
2108-
skb_shinfo(skb)->gso_size -= len_diff;
2114+
skb_decrease_gso_size(shinfo, len_diff);
21092115
/* Header must be checked, and gso_segs recomputed. */
2110-
skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY;
2111-
skb_shinfo(skb)->gso_segs = 0;
2116+
shinfo->gso_type |= SKB_GSO_DODGY;
2117+
shinfo->gso_segs = 0;
21122118
}
21132119

21142120
skb->protocol = htons(ETH_P_IPV6);
@@ -2123,6 +2129,10 @@ static int bpf_skb_proto_6_to_4(struct sk_buff *skb)
21232129
u32 off = skb_mac_header_len(skb);
21242130
int ret;
21252131

2132+
/* SCTP uses GSO_BY_FRAGS, thus cannot adjust it. */
2133+
if (skb_is_gso(skb) && unlikely(skb_is_gso_sctp(skb)))
2134+
return -ENOTSUPP;
2135+
21262136
ret = skb_unclone(skb, GFP_ATOMIC);
21272137
if (unlikely(ret < 0))
21282138
return ret;
@@ -2132,19 +2142,21 @@ static int bpf_skb_proto_6_to_4(struct sk_buff *skb)
21322142
return ret;
21332143

21342144
if (skb_is_gso(skb)) {
2145+
struct skb_shared_info *shinfo = skb_shinfo(skb);
2146+
21352147
/* SKB_GSO_TCPV6 needs to be changed into
21362148
* SKB_GSO_TCPV4.
21372149
*/
2138-
if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6) {
2139-
skb_shinfo(skb)->gso_type &= ~SKB_GSO_TCPV6;
2140-
skb_shinfo(skb)->gso_type |= SKB_GSO_TCPV4;
2150+
if (shinfo->gso_type & SKB_GSO_TCPV6) {
2151+
shinfo->gso_type &= ~SKB_GSO_TCPV6;
2152+
shinfo->gso_type |= SKB_GSO_TCPV4;
21412153
}
21422154

21432155
/* Due to IPv4 header, MSS can be upgraded. */
2144-
skb_shinfo(skb)->gso_size += len_diff;
2156+
skb_increase_gso_size(shinfo, len_diff);
21452157
/* Header must be checked, and gso_segs recomputed. */
2146-
skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY;
2147-
skb_shinfo(skb)->gso_segs = 0;
2158+
shinfo->gso_type |= SKB_GSO_DODGY;
2159+
shinfo->gso_segs = 0;
21482160
}
21492161

21502162
skb->protocol = htons(ETH_P_IP);
@@ -2243,6 +2255,10 @@ static int bpf_skb_net_grow(struct sk_buff *skb, u32 len_diff)
22432255
u32 off = skb_mac_header_len(skb) + bpf_skb_net_base_len(skb);
22442256
int ret;
22452257

2258+
/* SCTP uses GSO_BY_FRAGS, thus cannot adjust it. */
2259+
if (skb_is_gso(skb) && unlikely(skb_is_gso_sctp(skb)))
2260+
return -ENOTSUPP;
2261+
22462262
ret = skb_cow(skb, len_diff);
22472263
if (unlikely(ret < 0))
22482264
return ret;
@@ -2252,11 +2268,13 @@ static int bpf_skb_net_grow(struct sk_buff *skb, u32 len_diff)
22522268
return ret;
22532269

22542270
if (skb_is_gso(skb)) {
2271+
struct skb_shared_info *shinfo = skb_shinfo(skb);
2272+
22552273
/* Due to header grow, MSS needs to be downgraded. */
2256-
skb_shinfo(skb)->gso_size -= len_diff;
2274+
skb_decrease_gso_size(shinfo, len_diff);
22572275
/* Header must be checked, and gso_segs recomputed. */
2258-
skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY;
2259-
skb_shinfo(skb)->gso_segs = 0;
2276+
shinfo->gso_type |= SKB_GSO_DODGY;
2277+
shinfo->gso_segs = 0;
22602278
}
22612279

22622280
return 0;
@@ -2267,6 +2285,10 @@ static int bpf_skb_net_shrink(struct sk_buff *skb, u32 len_diff)
22672285
u32 off = skb_mac_header_len(skb) + bpf_skb_net_base_len(skb);
22682286
int ret;
22692287

2288+
/* SCTP uses GSO_BY_FRAGS, thus cannot adjust it. */
2289+
if (skb_is_gso(skb) && unlikely(skb_is_gso_sctp(skb)))
2290+
return -ENOTSUPP;
2291+
22702292
ret = skb_unclone(skb, GFP_ATOMIC);
22712293
if (unlikely(ret < 0))
22722294
return ret;
@@ -2276,11 +2298,13 @@ static int bpf_skb_net_shrink(struct sk_buff *skb, u32 len_diff)
22762298
return ret;
22772299

22782300
if (skb_is_gso(skb)) {
2301+
struct skb_shared_info *shinfo = skb_shinfo(skb);
2302+
22792303
/* Due to header shrink, MSS can be upgraded. */
2280-
skb_shinfo(skb)->gso_size += len_diff;
2304+
skb_increase_gso_size(shinfo, len_diff);
22812305
/* Header must be checked, and gso_segs recomputed. */
2282-
skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY;
2283-
skb_shinfo(skb)->gso_segs = 0;
2306+
shinfo->gso_type |= SKB_GSO_DODGY;
2307+
shinfo->gso_segs = 0;
22842308
}
22852309

22862310
return 0;

0 commit comments

Comments
 (0)