From: Eric Biggers <ebiggers@google•com>
To: Kees Cook <keescook@chromium•org>
Cc: David Howells <dhowells@redhat•com>,
Herbert Xu <herbert@gondor•apana.org.au>,
Arnd Bergmann <arnd@arndb•de>,
"Gustavo A. R. Silva" <gustavo@embeddedor•com>,
"David S. Miller" <davem@davemloft•net>,
netdev@vger•kernel.org, linux-kernel@vger•kernel.org
Subject: Re: [PATCH] rxrpc: Reuse SKCIPHER_REQUEST_ON_STACK buffer
Date: Mon, 16 Jul 2018 10:42:23 -0700 [thread overview]
Message-ID: <20180716174223.GE77258@google.com> (raw)
In-Reply-To: <20180716034947.GA32022@beast>
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(-)
>
> diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c
> index 278ac0807a60..6393391fac86 100644
> --- a/net/rxrpc/rxkad.c
> +++ b/net/rxrpc/rxkad.c
> @@ -146,10 +146,10 @@ static int rxkad_prime_packet_security(struct rxrpc_connection *conn)
> static int rxkad_secure_packet_auth(const struct rxrpc_call *call,
> struct sk_buff *skb,
> u32 data_size,
> - void *sechdr)
> + void *sechdr,
> + struct skcipher_request *req)
> {
> struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
> - SKCIPHER_REQUEST_ON_STACK(req, call->conn->cipher);
> struct rxkad_level1_hdr hdr;
> struct rxrpc_crypt iv;
> struct scatterlist sg;
> @@ -183,12 +183,12 @@ static int rxkad_secure_packet_auth(const struct rxrpc_call *call,
> static int rxkad_secure_packet_encrypt(const struct rxrpc_call *call,
> struct sk_buff *skb,
> u32 data_size,
> - void *sechdr)
> + void *sechdr,
> + struct skcipher_request *req)
> {
> const struct rxrpc_key_token *token;
> struct rxkad_level2_hdr rxkhdr;
> struct rxrpc_skb_priv *sp;
> - SKCIPHER_REQUEST_ON_STACK(req, call->conn->cipher);
> struct rxrpc_crypt iv;
> struct scatterlist sg[16];
> struct sk_buff *trailer;
> @@ -296,11 +296,12 @@ static int rxkad_secure_packet(struct rxrpc_call *call,
> ret = 0;
> break;
> case RXRPC_SECURITY_AUTH:
> - ret = rxkad_secure_packet_auth(call, skb, data_size, sechdr);
> + ret = rxkad_secure_packet_auth(call, skb, data_size, sechdr,
> + req);
> break;
> case RXRPC_SECURITY_ENCRYPT:
> ret = rxkad_secure_packet_encrypt(call, skb, data_size,
> - sechdr);
> + sechdr, req);
> break;
> default:
> ret = -EPERM;
> @@ -316,10 +317,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;
> @@ -402,11 +403,11 @@ static int rxkad_verify_packet_1(struct rxrpc_call *call, struct sk_buff *skb,
> */
> static int rxkad_verify_packet_2(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)
> {
> const struct rxrpc_key_token *token;
> struct rxkad_level2_hdr sechdr;
> - SKCIPHER_REQUEST_ON_STACK(req, call->conn->cipher);
> struct rxrpc_crypt iv;
> struct scatterlist _sg[4], *sg;
> struct sk_buff *trailer;
> @@ -549,9 +550,9 @@ 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);
> + return rxkad_verify_packet_2(call, skb, offset, len, seq, req);
> default:
> return -ENOANO;
> }
> --
> 2.17.1
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?
- Eric
next prev parent reply other threads:[~2018-07-16 17:42 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-16 3:49 [PATCH] rxrpc: Reuse SKCIPHER_REQUEST_ON_STACK buffer Kees Cook
2018-07-16 10:17 ` Arnd Bergmann
2018-07-19 4:29 ` Kees Cook
2018-07-16 17:42 ` Eric Biggers [this message]
2018-07-19 4:31 ` Kees Cook
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180716174223.GE77258@google.com \
--to=ebiggers@google$(echo .)com \
--cc=arnd@arndb$(echo .)de \
--cc=davem@davemloft$(echo .)net \
--cc=dhowells@redhat$(echo .)com \
--cc=gustavo@embeddedor$(echo .)com \
--cc=herbert@gondor$(echo .)apana.org.au \
--cc=keescook@chromium$(echo .)org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox