Messages in this thread | | | From | Kees Cook <> | Date | Wed, 18 Jul 2018 21:31:46 -0700 | Subject | Re: [PATCH] rxrpc: Reuse SKCIPHER_REQUEST_ON_STACK buffer |
| |
On Mon, Jul 16, 2018 at 10:42 AM, Eric Biggers <ebiggers@google.com> wrote: > On Sun, Jul 15, 2018 at 08:49:47PM -0700, Kees Cook wrote: >> The use of SKCIPHER_REQUEST_ON_STACK() will trigger FRAME_WARN warnings >> (when less than 2048) once the VLA is no longer hidden from the check: >> >> net/rxrpc/rxkad.c:398:1: warning: the frame size of 1152 bytes is larger than 1024 bytes [-Wframe-larger-than=] >> net/rxrpc/rxkad.c:242:1: warning: the frame size of 1152 bytes is larger than 1024 bytes [-Wframe-larger-than=] >> >> This passes the initial SKCIPHER_REQUEST_ON_STACK allocation to the leaf >> functions for reuse. Two requests allocated on the stack are not needed >> when only one is used at a time. >> >> Signed-off-by: Kees Cook <keescook@chromium.org> >> --- >> net/rxrpc/rxkad.c | 25 +++++++++++++------------ >> 1 file changed, 13 insertions(+), 12 deletions(-) >> [...] > How about doing the 'skcipher_request_set_tfm(req, call->conn->cipher)' and > 'skcipher_request_zero(req);' just once, in the top-level function > rxkad_verify_packet(), instead of before/after every time the request is used?
Hm. While that does sound reasonable, I like having it be operationally unchanged. And changing this makes the caller function a bit more weird. Right now it's doing direct returns, and we'd need to cover error paths, etc. Having the "zero" _right_ after the encrypt/decrypt seems more robust to me.
-Kees
-- Kees Cook Pixel Security
| |