Re: [PATCH v2 3/5] bpf: add helper masks for ADJ_ROOM flags and encap validation

[Hudson, Nick]

> On Mar 24, 2026, at 6:12 PM, Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote:
> 
> !-------------------------------------------------------------------|
> This Message Is From an External Sender
> This message came from outside your organization.
> |-------------------------------------------------------------------!
> 
> On 3/18/26 6:42 AM, Nick Hudson wrote:
>> Introduce helper masks for bpf_skb_adjust_room() flags to simplify
>> validation logic:
>> - BPF_F_ADJ_ROOM_DECAP_L4_MASK
>> - BPF_F_ADJ_ROOM_DECAP_IPXIP_MASK
>> - BPF_F_ADJ_ROOM_ENCAP_MASK
>> - BPF_F_ADJ_ROOM_DECAP_MASK
>> Add flag validation to bpf_skb_net_grow() to reject invalid encap
>> flags early. Refactor existing validation checks in bpf_skb_net_shrink()
>> and bpf_skb_adjust_room() to use the new masks (no behavior change).
>> Co-developed-by: Max Tottenham <mtottenh@xxxxxxxxxx>
>> Signed-off-by: Max Tottenham <mtottenh@xxxxxxxxxx>
>> Co-developed-by: Anna Glasgall <aglasgal@xxxxxxxxxx>
>> Signed-off-by: Anna Glasgall <aglasgal@xxxxxxxxxx>
>> Signed-off-by: Nick Hudson <nhudson@xxxxxxxxxx>
>> ---
>>  net/core/filter.c | 31 +++++++++++++++++++++++--------
>>  1 file changed, 23 insertions(+), 8 deletions(-)
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index 0d5d5a17acb2..7c2871b40fe4 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -3483,14 +3483,25 @@ static u32 bpf_skb_net_base_len(const struct sk_buff *skb)
>>  #define BPF_F_ADJ_ROOM_DECAP_L3_MASK (BPF_F_ADJ_ROOM_DECAP_L3_IPV4 | \
>>    BPF_F_ADJ_ROOM_DECAP_L3_IPV6)
>>  -#define BPF_F_ADJ_ROOM_MASK (BPF_F_ADJ_ROOM_FIXED_GSO | \
>> -  BPF_F_ADJ_ROOM_ENCAP_L3_MASK | \
>> +#define BPF_F_ADJ_ROOM_DECAP_L4_MASK (BPF_F_ADJ_ROOM_DECAP_L4_UDP | \
>> +  BPF_F_ADJ_ROOM_DECAP_L4_GRE)
>> +
>> +#define BPF_F_ADJ_ROOM_DECAP_IPXIP_MASK (BPF_F_ADJ_ROOM_DECAP_IPXIP4 | \
>> +  BPF_F_ADJ_ROOM_DECAP_IPXIP6)
>> +
>> +#define BPF_F_ADJ_ROOM_ENCAP_MASK (BPF_F_ADJ_ROOM_ENCAP_L3_MASK | \
>>    BPF_F_ADJ_ROOM_ENCAP_L4_GRE | \
>>    BPF_F_ADJ_ROOM_ENCAP_L4_UDP | \
>>    BPF_F_ADJ_ROOM_ENCAP_L2_ETH | \
>>    BPF_F_ADJ_ROOM_ENCAP_L2( \
>> -   BPF_ADJ_ROOM_ENCAP_L2_MASK) | \
>> -  BPF_F_ADJ_ROOM_DECAP_L3_MASK)
>> +   BPF_ADJ_ROOM_ENCAP_L2_MASK))
>> +
>> +#define BPF_F_ADJ_ROOM_DECAP_MASK (BPF_F_ADJ_ROOM_DECAP_L3_MASK)
>> +
>> +#define BPF_F_ADJ_ROOM_MASK (BPF_F_ADJ_ROOM_FIXED_GSO | \
>> +  BPF_F_ADJ_ROOM_ENCAP_MASK | \
>> +  BPF_F_ADJ_ROOM_DECAP_MASK | \
>> +  BPF_F_ADJ_ROOM_NO_CSUM_RESET)
> 
> The patch does two things: refactoring of existing macros (BPF_F_ADJ_ROOM_ENCAP_MASK, BPF_F_ADJ_ROOM_DECAP_MASK) and new additions (BPF_F_ADJ_ROOM_DECAP_L4_MASK, BPF_F_ADJ_ROOM_DECAP_IPXIP_MASK) that depend on the new flags from the UAPI changes in patch 2.
> 
> The refactoring does not depend on the new UAPI flags and could be a separate patch placed earlier in the series. That way a reviewer can verify it is a no-op without the new flag additions getting in
> the way. The (BPF_F_ADJ_ROOM_DECAP_L4_MASK, BPF_F_ADJ_ROOM_DECAP_IPXIP_MASK) can be introduced together in patch 4 when it is first used.

OK, will split further.

> 
>>    static int bpf_skb_net_grow(struct sk_buff *skb, u32 off, u32 len_diff,
>>       u64 flags)
>> @@ -3502,6 +3513,11 @@ static int bpf_skb_net_grow(struct sk_buff *skb, u32 off, u32 len_diff,
>>   unsigned int gso_type = SKB_GSO_DODGY;
>>   int ret;
>>  + if (unlikely(flags & ~(BPF_F_ADJ_ROOM_ENCAP_MASK |
>> +        BPF_F_ADJ_ROOM_NO_CSUM_RESET |
>> +        BPF_F_ADJ_ROOM_FIXED_GSO)))
> 
> Under which case this new check will be hit?

If a user supplies +ve len_diff and attempts to pass a DECAP flag.

The commit message had

    Add flag validation to bpf_skb_net_grow() to reject invalid encap
    flags early.

> 
>> + return -EINVAL;
>> +
>>   if (skb_is_gso(skb) && !skb_is_gso_tcp(skb)) {
>>   /* udp gso_size delineates datagrams, only allow if fixed */
>>   if (!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4) ||
>> @@ -3611,8 +3627,8 @@ static int bpf_skb_net_shrink(struct sk_buff *skb, u32 off, u32 len_diff,
>>  {
>>   int ret;
>>  - if (unlikely(flags & ~(BPF_F_ADJ_ROOM_FIXED_GSO |
>> -        BPF_F_ADJ_ROOM_DECAP_L3_MASK |
>> + if (unlikely(flags & ~(BPF_F_ADJ_ROOM_DECAP_MASK |
>> +        BPF_F_ADJ_ROOM_FIXED_GSO |
>>          BPF_F_ADJ_ROOM_NO_CSUM_RESET)))
>>   return -EINVAL;
>>  @@ -3708,8 +3724,7 @@ BPF_CALL_4(bpf_skb_adjust_room, struct sk_buff *, skb, s32, len_diff,
>>   u32 off;
>>   int ret;
>>  - if (unlikely(flags & ~(BPF_F_ADJ_ROOM_MASK |
>> -        BPF_F_ADJ_ROOM_NO_CSUM_RESET)))
>> + if (unlikely(flags & ~BPF_F_ADJ_ROOM_MASK))
>>   return -EINVAL;
>>   if (unlikely(len_diff_abs > 0xfffU))
>>   return -EFAULT;

Attachment: smime.p7s
Description: S/MIME cryptographic signature