Skip to content

Commit

Permalink
Merge tag 'rxrpc-fixes-20191007' of git://git.kernel.org/pub/scm/linu…
Browse files Browse the repository at this point in the history
…x/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 <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Oct 7, 2019
2 parents 57acce3 + 91fcfbe commit 7359db6
Show file tree
Hide file tree
Showing 10 changed files with 63 additions and 44 deletions.
18 changes: 9 additions & 9 deletions include/trace/events/rxrpc.h
Original file line number Diff line number Diff line change
Expand Up @@ -519,10 +519,10 @@ TRACE_EVENT(rxrpc_local,
);

TRACE_EVENT(rxrpc_peer,
TP_PROTO(struct rxrpc_peer *peer, enum rxrpc_peer_trace op,
TP_PROTO(unsigned int peer_debug_id, enum rxrpc_peer_trace op,
int usage, const void *where),

TP_ARGS(peer, op, usage, where),
TP_ARGS(peer_debug_id, op, usage, where),

TP_STRUCT__entry(
__field(unsigned int, peer )
Expand All @@ -532,7 +532,7 @@ TRACE_EVENT(rxrpc_peer,
),

TP_fast_assign(
__entry->peer = peer->debug_id;
__entry->peer = peer_debug_id;
__entry->op = op;
__entry->usage = usage;
__entry->where = where;
Expand All @@ -546,10 +546,10 @@ TRACE_EVENT(rxrpc_peer,
);

TRACE_EVENT(rxrpc_conn,
TP_PROTO(struct rxrpc_connection *conn, enum rxrpc_conn_trace op,
TP_PROTO(unsigned int conn_debug_id, enum rxrpc_conn_trace op,
int usage, const void *where),

TP_ARGS(conn, op, usage, where),
TP_ARGS(conn_debug_id, op, usage, where),

TP_STRUCT__entry(
__field(unsigned int, conn )
Expand All @@ -559,7 +559,7 @@ TRACE_EVENT(rxrpc_conn,
),

TP_fast_assign(
__entry->conn = conn->debug_id;
__entry->conn = conn_debug_id;
__entry->op = op;
__entry->usage = usage;
__entry->where = where;
Expand Down Expand Up @@ -606,10 +606,10 @@ TRACE_EVENT(rxrpc_client,
);

TRACE_EVENT(rxrpc_call,
TP_PROTO(struct rxrpc_call *call, enum rxrpc_call_trace op,
TP_PROTO(unsigned int call_debug_id, enum rxrpc_call_trace op,
int usage, const void *where, const void *aux),

TP_ARGS(call, op, usage, where, aux),
TP_ARGS(call_debug_id, op, usage, where, aux),

TP_STRUCT__entry(
__field(unsigned int, call )
Expand All @@ -620,7 +620,7 @@ TRACE_EVENT(rxrpc_call,
),

TP_fast_assign(
__entry->call = call->debug_id;
__entry->call = call_debug_id;
__entry->op = op;
__entry->usage = usage;
__entry->where = where;
Expand Down
1 change: 1 addition & 0 deletions net/rxrpc/ar-internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,7 @@ struct rxrpc_call {
struct rxrpc_peer *peer; /* Peer record for remote address */
struct rxrpc_sock __rcu *socket; /* socket responsible */
struct rxrpc_net *rxnet; /* Network namespace to which call belongs */
const struct rxrpc_security *security; /* applied security module */
struct mutex user_mutex; /* User access mutex */
unsigned long ack_at; /* When deferred ACK needs to happen */
unsigned long ack_lost_at; /* When ACK is figured as lost */
Expand Down
5 changes: 3 additions & 2 deletions net/rxrpc/call_accept.c
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ static int rxrpc_service_prealloc_one(struct rxrpc_sock *rx,
smp_store_release(&b->conn_backlog_head,
(head + 1) & (size - 1));

trace_rxrpc_conn(conn, rxrpc_conn_new_service,
trace_rxrpc_conn(conn->debug_id, rxrpc_conn_new_service,
atomic_read(&conn->usage), here);
}

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

trace_rxrpc_call(call, rxrpc_call_new_service,
trace_rxrpc_call(call->debug_id, rxrpc_call_new_service,
atomic_read(&call->usage),
here, (const void *)user_call_ID);

Expand Down Expand Up @@ -307,6 +307,7 @@ static struct rxrpc_call *rxrpc_alloc_incoming_call(struct rxrpc_sock *rx,

rxrpc_see_call(call);
call->conn = conn;
call->security = conn->security;
call->peer = rxrpc_get_peer(conn->params.peer);
call->cong_cwnd = call->peer->cong_cwnd;
return call;
Expand Down
34 changes: 20 additions & 14 deletions net/rxrpc/call_object.c
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,8 @@ struct rxrpc_call *rxrpc_new_client_call(struct rxrpc_sock *rx,
if (p->intr)
__set_bit(RXRPC_CALL_IS_INTR, &call->flags);
call->tx_total_len = p->tx_total_len;
trace_rxrpc_call(call, rxrpc_call_new_client, atomic_read(&call->usage),
trace_rxrpc_call(call->debug_id, rxrpc_call_new_client,
atomic_read(&call->usage),
here, (const void *)p->user_call_ID);

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

trace_rxrpc_call(call, rxrpc_call_connected, atomic_read(&call->usage),
here, NULL);
trace_rxrpc_call(call->debug_id, rxrpc_call_connected,
atomic_read(&call->usage), here, NULL);

rxrpc_start_call_timer(call);

Expand All @@ -313,8 +314,8 @@ struct rxrpc_call *rxrpc_new_client_call(struct rxrpc_sock *rx,
error:
__rxrpc_set_call_completion(call, RXRPC_CALL_LOCAL_ERROR,
RX_CALL_DEAD, ret);
trace_rxrpc_call(call, rxrpc_call_error, atomic_read(&call->usage),
here, ERR_PTR(ret));
trace_rxrpc_call(call->debug_id, rxrpc_call_error,
atomic_read(&call->usage), here, ERR_PTR(ret));
rxrpc_release_call(rx, call);
mutex_unlock(&call->user_mutex);
rxrpc_put_call(call, rxrpc_call_put);
Expand Down Expand Up @@ -376,7 +377,8 @@ bool rxrpc_queue_call(struct rxrpc_call *call)
if (n == 0)
return false;
if (rxrpc_queue_work(&call->processor))
trace_rxrpc_call(call, rxrpc_call_queued, n + 1, here, NULL);
trace_rxrpc_call(call->debug_id, rxrpc_call_queued, n + 1,
here, NULL);
else
rxrpc_put_call(call, rxrpc_call_put_noqueue);
return true;
Expand All @@ -391,7 +393,8 @@ bool __rxrpc_queue_call(struct rxrpc_call *call)
int n = atomic_read(&call->usage);
ASSERTCMP(n, >=, 1);
if (rxrpc_queue_work(&call->processor))
trace_rxrpc_call(call, rxrpc_call_queued_ref, n, here, NULL);
trace_rxrpc_call(call->debug_id, rxrpc_call_queued_ref, n,
here, NULL);
else
rxrpc_put_call(call, rxrpc_call_put_noqueue);
return true;
Expand All @@ -406,7 +409,8 @@ void rxrpc_see_call(struct rxrpc_call *call)
if (call) {
int n = atomic_read(&call->usage);

trace_rxrpc_call(call, rxrpc_call_seen, n, here, NULL);
trace_rxrpc_call(call->debug_id, rxrpc_call_seen, n,
here, NULL);
}
}

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

trace_rxrpc_call(call, op, n, here, NULL);
trace_rxrpc_call(call->debug_id, op, n, here, NULL);
}

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

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

trace_rxrpc_call(call, rxrpc_call_release, atomic_read(&call->usage),
trace_rxrpc_call(call->debug_id, rxrpc_call_release,
atomic_read(&call->usage),
here, (const void *)call->flags);

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

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

if (conn) {
if (conn)
rxrpc_disconnect_call(call);
conn->security->free_call_crypto(call);
}
if (call->security)
call->security->free_call_crypto(call);

rxrpc_cleanup_ring(call);
_leave("");
Expand Down Expand Up @@ -534,12 +539,13 @@ void rxrpc_put_call(struct rxrpc_call *call, enum rxrpc_call_trace op)
{
struct rxrpc_net *rxnet = call->rxnet;
const void *here = __builtin_return_address(0);
unsigned int debug_id = call->debug_id;
int n;

ASSERT(call != NULL);

n = atomic_dec_return(&call->usage);
trace_rxrpc_call(call, op, n, here, NULL);
trace_rxrpc_call(debug_id, op, n, here, NULL);
ASSERTCMP(n, >=, 0);
if (n == 0) {
_debug("call %d dead", call->debug_id);
Expand Down
9 changes: 7 additions & 2 deletions net/rxrpc/conn_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,8 @@ rxrpc_alloc_client_connection(struct rxrpc_conn_parameters *cp, gfp_t gfp)
rxrpc_get_local(conn->params.local);
key_get(conn->params.key);

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

if (cp->exclusive) {
call->conn = candidate;
call->security = candidate->security;
call->security_ix = candidate->security_ix;
call->service_id = candidate->service_id;
_leave(" = 0 [exclusive %d]", candidate->debug_id);
Expand Down Expand Up @@ -403,6 +405,7 @@ static int rxrpc_get_client_conn(struct rxrpc_sock *rx,
candidate_published:
set_bit(RXRPC_CONN_IN_CLIENT_CONNS, &candidate->flags);
call->conn = candidate;
call->security = candidate->security;
call->security_ix = candidate->security_ix;
call->service_id = candidate->service_id;
spin_unlock(&local->client_conns_lock);
Expand All @@ -425,6 +428,7 @@ static int rxrpc_get_client_conn(struct rxrpc_sock *rx,

spin_lock(&conn->channel_lock);
call->conn = conn;
call->security = conn->security;
call->security_ix = conn->security_ix;
call->service_id = conn->service_id;
list_add_tail(&call->chan_wait_link, &conn->waiting_calls);
Expand Down Expand Up @@ -985,11 +989,12 @@ rxrpc_put_one_client_conn(struct rxrpc_connection *conn)
void rxrpc_put_client_conn(struct rxrpc_connection *conn)
{
const void *here = __builtin_return_address(0);
unsigned int debug_id = conn->debug_id;
int n;

do {
n = atomic_dec_return(&conn->usage);
trace_rxrpc_conn(conn, rxrpc_conn_put_client, n, here);
trace_rxrpc_conn(debug_id, rxrpc_conn_put_client, n, here);
if (n > 0)
return;
ASSERTCMP(n, >=, 0);
Expand Down
13 changes: 7 additions & 6 deletions net/rxrpc/conn_object.c
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ bool rxrpc_queue_conn(struct rxrpc_connection *conn)
if (n == 0)
return false;
if (rxrpc_queue_work(&conn->processor))
trace_rxrpc_conn(conn, rxrpc_conn_queued, n + 1, here);
trace_rxrpc_conn(conn->debug_id, rxrpc_conn_queued, n + 1, here);
else
rxrpc_put_connection(conn);
return true;
Expand All @@ -284,7 +284,7 @@ void rxrpc_see_connection(struct rxrpc_connection *conn)
if (conn) {
int n = atomic_read(&conn->usage);

trace_rxrpc_conn(conn, rxrpc_conn_seen, n, here);
trace_rxrpc_conn(conn->debug_id, rxrpc_conn_seen, n, here);
}
}

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

trace_rxrpc_conn(conn, rxrpc_conn_got, n, here);
trace_rxrpc_conn(conn->debug_id, rxrpc_conn_got, n, here);
}

/*
Expand All @@ -310,7 +310,7 @@ rxrpc_get_connection_maybe(struct rxrpc_connection *conn)
if (conn) {
int n = atomic_fetch_add_unless(&conn->usage, 1, 0);
if (n > 0)
trace_rxrpc_conn(conn, rxrpc_conn_got, n + 1, here);
trace_rxrpc_conn(conn->debug_id, rxrpc_conn_got, n + 1, here);
else
conn = NULL;
}
Expand All @@ -333,10 +333,11 @@ static void rxrpc_set_service_reap_timer(struct rxrpc_net *rxnet,
void rxrpc_put_service_conn(struct rxrpc_connection *conn)
{
const void *here = __builtin_return_address(0);
unsigned int debug_id = conn->debug_id;
int n;

n = atomic_dec_return(&conn->usage);
trace_rxrpc_conn(conn, rxrpc_conn_put_service, n, here);
trace_rxrpc_conn(debug_id, rxrpc_conn_put_service, n, here);
ASSERTCMP(n, >=, 0);
if (n == 1)
rxrpc_set_service_reap_timer(conn->params.local->rxnet,
Expand Down Expand Up @@ -420,7 +421,7 @@ void rxrpc_service_connection_reaper(struct work_struct *work)
*/
if (atomic_cmpxchg(&conn->usage, 1, 0) != 1)
continue;
trace_rxrpc_conn(conn, rxrpc_conn_reap_service, 0, NULL);
trace_rxrpc_conn(conn->debug_id, rxrpc_conn_reap_service, 0, NULL);

if (rxrpc_conn_is_client(conn))
BUG();
Expand Down
2 changes: 1 addition & 1 deletion net/rxrpc/conn_service.c
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ struct rxrpc_connection *rxrpc_prealloc_service_connection(struct rxrpc_net *rxn
list_add_tail(&conn->proc_link, &rxnet->conn_proc_list);
write_unlock(&rxnet->conn_lock);

trace_rxrpc_conn(conn, rxrpc_conn_new_service,
trace_rxrpc_conn(conn->debug_id, rxrpc_conn_new_service,
atomic_read(&conn->usage),
__builtin_return_address(0));
}
Expand Down
16 changes: 10 additions & 6 deletions net/rxrpc/peer_object.c
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ struct rxrpc_peer *rxrpc_alloc_peer(struct rxrpc_local *local, gfp_t gfp)
peer = kzalloc(sizeof(struct rxrpc_peer), gfp);
if (peer) {
atomic_set(&peer->usage, 1);
peer->local = local;
peer->local = rxrpc_get_local(local);
INIT_HLIST_HEAD(&peer->error_targets);
peer->service_conns = RB_ROOT;
seqlock_init(&peer->service_conn_lock);
Expand Down Expand Up @@ -307,7 +307,6 @@ void rxrpc_new_incoming_peer(struct rxrpc_sock *rx, struct rxrpc_local *local,
unsigned long hash_key;

hash_key = rxrpc_peer_hash_key(local, &peer->srx);
peer->local = local;
rxrpc_init_peer(rx, peer, hash_key);

spin_lock(&rxnet->peer_hash_lock);
Expand Down Expand Up @@ -382,7 +381,7 @@ struct rxrpc_peer *rxrpc_get_peer(struct rxrpc_peer *peer)
int n;

n = atomic_inc_return(&peer->usage);
trace_rxrpc_peer(peer, rxrpc_peer_got, n, here);
trace_rxrpc_peer(peer->debug_id, rxrpc_peer_got, n, here);
return peer;
}

Expand All @@ -396,7 +395,7 @@ struct rxrpc_peer *rxrpc_get_peer_maybe(struct rxrpc_peer *peer)
if (peer) {
int n = atomic_fetch_add_unless(&peer->usage, 1, 0);
if (n > 0)
trace_rxrpc_peer(peer, rxrpc_peer_got, n + 1, here);
trace_rxrpc_peer(peer->debug_id, rxrpc_peer_got, n + 1, here);
else
peer = NULL;
}
Expand All @@ -417,6 +416,7 @@ static void __rxrpc_put_peer(struct rxrpc_peer *peer)
list_del_init(&peer->keepalive_link);
spin_unlock_bh(&rxnet->peer_hash_lock);

rxrpc_put_local(peer->local);
kfree_rcu(peer, rcu);
}

Expand All @@ -426,11 +426,13 @@ static void __rxrpc_put_peer(struct rxrpc_peer *peer)
void rxrpc_put_peer(struct rxrpc_peer *peer)
{
const void *here = __builtin_return_address(0);
unsigned int debug_id;
int n;

if (peer) {
debug_id = peer->debug_id;
n = atomic_dec_return(&peer->usage);
trace_rxrpc_peer(peer, rxrpc_peer_put, n, here);
trace_rxrpc_peer(debug_id, rxrpc_peer_put, n, here);
if (n == 0)
__rxrpc_put_peer(peer);
}
Expand All @@ -443,13 +445,15 @@ void rxrpc_put_peer(struct rxrpc_peer *peer)
void rxrpc_put_peer_locked(struct rxrpc_peer *peer)
{
const void *here = __builtin_return_address(0);
unsigned int debug_id = peer->debug_id;
int n;

n = atomic_dec_return(&peer->usage);
trace_rxrpc_peer(peer, rxrpc_peer_put, n, here);
trace_rxrpc_peer(debug_id, rxrpc_peer_put, n, here);
if (n == 0) {
hash_del_rcu(&peer->hash_link);
list_del_init(&peer->keepalive_link);
rxrpc_put_local(peer->local);
kfree_rcu(peer, rcu);
}
}
Expand Down
Loading

0 comments on commit 7359db6

Please sign in to comment.