| Message ID | CAGXu5jK7iRv1HE7JgW95vTgg5vhye4dxjfoQyN3G7HZzp7nZhA@mail.gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Mike Snitzer |
| Headers | show |
On Thu, Jul 12, 2018 at 10:30 PM, Kees Cook <keescook@chromium.org> wrote: > On Thu, Jul 12, 2018 at 1:23 PM, Kees Cook <keescook@chromium.org> wrote: >> On Thu, Jul 12, 2018 at 8:11 AM, Arnd Bergmann <arnd@arndb.de> wrote: >>> On Wed, Jul 11, 2018 at 10:36 PM, Kees Cook <keescook@chromium.org> wrote: >>>> Two uses 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 bumps the affected objects by 20% to silence the warnings while >>>> still providing coverage is anything grows even more. >>>> >>>> Signed-off-by: Kees Cook <keescook@chromium.org> >>> >>> (adding David Howells to cc) >>> >>> I don't think these are in a fast path, it should be possible to just use >>> skcipher_alloc_req() instead of SKCIPHER_REQUEST_ON_STACK() here. >>> From what I can tell, neither of the two are called in atomic context, so >>> you should be able to use a GFP_KERNEL allocation. >> >> Sure, I can do that instead. > > Actually, I think this can actually be adjusted to just re-use the > stack allocation, since rxkad_verify_packet() finishes one before > doing another in rxkad_verify_packet_1(): That looks very nice, yes. The same thing is needed in rxkad_secure_packet(), right? Arnd -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Thu, Jul 12, 2018 at 2:15 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Thu, Jul 12, 2018 at 10:30 PM, Kees Cook <keescook@chromium.org> wrote: >> Actually, I think this can actually be adjusted to just re-use the >> stack allocation, since rxkad_verify_packet() finishes one before >> doing another in rxkad_verify_packet_1(): > > That looks very nice, yes. The same thing is needed in > rxkad_secure_packet(), right? Yup. 4 leaf functions and the 2 callers. -Kees
diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c index 278ac0807a60..d6a2e7cab384 100644 --- a/net/rxrpc/rxkad.c +++ b/net/rxrpc/rxkad.c @@ -316,10 +316,10 @@ static int rxkad_secure_packet(struct rxrpc_call *call, */ static int rxkad_verify_packet_1(struct rxrpc_call *call, struct sk_buff *skb, unsigned int offset, unsigned int len, - rxrpc_seq_t seq) + rxrpc_seq_t seq, + struct skcipher_request *req) { struct rxkad_level1_hdr sechdr; - SKCIPHER_REQUEST_ON_STACK(req, call->conn->cipher); struct rxrpc_crypt iv; struct scatterlist sg[16]; struct sk_buff *trailer; @@ -549,7 +549,7 @@ static int rxkad_verify_packet(struct rxrpc_call *call, struct sk_buff *skb, case RXRPC_SECURITY_PLAIN: return 0; case RXRPC_SECURITY_AUTH: - return rxkad_verify_packet_1(call, skb, offset, len, seq); + return rxkad_verify_packet_1(call, skb, offset, len, seq, req); case RXRPC_SECURITY_ENCRYPT: return rxkad_verify_packet_2(call, skb, offset, len, seq);