diff mbox series

[RFC,bpf-next,01/11] bpf: Pull repeated reg access bounds check into helper fn

Message ID 20220722183438.3319790-2-davemarchevsky@fb.com (mailing list archive)
State RFC
Delegated to: BPF
Headers show
Series bpf: Introduce rbtree map | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR fail merge-conflict
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/apply fail Patch does not apply to bpf-next

Commit Message

Dave Marchevsky July 22, 2022, 6:34 p.m. UTC
Verifier's check_map_access function has some duplicated bounds checking
arithmetic to determine whether a map element access might touch a
protected field (bpf_spin_lock and bpf_timer). Pull this logic out into
new reg_access_may_touch_field helper function to improve readability
and understandability of the code.

Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>
---
 kernel/bpf/verifier.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)
diff mbox series

Patch

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 8ceccc420db3..c5fbaa9f025a 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3783,6 +3783,19 @@  static int check_map_kptr_access(struct bpf_verifier_env *env, u32 regno,
 	return 0;
 }
 
+/* if any part of struct field can be touched by
+ * load/store reject this program.
+ * To check that [x1, x2) overlaps with [y1, y2)
+ * it is sufficient to check x1 < y2 && y1 < x2.
+ */
+static bool reg_access_may_touch_field(const struct bpf_reg_state *reg,
+				       u32 access_off, size_t access_sz,
+				       u32 field_off, size_t field_sz)
+{
+	return reg->smin_value + access_off < field_off + field_sz &&
+			field_off < reg->umax_value + access_off + access_sz;
+}
+
 /* check read/write into a map element with possible variable offset */
 static int check_map_access(struct bpf_verifier_env *env, u32 regno,
 			    int off, int size, bool zero_size_allowed,
@@ -3800,15 +3813,9 @@  static int check_map_access(struct bpf_verifier_env *env, u32 regno,
 		return err;
 
 	if (map_value_has_spin_lock(map)) {
-		u32 lock = map->spin_lock_off;
+		u32 t = map->spin_lock_off;
 
-		/* if any part of struct bpf_spin_lock can be touched by
-		 * load/store reject this program.
-		 * To check that [x1, x2) overlaps with [y1, y2)
-		 * it is sufficient to check x1 < y2 && y1 < x2.
-		 */
-		if (reg->smin_value + off < lock + sizeof(struct bpf_spin_lock) &&
-		     lock < reg->umax_value + off + size) {
+		if (reg_access_may_touch_field(reg, off, size, t, sizeof(struct bpf_spin_lock))) {
 			verbose(env, "bpf_spin_lock cannot be accessed directly by load/store\n");
 			return -EACCES;
 		}
@@ -3816,8 +3823,7 @@  static int check_map_access(struct bpf_verifier_env *env, u32 regno,
 	if (map_value_has_timer(map)) {
 		u32 t = map->timer_off;
 
-		if (reg->smin_value + off < t + sizeof(struct bpf_timer) &&
-		     t < reg->umax_value + off + size) {
+		if (reg_access_may_touch_field(reg, off, size, t, sizeof(struct bpf_timer))) {
 			verbose(env, "bpf_timer cannot be accessed directly by load/store\n");
 			return -EACCES;
 		}