* [bug report] rxrpc: rxgk: Implement the yfs-rxgk security class (GSSAPI)
@ 2025-04-16 11:11 Dan Carpenter
0 siblings, 0 replies; 2+ messages in thread
From: Dan Carpenter @ 2025-04-16 11:11 UTC (permalink / raw)
To: David Howells; +Cc: linux-afs, netdev
Hello David Howells,
Commit 9d1d2b59341f ("rxrpc: rxgk: Implement the yfs-rxgk security
class (GSSAPI)") from Apr 11, 2025 (linux-next), leads to the
following Smatch static checker warning:
net/rxrpc/rxgk.c:501 rxgk_verify_packet_integrity()
error: uninitialized symbol 'ac'.
net/rxrpc/rxgk.c
467 static int rxgk_verify_packet_integrity(struct rxrpc_call *call,
468 struct rxgk_context *gk,
469 struct sk_buff *skb)
470 {
471 struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
472 struct rxgk_header *hdr;
473 struct krb5_buffer metadata;
474 unsigned int offset = sp->offset, len = sp->len;
475 size_t data_offset = 0, data_len = len;
476 u32 ac;
477 int ret = -ENOMEM;
478
479 _enter("");
480
481 crypto_krb5_where_is_the_data(gk->krb5, KRB5_CHECKSUM_MODE,
482 &data_offset, &data_len);
483
484 hdr = kzalloc(sizeof(*hdr), GFP_NOFS);
485 if (!hdr)
486 return -ENOMEM;
487
488 hdr->epoch = htonl(call->conn->proto.epoch);
489 hdr->cid = htonl(call->cid);
490 hdr->call_number = htonl(call->call_id);
491 hdr->seq = htonl(sp->hdr.seq);
492 hdr->sec_index = htonl(call->security_ix);
493 hdr->data_len = htonl(data_len);
494
495 metadata.len = sizeof(*hdr);
496 metadata.data = hdr;
497 ret = rxgk_verify_mic_skb(gk->krb5, gk->rx_Kc, &metadata,
498 skb, &offset, &len, &ac);
499 kfree(hdr);
500 if (ret == -EPROTO) {
--> 501 rxrpc_abort_eproto(call, skb, ac,
This is a false positive in Smatch, but why is only -EPROTO handled and
not other error codes? It could be intentional, but it's hard for me to
be sure because I don't know the code well.
502 rxgk_abort_1_verify_mic_eproto);
503 } else {
504 sp->offset = offset;
505 sp->len = len;
506 }
507
508 rxgk_put(gk);
509 _leave(" = %d", ret);
510 return ret;
511 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* [bug report] rxrpc: rxgk: Implement the yfs-rxgk security class (GSSAPI)
@ 2025-04-16 14:12 Dan Carpenter
0 siblings, 0 replies; 2+ messages in thread
From: Dan Carpenter @ 2025-04-16 14:12 UTC (permalink / raw)
To: David Howells; +Cc: linux-afs, netdev
Hello David Howells,
Commit 9d1d2b59341f ("rxrpc: rxgk: Implement the yfs-rxgk security
class (GSSAPI)") from Apr 11, 2025 (linux-next), leads to the
following Smatch static checker warning:
net/rxrpc/rxgk_app.c:240 rxgk_extract_token()
error: uninitialized symbol 'ec'.
net/rxrpc/rxgk_app.c
180 int rxgk_extract_token(struct rxrpc_connection *conn, struct sk_buff *skb,
181 unsigned int token_offset, unsigned int token_len,
182 struct key **_key)
183 {
184 const struct krb5_enctype *krb5;
185 const struct krb5_buffer *server_secret;
186 struct crypto_aead *token_enc = NULL;
187 struct key *server_key;
188 unsigned int ticket_offset, ticket_len;
189 u32 kvno, enctype;
190 int ret, ec;
191
192 struct {
193 __be32 kvno;
194 __be32 enctype;
195 __be32 token_len;
196 } container;
197
198 /* Decode the RXGK_TokenContainer object. This tells us which server
199 * key we should be using. We can then fetch the key, get the secret
200 * and set up the crypto to extract the token.
201 */
202 if (skb_copy_bits(skb, token_offset, &container, sizeof(container)) < 0)
203 return rxrpc_abort_conn(conn, skb, RXGK_PACKETSHORT, -EPROTO,
204 rxgk_abort_resp_tok_short);
205
206 kvno = ntohl(container.kvno);
207 enctype = ntohl(container.enctype);
208 ticket_len = ntohl(container.token_len);
209 ticket_offset = token_offset + sizeof(container);
210
211 if (xdr_round_up(ticket_len) > token_len - 3 * 4)
212 return rxrpc_abort_conn(conn, skb, RXGK_PACKETSHORT, -EPROTO,
213 rxgk_abort_resp_tok_short);
214
215 _debug("KVNO %u", kvno);
216 _debug("ENC %u", enctype);
217 _debug("TLEN %u", ticket_len);
218
219 server_key = rxrpc_look_up_server_security(conn, skb, kvno, enctype);
220 if (IS_ERR(server_key))
221 goto cant_get_server_key;
222
223 down_read(&server_key->sem);
224 server_secret = (const void *)&server_key->payload.data[2];
225 ret = rxgk_set_up_token_cipher(server_secret, &token_enc, enctype, &krb5, GFP_NOFS);
226 up_read(&server_key->sem);
227 key_put(server_key);
228 if (ret < 0)
229 goto cant_get_token;
230
231 /* We can now decrypt and parse the token/ticket. This allows us to
232 * gain access to K0, from which we can derive the transport key and
233 * thence decode the authenticator.
234 */
235 ret = rxgk_decrypt_skb(krb5, token_enc, skb,
236 &ticket_offset, &ticket_len, &ec);
^^^
ec is only sometimes set here.
237 crypto_free_aead(token_enc);
238 token_enc = NULL;
--> 239 if (ret < 0)
240 return rxrpc_abort_conn(conn, skb, ec, ret,
^^
This is Undefined Behavior.
241 rxgk_abort_resp_tok_dec);
242
243 ret = conn->security->default_decode_ticket(conn, skb, ticket_offset,
244 ticket_len, _key);
245 if (ret < 0)
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-04-16 14:12 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-16 11:11 [bug report] rxrpc: rxgk: Implement the yfs-rxgk security class (GSSAPI) Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2025-04-16 14:12 Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).