For a MEM_UNINIT ("raw mode") helper argument, check_stack_range_initialized() open-coded a scan that rejected any STACK_DYNPTR slot in the range with "potential write to dynptr". This duplicated, and was stricter than, the handling that runs when the buffer is actually marked initialized. check_helper_call() later replays the write byte by byte via check_mem_access(), which goes through destroy_if_dynptr_stack_slot(), which rejects overwritting a referenced dynptr. Therefore drop the redundant scan and rely on check_mem_access().
Unified · Split
| @@ -6733,31 +6733,6 @@ static int check_stack_range_initialized( · 6732 unmodified lines | |||
| 6733 | } | 6733 | } |
| 6734 | 6734 | ||
| 6735 | if (meta && meta->raw_mode) { | 6735 | if (meta && meta->raw_mode) { |
| 6736 | /* Ensure we won't be overwriting dynptrs when simulating byte | ||
| 6737 | * by byte access in check_helper_call using meta.access_size. | ||
| 6738 | * This would be a problem if we have a helper in the future | ||
| 6739 | * which takes: | ||
| 6740 | * | ||
| 6741 | * helper(uninit_mem, len, dynptr) | ||
| 6742 | * | ||
| 6743 | * Now, uninint_mem may overlap with dynptr pointer. Hence, it | ||
| 6744 | * may end up writing to dynptr itself when touching memory from | ||
| 6745 | * arg 1. This can be relaxed on a case by case basis for known | ||
| 6746 | * safe cases, but reject due to the possibilitiy of aliasing by | ||
| 6747 | * default. | ||
| 6748 | */ | ||
| 6749 | for (i = min_off; i < max_off + access_size; i++) { | ||
| 6750 | int stack_off = -i - 1; | ||
| 6751 | |||
| 6752 | spi = bpf_get_spi(i); | ||
| 6753 | /* raw_mode may write past allocated_stack */ | ||
| 6754 | if (state->allocated_stack <= stack_off) | ||
| 6755 | continue; | ||
| 6756 | if (state->stack[spi].slot_type[stack_off % BPF_REG_SIZE] == STACK_DYNPTR) { | ||
| 6757 | verbose(env, "potential write to dynptr at off=%d disallowed\n", i); | ||
| 6758 | return -EACCES; | ||
| 6759 | } | ||
| 6760 | } | ||
| 6761 | meta->access_size = access_size; | 6736 | meta->access_size = access_size; |
| 6762 | meta->regno = reg_from_argno(argno); | 6737 | meta->regno = reg_from_argno(argno); |
| 6763 | return 0; | 6738 | return 0; |
| @@ -1112,7 +1112,7 @@ int dynptr_overwrite_ref(void *ctx) · 1111 unmodified lines | |||
| 1112 | 1112 | ||
| 1113 | /* Reject writes to dynptr slot from bpf_dynptr_read */ | 1113 | /* Reject writes to dynptr slot from bpf_dynptr_read */ |
| 1114 | SEC("?raw_tp") | 1114 | SEC("?raw_tp") |
| 1115 | __failure __msg("potential write to dynptr at off=-16") | 1115 | __failure __msg("cannot overwrite referenced dynptr") |
| 1116 | int dynptr_read_into_slot(void *ctx) | 1116 | int dynptr_read_into_slot(void *ctx) |
| 1117 | { | 1117 | { |
| 1118 | union { | 1118 | union { |
| @@ -1558,7 +1558,7 @@ int BPF_PROG(skb_invalid_ctx_fexit, void *skb) · 439 unmodified lines | |||
| 1558 | 1558 | ||
| 1559 | /* Reject writes to dynptr slot for uninit arg */ | 1559 | /* Reject writes to dynptr slot for uninit arg */ |
| 1560 | SEC("?raw_tp") | 1560 | SEC("?raw_tp") |
| 1561 | __failure __msg("potential write to dynptr at off=-16") | 1561 | __failure __msg("cannot overwrite referenced dynptr") |
| 1562 | int uninit_write_into_slot(void *ctx) | 1562 | int uninit_write_into_slot(void *ctx) |
| 1563 | { | 1563 | { |
| 1564 | struct { | 1564 | struct { |