Skip to content

Commit 8c2f826

Browse files
dhowellsdavem330
dhowells
authored and
davem330
committed
rxrpc: Don't put crypto buffers on the stack
Don't put buffers of data to be handed to crypto on the stack as this may cause an assertion failure in the kernel (see below). Fix this by using an kmalloc'd buffer instead. kernel BUG at ./include/linux/scatterlist.h:147! ... RIP: 0010:rxkad_encrypt_response.isra.6+0x191/0x1b0 [rxrpc] RSP: 0018:ffffbe2fc06cfca8 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ffff989277d59900 RCX: 0000000000000028 RDX: 0000259dc06cfd88 RSI: 0000000000000025 RDI: ffffbe30406cfd88 RBP: ffffbe2fc06cfd60 R08: ffffbe2fc06cfd08 R09: ffffbe2fc06cfd08 R10: 0000000000000000 R11: 0000000000000000 R12: 1ffff7c5f80d9f95 R13: ffffbe2fc06cfd88 R14: ffff98927a3f7aa0 R15: ffffbe2fc06cfd08 FS: 0000000000000000(0000) GS:ffff98927fc00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 000055b1ff28f0f8 CR3: 000000001b412003 CR4: 00000000003606f0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: rxkad_respond_to_challenge+0x297/0x330 [rxrpc] rxrpc_process_connection+0xd1/0x690 [rxrpc] ? process_one_work+0x1c3/0x680 ? __lock_is_held+0x59/0xa0 process_one_work+0x249/0x680 worker_thread+0x3a/0x390 ? process_one_work+0x680/0x680 kthread+0x121/0x140 ? kthread_create_worker_on_cpu+0x70/0x70 ret_from_fork+0x3a/0x50 Reported-by: Jonathan Billings <[email protected]> Reported-by: Marc Dionne <[email protected]> Signed-off-by: David Howells <[email protected]> Tested-by: Jonathan Billings <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent c702558 commit 8c2f826

File tree

2 files changed

+52
-41
lines changed

2 files changed

+52
-41
lines changed

‎net/rxrpc/conn_event.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,7 @@ void rxrpc_process_connection(struct work_struct *work)
460460
case -EKEYEXPIRED:
461461
case -EKEYREJECTED:
462462
goto protocol_error;
463+
case -ENOMEM:
463464
case -EAGAIN:
464465
goto requeue_and_leave;
465466
case -ECONNABORTED:

‎net/rxrpc/rxkad.c

Lines changed: 51 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -773,8 +773,7 @@ static int rxkad_respond_to_challenge(struct rxrpc_connection *conn,
773773
{
774774
const struct rxrpc_key_token *token;
775775
struct rxkad_challenge challenge;
776-
struct rxkad_response resp
777-
__attribute__((aligned(8))); /* must be aligned for crypto */
776+
struct rxkad_response *resp;
778777
struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
779778
const char *eproto;
780779
u32 version, nonce, min_level, abort_code;
@@ -818,26 +817,29 @@ static int rxkad_respond_to_challenge(struct rxrpc_connection *conn,
818817
token = conn->params.key->payload.data[0];
819818

820819
/* build the response packet */
821-
memset(&resp, 0, sizeof(resp));
822-
823-
resp.version = htonl(RXKAD_VERSION);
824-
resp.encrypted.epoch = htonl(conn->proto.epoch);
825-
resp.encrypted.cid = htonl(conn->proto.cid);
826-
resp.encrypted.securityIndex = htonl(conn->security_ix);
827-
resp.encrypted.inc_nonce = htonl(nonce + 1);
828-
resp.encrypted.level = htonl(conn->params.security_level);
829-
resp.kvno = htonl(token->kad->kvno);
830-
resp.ticket_len = htonl(token->kad->ticket_len);
831-
832-
resp.encrypted.call_id[0] = htonl(conn->channels[0].call_counter);
833-
resp.encrypted.call_id[1] = htonl(conn->channels[1].call_counter);
834-
resp.encrypted.call_id[2] = htonl(conn->channels[2].call_counter);
835-
resp.encrypted.call_id[3] = htonl(conn->channels[3].call_counter);
820+
resp = kzalloc(sizeof(struct rxkad_response), GFP_NOFS);
821+
if (!resp)
822+
return -ENOMEM;
823+
824+
resp->version = htonl(RXKAD_VERSION);
825+
resp->encrypted.epoch = htonl(conn->proto.epoch);
826+
resp->encrypted.cid = htonl(conn->proto.cid);
827+
resp->encrypted.securityIndex = htonl(conn->security_ix);
828+
resp->encrypted.inc_nonce = htonl(nonce + 1);
829+
resp->encrypted.level = htonl(conn->params.security_level);
830+
resp->kvno = htonl(token->kad->kvno);
831+
resp->ticket_len = htonl(token->kad->ticket_len);
832+
resp->encrypted.call_id[0] = htonl(conn->channels[0].call_counter);
833+
resp->encrypted.call_id[1] = htonl(conn->channels[1].call_counter);
834+
resp->encrypted.call_id[2] = htonl(conn->channels[2].call_counter);
835+
resp->encrypted.call_id[3] = htonl(conn->channels[3].call_counter);
836836

837837
/* calculate the response checksum and then do the encryption */
838-
rxkad_calc_response_checksum(&resp);
839-
rxkad_encrypt_response(conn, &resp, token->kad);
840-
return rxkad_send_response(conn, &sp->hdr, &resp, token->kad);
838+
rxkad_calc_response_checksum(resp);
839+
rxkad_encrypt_response(conn, resp, token->kad);
840+
ret = rxkad_send_response(conn, &sp->hdr, resp, token->kad);
841+
kfree(resp);
842+
return ret;
841843

842844
protocol_error:
843845
trace_rxrpc_rx_eproto(NULL, sp->hdr.serial, eproto);
@@ -1048,8 +1050,7 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
10481050
struct sk_buff *skb,
10491051
u32 *_abort_code)
10501052
{
1051-
struct rxkad_response response
1052-
__attribute__((aligned(8))); /* must be aligned for crypto */
1053+
struct rxkad_response *response;
10531054
struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
10541055
struct rxrpc_crypt session_key;
10551056
const char *eproto;
@@ -1061,17 +1062,22 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
10611062

10621063
_enter("{%d,%x}", conn->debug_id, key_serial(conn->server_key));
10631064

1065+
ret = -ENOMEM;
1066+
response = kzalloc(sizeof(struct rxkad_response), GFP_NOFS);
1067+
if (!response)
1068+
goto temporary_error;
1069+
10641070
eproto = tracepoint_string("rxkad_rsp_short");
10651071
abort_code = RXKADPACKETSHORT;
10661072
if (skb_copy_bits(skb, sizeof(struct rxrpc_wire_header),
1067-
&response, sizeof(response)) < 0)
1073+
response, sizeof(*response)) < 0)
10681074
goto protocol_error;
1069-
if (!pskb_pull(skb, sizeof(response)))
1075+
if (!pskb_pull(skb, sizeof(*response)))
10701076
BUG();
10711077

1072-
version = ntohl(response.version);
1073-
ticket_len = ntohl(response.ticket_len);
1074-
kvno = ntohl(response.kvno);
1078+
version = ntohl(response->version);
1079+
ticket_len = ntohl(response->ticket_len);
1080+
kvno = ntohl(response->kvno);
10751081
_proto("Rx RESPONSE %%%u { v=%u kv=%u tl=%u }",
10761082
sp->hdr.serial, version, kvno, ticket_len);
10771083

@@ -1105,31 +1111,31 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
11051111
ret = rxkad_decrypt_ticket(conn, skb, ticket, ticket_len, &session_key,
11061112
&expiry, _abort_code);
11071113
if (ret < 0)
1108-
goto temporary_error_free;
1114+
goto temporary_error_free_resp;
11091115

11101116
/* use the session key from inside the ticket to decrypt the
11111117
* response */
1112-
rxkad_decrypt_response(conn, &response, &session_key);
1118+
rxkad_decrypt_response(conn, response, &session_key);
11131119

11141120
eproto = tracepoint_string("rxkad_rsp_param");
11151121
abort_code = RXKADSEALEDINCON;
1116-
if (ntohl(response.encrypted.epoch) != conn->proto.epoch)
1122+
if (ntohl(response->encrypted.epoch) != conn->proto.epoch)
11171123
goto protocol_error_free;
1118-
if (ntohl(response.encrypted.cid) != conn->proto.cid)
1124+
if (ntohl(response->encrypted.cid) != conn->proto.cid)
11191125
goto protocol_error_free;
1120-
if (ntohl(response.encrypted.securityIndex) != conn->security_ix)
1126+
if (ntohl(response->encrypted.securityIndex) != conn->security_ix)
11211127
goto protocol_error_free;
1122-
csum = response.encrypted.checksum;
1123-
response.encrypted.checksum = 0;
1124-
rxkad_calc_response_checksum(&response);
1128+
csum = response->encrypted.checksum;
1129+
response->encrypted.checksum = 0;
1130+
rxkad_calc_response_checksum(response);
11251131
eproto = tracepoint_string("rxkad_rsp_csum");
1126-
if (response.encrypted.checksum != csum)
1132+
if (response->encrypted.checksum != csum)
11271133
goto protocol_error_free;
11281134

11291135
spin_lock(&conn->channel_lock);
11301136
for (i = 0; i < RXRPC_MAXCALLS; i++) {
11311137
struct rxrpc_call *call;
1132-
u32 call_id = ntohl(response.encrypted.call_id[i]);
1138+
u32 call_id = ntohl(response->encrypted.call_id[i]);
11331139

11341140
eproto = tracepoint_string("rxkad_rsp_callid");
11351141
if (call_id > INT_MAX)
@@ -1153,12 +1159,12 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
11531159

11541160
eproto = tracepoint_string("rxkad_rsp_seq");
11551161
abort_code = RXKADOUTOFSEQUENCE;
1156-
if (ntohl(response.encrypted.inc_nonce) != conn->security_nonce + 1)
1162+
if (ntohl(response->encrypted.inc_nonce) != conn->security_nonce + 1)
11571163
goto protocol_error_free;
11581164

11591165
eproto = tracepoint_string("rxkad_rsp_level");
11601166
abort_code = RXKADLEVELFAIL;
1161-
level = ntohl(response.encrypted.level);
1167+
level = ntohl(response->encrypted.level);
11621168
if (level > RXRPC_SECURITY_ENCRYPT)
11631169
goto protocol_error_free;
11641170
conn->params.security_level = level;
@@ -1168,9 +1174,10 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
11681174
* as for a client connection */
11691175
ret = rxrpc_get_server_data_key(conn, &session_key, expiry, kvno);
11701176
if (ret < 0)
1171-
goto temporary_error_free;
1177+
goto temporary_error_free_ticket;
11721178

11731179
kfree(ticket);
1180+
kfree(response);
11741181
_leave(" = 0");
11751182
return 0;
11761183

@@ -1179,12 +1186,15 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
11791186
protocol_error_free:
11801187
kfree(ticket);
11811188
protocol_error:
1189+
kfree(response);
11821190
trace_rxrpc_rx_eproto(NULL, sp->hdr.serial, eproto);
11831191
*_abort_code = abort_code;
11841192
return -EPROTO;
11851193

1186-
temporary_error_free:
1194+
temporary_error_free_ticket:
11871195
kfree(ticket);
1196+
temporary_error_free_resp:
1197+
kfree(response);
11881198
temporary_error:
11891199
/* Ignore the response packet if we got a temporary error such as
11901200
* ENOMEM. We just want to send the challenge again. Note that we

0 commit comments

Comments
 (0)