Currently, the verifier doesn't reject unaligned access for map_value_adj register types. Commit 484611357c19 ("bpf: allow access into map value arrays") added logic to check_ptr_alignment() extending it from PTR_TO_PACKET to also PTR_TO_MAP_VALUE_ADJ, but for PTR_TO_MAP_VALUE_ADJ no enforcement is in place, because reg->id for PTR_TO_MAP_VALUE_ADJ reg types is never non-zero, meaning, we can cause BPF_H/_W/_DW-based unaligned access for architectures not supporting efficient unaligned access, and thus worst case could raise exceptions on some archs that are unable to correct the unaligned access or perform a different memory access to the actual requested one and such. i) Unaligned load with !CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS on r0 (map_value_adj): 0: (bf) r2 = r10 1: (07) r2 += -8 2: (7a) *(u64 *)(r2 +0) = 0 3: (18) r1 = 0x42533a00 5: (85) call bpf_map_lookup_elem#1 6: (15) if r0 == 0x0 goto pc+11 R0=map_value(ks=8,vs=48,id=0),min_value=0,max_value=0 R10=fp 7: (61) r1 = *(u32 *)(r0 +0) 8: (35) if r1 >= 0xb goto pc+9 R0=map_value(ks=8,vs=48,id=0),min_value=0,max_value=0 R1=inv,min_value=0,max_value=10 R10=fp 9: (07) r0 += 3 10: (79) r7 = *(u64 *)(r0 +0) R0=map_value_adj(ks=8,vs=48,id=0),min_value=3,max_value=3 R1=inv,min_value=0,max_value=10 R10=fp 11: (79) r7 = *(u64 *)(r0 +2) R0=map_value_adj(ks=8,vs=48,id=0),min_value=3,max_value=3 R1=inv,min_value=0,max_value=10 R7=inv R10=fp [...] ii) Unaligned store with !CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS on r0 (map_value_adj): 0: (bf) r2 = r10 1: (07) r2 += -8 2: (7a) *(u64 *)(r2 +0) = 0 3: (18) r1 = 0x4df16a00 5: (85) call bpf_map_lookup_elem#1 6: (15) if r0 == 0x0 goto pc+19 R0=map_value(ks=8,vs=48,id=0),min_value=0,max_value=0 R10=fp 7: (07) r0 += 3 8: (7a) *(u64 *)(r0 +0) = 42 R0=map_value_adj(ks=8,vs=48,id=0),min_value=3,max_value=3 R10=fp 9: (7a) *(u64 *)(r0 +2) = 43 R0=map_value_adj(ks=8,vs=48,id=0),min_value=3,max_value=3 R10=fp 10: (7a) *(u64 *)(r0 -2) = 44 R0=map_value_adj(ks=8,vs=48,id=0),min_value=3,max_value=3 R10=fp [...] For the PTR_TO_PACKET type, reg->id is initially zero when skb->data was fetched, it later receives a reg->id from env->id_gen generator once another register with UNKNOWN_VALUE type was added to it via check_packet_ptr_add(). The purpose of this reg->id is twofold: i) it is used in find_good_pkt_pointers() for setting the allowed access range for regs with PTR_TO_PACKET of same id once verifier matched on data/data_end tests, and ii) for check_ptr_alignment() to determine that when not having efficient unaligned access and register with UNKNOWN_VALUE was added to PTR_TO_PACKET, that we're only allowed to access the content bytewise due to unknown unalignment. reg->id was never intended for PTR_TO_MAP_VALUE{,_ADJ} types and thus is always zero, the only marking is in PTR_TO_MAP_VALUE_OR_NULL that was added after 484611357c19 via 57a09bf0a416 ("bpf: Detect identical PTR_TO_MAP_VALUE_OR_NULL registers"). Above tests will fail for non-root environment due to prohibited pointer arithmetic. The fix splits register-type specific checks into their own helper instead of keeping them combined, so we don't run into a similar issue in future once we extend check_ptr_alignment() further and forget to add reg->type checks for some of the checks.
Unified · Split
| @@ -765,38 +765,56 @@ static bool is_pointer_value(struct bpf_verifier_env *env, int regno) · 764 unmodified lines | ||
| 765 | 765 | } |
| 766 | 766 | } |
| 767 | 767 | |
| 768 | −static int check_ptr_alignment(struct bpf_verifier_env *env, | |
| 769 | − struct bpf_reg_state *reg, int off, int size) | |
| 768 | +static int check_pkt_ptr_alignment(const struct bpf_reg_state *reg, | |
| 769 | + int off, int size) | |
| 770 | 770 | { |
| 771 | − if (reg->type != PTR_TO_PACKET && reg->type != PTR_TO_MAP_VALUE_ADJ) { | |
| 772 | − if (off % size != 0) { | |
| 773 | − verbose("misaligned access off %d size %d\n", | |
| 774 | − off, size); | |
| 775 | − return -EACCES; | |
| 776 | − } else { | |
| 777 | − return 0; | |
| 778 | − } | |
| 779 | − } | |
| 780 | − | |
| 781 | − if (IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)) | |
| 782 | − /* misaligned access to packet is ok on x86,arm,arm64 */ | |
| 783 | − return 0; | |
| 784 | − | |
| 785 | 771 | if (reg->id && size != 1) { |
| 786 | − verbose("Unknown packet alignment. Only byte-sized access allowed\n"); | |
| 772 | + verbose("Unknown alignment. Only byte-sized access allowed in packet access.\n"); | |
| 787 | 773 | return -EACCES; |
| 788 | 774 | } |
| 789 | 775 | |
| 790 | 776 | /* skb->data is NET_IP_ALIGN-ed */ |
| 791 | − if (reg->type == PTR_TO_PACKET && | |
| 792 | − (NET_IP_ALIGN + reg->off + off) % size != 0) { | |
| 777 | + if ((NET_IP_ALIGN + reg->off + off) % size != 0) { | |
| 793 | 778 | verbose("misaligned packet access off %d+%d+%d size %d\n", |
| 794 | 779 | NET_IP_ALIGN, reg->off, off, size); |
| 795 | 780 | return -EACCES; |
| 796 | 781 | } |
| 782 | + | |
| 797 | 783 | return 0; |
| 798 | 784 | } |
| 799 | 785 | |
| 786 | +static int check_val_ptr_alignment(const struct bpf_reg_state *reg, | |
| 787 | + int size) | |
| 788 | +{ | |
| 789 | + if (size != 1) { | |
| 790 | + verbose("Unknown alignment. Only byte-sized access allowed in value access.\n"); | |
| 791 | + return -EACCES; | |
| 792 | + } | |
| 793 | + | |
| 794 | + return 0; | |
| 795 | +} | |
| 796 | + | |
| 797 | +static int check_ptr_alignment(const struct bpf_reg_state *reg, | |
| 798 | + int off, int size) | |
| 799 | +{ | |
| 800 | + switch (reg->type) { | |
| 801 | + case PTR_TO_PACKET: | |
| 802 | + return IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) ? 0 : | |
| 803 | + check_pkt_ptr_alignment(reg, off, size); | |
| 804 | + case PTR_TO_MAP_VALUE_ADJ: | |
| 805 | + return IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) ? 0 : | |
| 806 | + check_val_ptr_alignment(reg, size); | |
| 807 | + default: | |
| 808 | + if (off % size != 0) { | |
| 809 | + verbose("misaligned access off %d size %d\n", | |
| 810 | + off, size); | |
| 811 | + return -EACCES; | |
| 812 | + } | |
| 813 | + | |
| 814 | + return 0; | |
| 815 | + } | |
| 816 | +} | |
| 817 | + | |
| 800 | 818 | /* check whether memory at (regno + off) is accessible for t = (read | write) |
| 801 | 819 | * if t==write, value_regno is a register which value is stored into memory |
| 802 | 820 | * if t==read, value_regno is a register which will receive the value from memory |
| @@ -818,7 +836,7 @@ static int check_mem_access(struct bpf_verifier_env *env, u32 regno, int off, · 15 unmodified lines | ||
| 818 | 836 | if (size < 0) |
| 819 | 837 | return size; |
| 820 | 838 | |
| 821 | − err = check_ptr_alignment(env, reg, off, size); | |
| 839 | + err = check_ptr_alignment(reg, off, size); | |
| 822 | 840 | if (err) |
| 823 | 841 | return err; |
| 824 | 842 | |