On Tue, Dec 5, 2023 at 5:34 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > 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? nope :) this will turn into another retval patch set story. Feel free to follow up if you care enough about this, though! > > > 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.
