On 04/01/2019 07:23 PM, Alexei Starovoitov wrote: > On 4/1/19 9:09 AM, Daniel Borkmann wrote: >> On 03/29/2019 08:10 PM, Alexei Starovoitov wrote: >>> On Thu, Mar 28, 2019 at 6:02 PM Andrey Ignatov <r...@fb.com> wrote: >>>> >>>> The patch set adds support for stack access with variable offset from >>>> helpers. >>>> >>>> Patch 1 is the main patch in the set and provides more details. >>>> Patch 2 adds selftests for new functionality. >>> >>> Applied. Thanks >> >> Hmm, I think this series needs more work unfortunately. The selftests are >> only >> checking root-only programs, which is way to little. For !root we do the >> spectre >> masking for map and stack ALU, and that hasn't been adapted here, so it will >> generate a wrong masking for runtime since it doesn't take variable part into >> account. Andrey, please take a look. > > right. may be we should allow this for root only then?
Probably yeah, though thinking more about it, what about the case where we pass in raw (uninitialized) buffers from stack into a helper? Our assumption has been thus far that given the size is const, we can mark them in verifier as initialized after the call (as helpers memset it on error). With variable access it could be within a given range from verification side, but at runtime it's concrete value, meaning, upon function return we could leak uninitialized stack where verifier thinks it has been initialized by the helper. I think the set doesn't address this either, unfortunately. (So would need to be restricted to helpers where we pass always initialized buffers into it.) Thanks, Daniel
