Skip to content

Commit 7359db6

Browse files
committed
Merge tag 'rxrpc-fixes-20191007' of git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs
David Howells says: ==================== rxrpc: Syzbot-inspired fixes Here's a series of patches that fix a number of issues found by syzbot: (1) A reference leak on rxrpc_call structs in a sendmsg error path. (2) A tracepoint that looked in the rxrpc_peer record after putting it. Analogous with this, though not presently detected, the same bug is also fixed in relation to rxrpc_connection and rxrpc_call records. (3) Peer records don't pin local endpoint records, despite accessing them. (4) Access to connection crypto ops to clean up a call after the call's ref on that connection has been put. ==================== Signed-off-by: David S. Miller <[email protected]>
2 parents 57acce3 + 91fcfbe commit 7359db6

File tree

10 files changed

+63
-44
lines changed

10 files changed

+63
-44
lines changed

include/trace/events/rxrpc.h

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -519,10 +519,10 @@ TRACE_EVENT(rxrpc_local,
519519
);
520520

521521
TRACE_EVENT(rxrpc_peer,
522-
TP_PROTO(struct rxrpc_peer *peer, enum rxrpc_peer_trace op,
522+
TP_PROTO(unsigned int peer_debug_id, enum rxrpc_peer_trace op,
523523
int usage, const void *where),
524524

525-
TP_ARGS(peer, op, usage, where),
525+
TP_ARGS(peer_debug_id, op, usage, where),
526526

527527
TP_STRUCT__entry(
528528
__field(unsigned int, peer )
@@ -532,7 +532,7 @@ TRACE_EVENT(rxrpc_peer,
532532
),
533533

534534
TP_fast_assign(
535-
__entry->peer = peer->debug_id;
535+
__entry->peer = peer_debug_id;
536536
__entry->op = op;
537537
__entry->usage = usage;
538538
__entry->where = where;
@@ -546,10 +546,10 @@ TRACE_EVENT(rxrpc_peer,
546546
);
547547

548548
TRACE_EVENT(rxrpc_conn,
549-
TP_PROTO(struct rxrpc_connection *conn, enum rxrpc_conn_trace op,
549+
TP_PROTO(unsigned int conn_debug_id, enum rxrpc_conn_trace op,
550550
int usage, const void *where),
551551

552-
TP_ARGS(conn, op, usage, where),
552+
TP_ARGS(conn_debug_id, op, usage, where),
553553

554554
TP_STRUCT__entry(
555555
__field(unsigned int, conn )
@@ -559,7 +559,7 @@ TRACE_EVENT(rxrpc_conn,
559559
),
560560

561561
TP_fast_assign(
562-
__entry->conn = conn->debug_id;
562+
__entry->conn = conn_debug_id;
563563
__entry->op = op;
564564
__entry->usage = usage;
565565
__entry->where = where;
@@ -606,10 +606,10 @@ TRACE_EVENT(rxrpc_client,
606606
);
607607

608608
TRACE_EVENT(rxrpc_call,
609-
TP_PROTO(struct rxrpc_call *call, enum rxrpc_call_trace op,
609+
TP_PROTO(unsigned int call_debug_id, enum rxrpc_call_trace op,
610610
int usage, const void *where, const void *aux),
611611

612-
TP_ARGS(call, op, usage, where, aux),
612+
TP_ARGS(call_debug_id, op, usage, where, aux),
613613

614614
TP_STRUCT__entry(
615615
__field(unsigned int, call )
@@ -620,7 +620,7 @@ TRACE_EVENT(rxrpc_call,
620620
),
621621

622622
TP_fast_assign(
623-
__entry->call = call->debug_id;
623+
__entry->call = call_debug_id;
624624
__entry->op = op;
625625
__entry->usage = usage;
626626
__entry->where = where;

net/rxrpc/ar-internal.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,7 @@ struct rxrpc_call {
556556
struct rxrpc_peer *peer; /* Peer record for remote address */
557557
struct rxrpc_sock __rcu *socket; /* socket responsible */
558558
struct rxrpc_net *rxnet; /* Network namespace to which call belongs */
559+
const struct rxrpc_security *security; /* applied security module */
559560
struct mutex user_mutex; /* User access mutex */
560561
unsigned long ack_at; /* When deferred ACK needs to happen */
561562
unsigned long ack_lost_at; /* When ACK is figured as lost */

net/rxrpc/call_accept.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ static int rxrpc_service_prealloc_one(struct rxrpc_sock *rx,
8484
smp_store_release(&b->conn_backlog_head,
8585
(head + 1) & (size - 1));
8686

87-
trace_rxrpc_conn(conn, rxrpc_conn_new_service,
87+
trace_rxrpc_conn(conn->debug_id, rxrpc_conn_new_service,
8888
atomic_read(&conn->usage), here);
8989
}
9090

@@ -97,7 +97,7 @@ static int rxrpc_service_prealloc_one(struct rxrpc_sock *rx,
9797
call->flags |= (1 << RXRPC_CALL_IS_SERVICE);
9898
call->state = RXRPC_CALL_SERVER_PREALLOC;
9999

100-
trace_rxrpc_call(call, rxrpc_call_new_service,
100+
trace_rxrpc_call(call->debug_id, rxrpc_call_new_service,
101101
atomic_read(&call->usage),
102102
here, (const void *)user_call_ID);
103103

@@ -307,6 +307,7 @@ static struct rxrpc_call *rxrpc_alloc_incoming_call(struct rxrpc_sock *rx,
307307

308308
rxrpc_see_call(call);
309309
call->conn = conn;
310+
call->security = conn->security;
310311
call->peer = rxrpc_get_peer(conn->params.peer);
311312
call->cong_cwnd = call->peer->cong_cwnd;
312313
return call;

net/rxrpc/call_object.c

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,8 @@ struct rxrpc_call *rxrpc_new_client_call(struct rxrpc_sock *rx,
240240
if (p->intr)
241241
__set_bit(RXRPC_CALL_IS_INTR, &call->flags);
242242
call->tx_total_len = p->tx_total_len;
243-
trace_rxrpc_call(call, rxrpc_call_new_client, atomic_read(&call->usage),
243+
trace_rxrpc_call(call->debug_id, rxrpc_call_new_client,
244+
atomic_read(&call->usage),
244245
here, (const void *)p->user_call_ID);
245246

246247
/* We need to protect a partially set up call against the user as we
@@ -290,8 +291,8 @@ struct rxrpc_call *rxrpc_new_client_call(struct rxrpc_sock *rx,
290291
if (ret < 0)
291292
goto error;
292293

293-
trace_rxrpc_call(call, rxrpc_call_connected, atomic_read(&call->usage),
294-
here, NULL);
294+
trace_rxrpc_call(call->debug_id, rxrpc_call_connected,
295+
atomic_read(&call->usage), here, NULL);
295296

296297
rxrpc_start_call_timer(call);
297298

@@ -313,8 +314,8 @@ struct rxrpc_call *rxrpc_new_client_call(struct rxrpc_sock *rx,
313314
error:
314315
__rxrpc_set_call_completion(call, RXRPC_CALL_LOCAL_ERROR,
315316
RX_CALL_DEAD, ret);
316-
trace_rxrpc_call(call, rxrpc_call_error, atomic_read(&call->usage),
317-
here, ERR_PTR(ret));
317+
trace_rxrpc_call(call->debug_id, rxrpc_call_error,
318+
atomic_read(&call->usage), here, ERR_PTR(ret));
318319
rxrpc_release_call(rx, call);
319320
mutex_unlock(&call->user_mutex);
320321
rxrpc_put_call(call, rxrpc_call_put);
@@ -376,7 +377,8 @@ bool rxrpc_queue_call(struct rxrpc_call *call)
376377
if (n == 0)
377378
return false;
378379
if (rxrpc_queue_work(&call->processor))
379-
trace_rxrpc_call(call, rxrpc_call_queued, n + 1, here, NULL);
380+
trace_rxrpc_call(call->debug_id, rxrpc_call_queued, n + 1,
381+
here, NULL);
380382
else
381383
rxrpc_put_call(call, rxrpc_call_put_noqueue);
382384
return true;
@@ -391,7 +393,8 @@ bool __rxrpc_queue_call(struct rxrpc_call *call)
391393
int n = atomic_read(&call->usage);
392394
ASSERTCMP(n, >=, 1);
393395
if (rxrpc_queue_work(&call->processor))
394-
trace_rxrpc_call(call, rxrpc_call_queued_ref, n, here, NULL);
396+
trace_rxrpc_call(call->debug_id, rxrpc_call_queued_ref, n,
397+
here, NULL);
395398
else
396399
rxrpc_put_call(call, rxrpc_call_put_noqueue);
397400
return true;
@@ -406,7 +409,8 @@ void rxrpc_see_call(struct rxrpc_call *call)
406409
if (call) {
407410
int n = atomic_read(&call->usage);
408411

409-
trace_rxrpc_call(call, rxrpc_call_seen, n, here, NULL);
412+
trace_rxrpc_call(call->debug_id, rxrpc_call_seen, n,
413+
here, NULL);
410414
}
411415
}
412416

@@ -418,7 +422,7 @@ void rxrpc_get_call(struct rxrpc_call *call, enum rxrpc_call_trace op)
418422
const void *here = __builtin_return_address(0);
419423
int n = atomic_inc_return(&call->usage);
420424

421-
trace_rxrpc_call(call, op, n, here, NULL);
425+
trace_rxrpc_call(call->debug_id, op, n, here, NULL);
422426
}
423427

424428
/*
@@ -445,7 +449,8 @@ void rxrpc_release_call(struct rxrpc_sock *rx, struct rxrpc_call *call)
445449

446450
_enter("{%d,%d}", call->debug_id, atomic_read(&call->usage));
447451

448-
trace_rxrpc_call(call, rxrpc_call_release, atomic_read(&call->usage),
452+
trace_rxrpc_call(call->debug_id, rxrpc_call_release,
453+
atomic_read(&call->usage),
449454
here, (const void *)call->flags);
450455

451456
ASSERTCMP(call->state, ==, RXRPC_CALL_COMPLETE);
@@ -488,10 +493,10 @@ void rxrpc_release_call(struct rxrpc_sock *rx, struct rxrpc_call *call)
488493

489494
_debug("RELEASE CALL %p (%d CONN %p)", call, call->debug_id, conn);
490495

491-
if (conn) {
496+
if (conn)
492497
rxrpc_disconnect_call(call);
493-
conn->security->free_call_crypto(call);
494-
}
498+
if (call->security)
499+
call->security->free_call_crypto(call);
495500

496501
rxrpc_cleanup_ring(call);
497502
_leave("");
@@ -534,12 +539,13 @@ void rxrpc_put_call(struct rxrpc_call *call, enum rxrpc_call_trace op)
534539
{
535540
struct rxrpc_net *rxnet = call->rxnet;
536541
const void *here = __builtin_return_address(0);
542+
unsigned int debug_id = call->debug_id;
537543
int n;
538544

539545
ASSERT(call != NULL);
540546

541547
n = atomic_dec_return(&call->usage);
542-
trace_rxrpc_call(call, op, n, here, NULL);
548+
trace_rxrpc_call(debug_id, op, n, here, NULL);
543549
ASSERTCMP(n, >=, 0);
544550
if (n == 0) {
545551
_debug("call %d dead", call->debug_id);

net/rxrpc/conn_client.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,8 @@ rxrpc_alloc_client_connection(struct rxrpc_conn_parameters *cp, gfp_t gfp)
212212
rxrpc_get_local(conn->params.local);
213213
key_get(conn->params.key);
214214

215-
trace_rxrpc_conn(conn, rxrpc_conn_new_client, atomic_read(&conn->usage),
215+
trace_rxrpc_conn(conn->debug_id, rxrpc_conn_new_client,
216+
atomic_read(&conn->usage),
216217
__builtin_return_address(0));
217218
trace_rxrpc_client(conn, -1, rxrpc_client_alloc);
218219
_leave(" = %p", conn);
@@ -352,6 +353,7 @@ static int rxrpc_get_client_conn(struct rxrpc_sock *rx,
352353

353354
if (cp->exclusive) {
354355
call->conn = candidate;
356+
call->security = candidate->security;
355357
call->security_ix = candidate->security_ix;
356358
call->service_id = candidate->service_id;
357359
_leave(" = 0 [exclusive %d]", candidate->debug_id);
@@ -403,6 +405,7 @@ static int rxrpc_get_client_conn(struct rxrpc_sock *rx,
403405
candidate_published:
404406
set_bit(RXRPC_CONN_IN_CLIENT_CONNS, &candidate->flags);
405407
call->conn = candidate;
408+
call->security = candidate->security;
406409
call->security_ix = candidate->security_ix;
407410
call->service_id = candidate->service_id;
408411
spin_unlock(&local->client_conns_lock);
@@ -425,6 +428,7 @@ static int rxrpc_get_client_conn(struct rxrpc_sock *rx,
425428

426429
spin_lock(&conn->channel_lock);
427430
call->conn = conn;
431+
call->security = conn->security;
428432
call->security_ix = conn->security_ix;
429433
call->service_id = conn->service_id;
430434
list_add_tail(&call->chan_wait_link, &conn->waiting_calls);
@@ -985,11 +989,12 @@ rxrpc_put_one_client_conn(struct rxrpc_connection *conn)
985989
void rxrpc_put_client_conn(struct rxrpc_connection *conn)
986990
{
987991
const void *here = __builtin_return_address(0);
992+
unsigned int debug_id = conn->debug_id;
988993
int n;
989994

990995
do {
991996
n = atomic_dec_return(&conn->usage);
992-
trace_rxrpc_conn(conn, rxrpc_conn_put_client, n, here);
997+
trace_rxrpc_conn(debug_id, rxrpc_conn_put_client, n, here);
993998
if (n > 0)
994999
return;
9951000
ASSERTCMP(n, >=, 0);

net/rxrpc/conn_object.c

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ bool rxrpc_queue_conn(struct rxrpc_connection *conn)
269269
if (n == 0)
270270
return false;
271271
if (rxrpc_queue_work(&conn->processor))
272-
trace_rxrpc_conn(conn, rxrpc_conn_queued, n + 1, here);
272+
trace_rxrpc_conn(conn->debug_id, rxrpc_conn_queued, n + 1, here);
273273
else
274274
rxrpc_put_connection(conn);
275275
return true;
@@ -284,7 +284,7 @@ void rxrpc_see_connection(struct rxrpc_connection *conn)
284284
if (conn) {
285285
int n = atomic_read(&conn->usage);
286286

287-
trace_rxrpc_conn(conn, rxrpc_conn_seen, n, here);
287+
trace_rxrpc_conn(conn->debug_id, rxrpc_conn_seen, n, here);
288288
}
289289
}
290290

@@ -296,7 +296,7 @@ void rxrpc_get_connection(struct rxrpc_connection *conn)
296296
const void *here = __builtin_return_address(0);
297297
int n = atomic_inc_return(&conn->usage);
298298

299-
trace_rxrpc_conn(conn, rxrpc_conn_got, n, here);
299+
trace_rxrpc_conn(conn->debug_id, rxrpc_conn_got, n, here);
300300
}
301301

302302
/*
@@ -310,7 +310,7 @@ rxrpc_get_connection_maybe(struct rxrpc_connection *conn)
310310
if (conn) {
311311
int n = atomic_fetch_add_unless(&conn->usage, 1, 0);
312312
if (n > 0)
313-
trace_rxrpc_conn(conn, rxrpc_conn_got, n + 1, here);
313+
trace_rxrpc_conn(conn->debug_id, rxrpc_conn_got, n + 1, here);
314314
else
315315
conn = NULL;
316316
}
@@ -333,10 +333,11 @@ static void rxrpc_set_service_reap_timer(struct rxrpc_net *rxnet,
333333
void rxrpc_put_service_conn(struct rxrpc_connection *conn)
334334
{
335335
const void *here = __builtin_return_address(0);
336+
unsigned int debug_id = conn->debug_id;
336337
int n;
337338

338339
n = atomic_dec_return(&conn->usage);
339-
trace_rxrpc_conn(conn, rxrpc_conn_put_service, n, here);
340+
trace_rxrpc_conn(debug_id, rxrpc_conn_put_service, n, here);
340341
ASSERTCMP(n, >=, 0);
341342
if (n == 1)
342343
rxrpc_set_service_reap_timer(conn->params.local->rxnet,
@@ -420,7 +421,7 @@ void rxrpc_service_connection_reaper(struct work_struct *work)
420421
*/
421422
if (atomic_cmpxchg(&conn->usage, 1, 0) != 1)
422423
continue;
423-
trace_rxrpc_conn(conn, rxrpc_conn_reap_service, 0, NULL);
424+
trace_rxrpc_conn(conn->debug_id, rxrpc_conn_reap_service, 0, NULL);
424425

425426
if (rxrpc_conn_is_client(conn))
426427
BUG();

net/rxrpc/conn_service.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ struct rxrpc_connection *rxrpc_prealloc_service_connection(struct rxrpc_net *rxn
134134
list_add_tail(&conn->proc_link, &rxnet->conn_proc_list);
135135
write_unlock(&rxnet->conn_lock);
136136

137-
trace_rxrpc_conn(conn, rxrpc_conn_new_service,
137+
trace_rxrpc_conn(conn->debug_id, rxrpc_conn_new_service,
138138
atomic_read(&conn->usage),
139139
__builtin_return_address(0));
140140
}

net/rxrpc/peer_object.c

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ struct rxrpc_peer *rxrpc_alloc_peer(struct rxrpc_local *local, gfp_t gfp)
216216
peer = kzalloc(sizeof(struct rxrpc_peer), gfp);
217217
if (peer) {
218218
atomic_set(&peer->usage, 1);
219-
peer->local = local;
219+
peer->local = rxrpc_get_local(local);
220220
INIT_HLIST_HEAD(&peer->error_targets);
221221
peer->service_conns = RB_ROOT;
222222
seqlock_init(&peer->service_conn_lock);
@@ -307,7 +307,6 @@ void rxrpc_new_incoming_peer(struct rxrpc_sock *rx, struct rxrpc_local *local,
307307
unsigned long hash_key;
308308

309309
hash_key = rxrpc_peer_hash_key(local, &peer->srx);
310-
peer->local = local;
311310
rxrpc_init_peer(rx, peer, hash_key);
312311

313312
spin_lock(&rxnet->peer_hash_lock);
@@ -382,7 +381,7 @@ struct rxrpc_peer *rxrpc_get_peer(struct rxrpc_peer *peer)
382381
int n;
383382

384383
n = atomic_inc_return(&peer->usage);
385-
trace_rxrpc_peer(peer, rxrpc_peer_got, n, here);
384+
trace_rxrpc_peer(peer->debug_id, rxrpc_peer_got, n, here);
386385
return peer;
387386
}
388387

@@ -396,7 +395,7 @@ struct rxrpc_peer *rxrpc_get_peer_maybe(struct rxrpc_peer *peer)
396395
if (peer) {
397396
int n = atomic_fetch_add_unless(&peer->usage, 1, 0);
398397
if (n > 0)
399-
trace_rxrpc_peer(peer, rxrpc_peer_got, n + 1, here);
398+
trace_rxrpc_peer(peer->debug_id, rxrpc_peer_got, n + 1, here);
400399
else
401400
peer = NULL;
402401
}
@@ -417,6 +416,7 @@ static void __rxrpc_put_peer(struct rxrpc_peer *peer)
417416
list_del_init(&peer->keepalive_link);
418417
spin_unlock_bh(&rxnet->peer_hash_lock);
419418

419+
rxrpc_put_local(peer->local);
420420
kfree_rcu(peer, rcu);
421421
}
422422

@@ -426,11 +426,13 @@ static void __rxrpc_put_peer(struct rxrpc_peer *peer)
426426
void rxrpc_put_peer(struct rxrpc_peer *peer)
427427
{
428428
const void *here = __builtin_return_address(0);
429+
unsigned int debug_id;
429430
int n;
430431

431432
if (peer) {
433+
debug_id = peer->debug_id;
432434
n = atomic_dec_return(&peer->usage);
433-
trace_rxrpc_peer(peer, rxrpc_peer_put, n, here);
435+
trace_rxrpc_peer(debug_id, rxrpc_peer_put, n, here);
434436
if (n == 0)
435437
__rxrpc_put_peer(peer);
436438
}
@@ -443,13 +445,15 @@ void rxrpc_put_peer(struct rxrpc_peer *peer)
443445
void rxrpc_put_peer_locked(struct rxrpc_peer *peer)
444446
{
445447
const void *here = __builtin_return_address(0);
448+
unsigned int debug_id = peer->debug_id;
446449
int n;
447450

448451
n = atomic_dec_return(&peer->usage);
449-
trace_rxrpc_peer(peer, rxrpc_peer_put, n, here);
452+
trace_rxrpc_peer(debug_id, rxrpc_peer_put, n, here);
450453
if (n == 0) {
451454
hash_del_rcu(&peer->hash_link);
452455
list_del_init(&peer->keepalive_link);
456+
rxrpc_put_local(peer->local);
453457
kfree_rcu(peer, rcu);
454458
}
455459
}

0 commit comments

Comments
 (0)