On Mon, 2023-12-04 at 19:56 -0800, Andrii Nakryiko wrote: [...] > > > So it makes me feel like the intent was to reject any partial writes > > > with spilled reg slots. We could probably improve that to just make > > > sure that we don't turn spilled pointers into STACK_MISC in unpriv, > > > but I'm not sure if it's worth doing that instead of keeping things > > > simple? > > > > You mean like below? > > > > if (!env->allow_ptr_leaks && > > is_spilled_reg(&state->stack[spi]) && > > is_spillable_regtype(state->stack[spi].spilled_ptr.type) && > > Honestly, I wouldn't trust is_spillable_regtype() the way it's > written, it's too easy to forget to add a new register type to the > list. I think the only "safe to spill" register is probably > SCALAR_VALUE, so I'd just do `type != SCALAR_VALUE`. > > But yes, I think that's the right approach. 'type != SCALAR_VALUE' makes sense as well. Do you plan to add this check as a part of current patch? > If we were being pedantic, though, we'd need to take into account > offset and see if [offset, offset + size) overlaps with any > STACK_SPILL/STACK_DYNPTR/STACK_ITER slots. > > But tbh, given it's unpriv programs we are talking about, I probably > wouldn't bother extending this logic too much. Yes, that's definitely is an ommission.
