Daniel Borkmann <daniel@xxxxxxxxxxxxx> [Wed, 2021-08-18 15:48 -0700]: > On 8/19/21 12:11 AM, Andrey Ignatov wrote: > > Fix a verifier bug found by smatch static checker in [0]. > > > > When narrow load is handled, one or two new instructions are added to > > insn_buf array, but before it was only checked that > > > > cnt >= ARRAY_SIZE(insn_buf) > > > > And it's safe to add a new instruction to insn_buf[cnt++] only once. The > > second try will lead to out of bound write. And this is what can happen > > if `shift` is set. > > Afaik, the insn_buf[] should always be large enough, could you add something to > the commit message of this fix whether this created an actual issue in practice > where we do oob overrun insn_buf[] (or whether it's 'only' the static checker > report ... from above paragraph I read you saw the former in practice)? It's 'only' the static checker report. I've never seen this problem in practice. I also have an impression that convert_ctx_accesses should not in general (or should never?) return too many insn to hit insn_buf[] limit, but it may not be trivial to check all scenarios so I can't say for sure. That's why it makes sense to me to address the report. Sure, I can add this info to the commit message in v2. -- Andrey Ignatov
