RxRPC fixes

-----BEGIN PGP SIGNATURE-----
 
 iQIzBAABCAAdFiEEqG5UsNXhtOCrfGQP+7dXa6fLC2sFAl39UCAACgkQ+7dXa6fL
 C2sZhw//S7+XNXpA9HfwUO/Vic9jUFi4FRPkS+zqHUCJQpbC32+Gj1L13J++EEJm
 aIaf8yo5DSNIKRaja+d4UTmZFgXOOg64Jq/p9ZOltdomS+ZihGWoigKHxYQtLfin
 fsBVbPqMO+PSZbSqcwy7inMKfJ1EkFXiu5lhI5o4Swc3gw9w9HTui7u5c02Wol8L
 2ooI2hiuali9FSJ592UfvHoxK3auNUJ6oB2aA+EFuLhMLz+8hhnVCNELsaWoLDOh
 qZkI0D+X5w61MsYiND3jOB85pl+d8ADKPlMw/WM1hIwn/ODPvkqnGHVhOslB7/fh
 dBsNUcGhAsdGz9jBDRXcUv5ak4G/wL1IiV5CRJ0xSX6I5lTm+byIRE2rqtO5mkQg
 ESXCGGodbxFh7EIjjWskdTtusQN+O+v78GLue7h9KVLrsTvzQRT99BIAJLvT6rGM
 F6I/Zirj1ZbRhn7uwkr+hwoD4CJvtF/mvImg0P2iPQmUrhEUJkE5VRVIDf/OnBd0
 vGQJG7IKr5tUK+x3ztk34H3pqIakfQ03FX0mqdTFuoVYmaBlooSk9Do7xNTUHi2o
 2rC3R5S1alRBqFBOKIz5dL6SK8L1wcLCgSzC1pI+q9bcusX6yjW0b3vMn6tzCUHA
 eb1NM0jp7VXx5yN88pfi7U7JFxafcCMoKKBodvdLgnL06mfHnJc=
 =vwQ0
 -----END PGP SIGNATURE-----

Merge tag 'rxrpc-fixes-20191220' of git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs

David Howells says:

====================
rxrpc: Fixes

Here are a couple of bugfixes plus a patch that makes one of the bugfixes
easier:

 (1) Move the ping and mutex unlock on a new call from rxrpc_input_packet()
     into rxrpc_new_incoming_call(), which it calls.  This means the
     lock-unlock section is entirely within the latter function.  This
     simplifies patch (2).

 (2) Don't take the call->user_mutex at all in the softirq path.  Mutexes
     aren't allowed to be taken or released there and a patch was merged
     that caused a warning to be emitted every time this happened.  Looking
     at the code again, it looks like that taking the mutex isn't actually
     necessary, as the value of call->state will block access to the call.

 (3) Fix the incoming call path to check incoming calls earlier to reject
     calls to RPC services for which we don't have a security key of the
     appropriate class.  This avoids an assertion failure if YFS tries
     making a secure call to the kafs cache manager RPC service.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
David S. Miller 2019-12-24 16:12:47 -08:00
commit ff43ae4bd5
7 changed files with 85 additions and 98 deletions

View File

@ -209,6 +209,7 @@ struct rxrpc_skb_priv {
struct rxrpc_security {
const char *name; /* name of this service */
u8 security_index; /* security type provided */
u32 no_key_abort; /* Abort code indicating no key */
/* Initialise a security service */
int (*init)(void);
@ -977,8 +978,9 @@ static inline void rxrpc_reduce_conn_timer(struct rxrpc_connection *conn,
struct rxrpc_connection *rxrpc_find_service_conn_rcu(struct rxrpc_peer *,
struct sk_buff *);
struct rxrpc_connection *rxrpc_prealloc_service_connection(struct rxrpc_net *, gfp_t);
void rxrpc_new_incoming_connection(struct rxrpc_sock *,
struct rxrpc_connection *, struct sk_buff *);
void rxrpc_new_incoming_connection(struct rxrpc_sock *, struct rxrpc_connection *,
const struct rxrpc_security *, struct key *,
struct sk_buff *);
void rxrpc_unpublish_service_conn(struct rxrpc_connection *);
/*
@ -1103,7 +1105,9 @@ extern const struct rxrpc_security rxkad;
int __init rxrpc_init_security(void);
void rxrpc_exit_security(void);
int rxrpc_init_client_conn_security(struct rxrpc_connection *);
int rxrpc_init_server_conn_security(struct rxrpc_connection *);
bool rxrpc_look_up_server_security(struct rxrpc_local *, struct rxrpc_sock *,
const struct rxrpc_security **, struct key **,
struct sk_buff *);
/*
* sendmsg.c

View File

@ -239,6 +239,22 @@ void rxrpc_discard_prealloc(struct rxrpc_sock *rx)
kfree(b);
}
/*
* Ping the other end to fill our RTT cache and to retrieve the rwind
* and MTU parameters.
*/
static void rxrpc_send_ping(struct rxrpc_call *call, struct sk_buff *skb)
{
struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
ktime_t now = skb->tstamp;
if (call->peer->rtt_usage < 3 ||
ktime_before(ktime_add_ms(call->peer->rtt_last_req, 1000), now))
rxrpc_propose_ACK(call, RXRPC_ACK_PING, sp->hdr.serial,
true, true,
rxrpc_propose_ack_ping_for_params);
}
/*
* Allocate a new incoming call from the prealloc pool, along with a connection
* and a peer as necessary.
@ -247,6 +263,8 @@ static struct rxrpc_call *rxrpc_alloc_incoming_call(struct rxrpc_sock *rx,
struct rxrpc_local *local,
struct rxrpc_peer *peer,
struct rxrpc_connection *conn,
const struct rxrpc_security *sec,
struct key *key,
struct sk_buff *skb)
{
struct rxrpc_backlog *b = rx->backlog;
@ -294,7 +312,7 @@ static struct rxrpc_call *rxrpc_alloc_incoming_call(struct rxrpc_sock *rx,
conn->params.local = rxrpc_get_local(local);
conn->params.peer = peer;
rxrpc_see_connection(conn);
rxrpc_new_incoming_connection(rx, conn, skb);
rxrpc_new_incoming_connection(rx, conn, sec, key, skb);
} else {
rxrpc_get_connection(conn);
}
@ -333,9 +351,11 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
struct sk_buff *skb)
{
struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
const struct rxrpc_security *sec = NULL;
struct rxrpc_connection *conn;
struct rxrpc_peer *peer = NULL;
struct rxrpc_call *call;
struct rxrpc_call *call = NULL;
struct key *key = NULL;
_enter("");
@ -346,9 +366,7 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
sp->hdr.seq, RX_INVALID_OPERATION, ESHUTDOWN);
skb->mark = RXRPC_SKB_MARK_REJECT_ABORT;
skb->priority = RX_INVALID_OPERATION;
_leave(" = NULL [close]");
call = NULL;
goto out;
goto no_call;
}
/* The peer, connection and call may all have sprung into existence due
@ -358,29 +376,19 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
*/
conn = rxrpc_find_connection_rcu(local, skb, &peer);
call = rxrpc_alloc_incoming_call(rx, local, peer, conn, skb);
if (!conn && !rxrpc_look_up_server_security(local, rx, &sec, &key, skb))
goto no_call;
call = rxrpc_alloc_incoming_call(rx, local, peer, conn, sec, key, skb);
key_put(key);
if (!call) {
skb->mark = RXRPC_SKB_MARK_REJECT_BUSY;
_leave(" = NULL [busy]");
call = NULL;
goto out;
goto no_call;
}
trace_rxrpc_receive(call, rxrpc_receive_incoming,
sp->hdr.serial, sp->hdr.seq);
/* Lock the call to prevent rxrpc_kernel_send/recv_data() and
* sendmsg()/recvmsg() inconveniently stealing the mutex once the
* notification is generated.
*
* The BUG should never happen because the kernel should be well
* behaved enough not to access the call before the first notification
* event and userspace is prevented from doing so until the state is
* appropriate.
*/
if (!mutex_trylock(&call->user_mutex))
BUG();
/* Make the call live. */
rxrpc_incoming_call(rx, call, skb);
conn = call->conn;
@ -421,6 +429,9 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
BUG();
}
spin_unlock(&conn->state_lock);
spin_unlock(&rx->incoming_lock);
rxrpc_send_ping(call, skb);
if (call->state == RXRPC_CALL_SERVER_ACCEPTING)
rxrpc_notify_socket(call);
@ -433,9 +444,12 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
rxrpc_put_call(call, rxrpc_call_put);
_leave(" = %p{%d}", call, call->debug_id);
out:
spin_unlock(&rx->incoming_lock);
return call;
no_call:
spin_unlock(&rx->incoming_lock);
_leave(" = NULL [%u]", skb->mark);
return NULL;
}
/*

View File

@ -376,21 +376,7 @@ static void rxrpc_secure_connection(struct rxrpc_connection *conn)
_enter("{%d}", conn->debug_id);
ASSERT(conn->security_ix != 0);
if (!conn->params.key) {
_debug("set up security");
ret = rxrpc_init_server_conn_security(conn);
switch (ret) {
case 0:
break;
case -ENOENT:
abort_code = RX_CALL_DEAD;
goto abort;
default:
abort_code = RXKADNOAUTH;
goto abort;
}
}
ASSERT(conn->server_key);
if (conn->security->issue_challenge(conn) < 0) {
abort_code = RX_CALL_DEAD;

View File

@ -148,6 +148,8 @@ struct rxrpc_connection *rxrpc_prealloc_service_connection(struct rxrpc_net *rxn
*/
void rxrpc_new_incoming_connection(struct rxrpc_sock *rx,
struct rxrpc_connection *conn,
const struct rxrpc_security *sec,
struct key *key,
struct sk_buff *skb)
{
struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
@ -160,6 +162,8 @@ void rxrpc_new_incoming_connection(struct rxrpc_sock *rx,
conn->service_id = sp->hdr.serviceId;
conn->security_ix = sp->hdr.securityIndex;
conn->out_clientflag = 0;
conn->security = sec;
conn->server_key = key_get(key);
if (conn->security_ix)
conn->state = RXRPC_CONN_SERVICE_UNSECURED;
else

View File

@ -192,22 +192,6 @@ static void rxrpc_congestion_management(struct rxrpc_call *call,
goto out_no_clear_ca;
}
/*
* Ping the other end to fill our RTT cache and to retrieve the rwind
* and MTU parameters.
*/
static void rxrpc_send_ping(struct rxrpc_call *call, struct sk_buff *skb)
{
struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
ktime_t now = skb->tstamp;
if (call->peer->rtt_usage < 3 ||
ktime_before(ktime_add_ms(call->peer->rtt_last_req, 1000), now))
rxrpc_propose_ACK(call, RXRPC_ACK_PING, sp->hdr.serial,
true, true,
rxrpc_propose_ack_ping_for_params);
}
/*
* Apply a hard ACK by advancing the Tx window.
*/
@ -1396,8 +1380,6 @@ int rxrpc_input_packet(struct sock *udp_sk, struct sk_buff *skb)
call = rxrpc_new_incoming_call(local, rx, skb);
if (!call)
goto reject_packet;
rxrpc_send_ping(call, skb);
mutex_unlock(&call->user_mutex);
}
/* Process a call packet; this either discards or passes on the ref

View File

@ -648,9 +648,9 @@ static int rxkad_issue_challenge(struct rxrpc_connection *conn)
u32 serial;
int ret;
_enter("{%d,%x}", conn->debug_id, key_serial(conn->params.key));
_enter("{%d,%x}", conn->debug_id, key_serial(conn->server_key));
ret = key_validate(conn->params.key);
ret = key_validate(conn->server_key);
if (ret < 0)
return ret;
@ -1293,6 +1293,7 @@ static void rxkad_exit(void)
const struct rxrpc_security rxkad = {
.name = "rxkad",
.security_index = RXRPC_SECURITY_RXKAD,
.no_key_abort = RXKADUNKNOWNKEY,
.init = rxkad_init,
.exit = rxkad_exit,
.init_connection_security = rxkad_init_connection_security,

View File

@ -101,62 +101,58 @@ int rxrpc_init_client_conn_security(struct rxrpc_connection *conn)
}
/*
* initialise the security on a server connection
* Find the security key for a server connection.
*/
int rxrpc_init_server_conn_security(struct rxrpc_connection *conn)
bool rxrpc_look_up_server_security(struct rxrpc_local *local, struct rxrpc_sock *rx,
const struct rxrpc_security **_sec,
struct key **_key,
struct sk_buff *skb)
{
const struct rxrpc_security *sec;
struct rxrpc_local *local = conn->params.local;
struct rxrpc_sock *rx;
struct key *key;
key_ref_t kref;
struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
key_ref_t kref = NULL;
char kdesc[5 + 1 + 3 + 1];
_enter("");
sprintf(kdesc, "%u:%u", conn->service_id, conn->security_ix);
sprintf(kdesc, "%u:%u", sp->hdr.serviceId, sp->hdr.securityIndex);
sec = rxrpc_security_lookup(conn->security_ix);
sec = rxrpc_security_lookup(sp->hdr.securityIndex);
if (!sec) {
_leave(" = -ENOKEY [lookup]");
return -ENOKEY;
trace_rxrpc_abort(0, "SVS",
sp->hdr.cid, sp->hdr.callNumber, sp->hdr.seq,
RX_INVALID_OPERATION, EKEYREJECTED);
skb->mark = RXRPC_SKB_MARK_REJECT_ABORT;
skb->priority = RX_INVALID_OPERATION;
return false;
}
/* find the service */
read_lock(&local->services_lock);
rx = rcu_dereference_protected(local->service,
lockdep_is_held(&local->services_lock));
if (rx && (rx->srx.srx_service == conn->service_id ||
rx->second_service == conn->service_id))
goto found_service;
if (sp->hdr.securityIndex == RXRPC_SECURITY_NONE)
goto out;
/* the service appears to have died */
read_unlock(&local->services_lock);
_leave(" = -ENOENT");
return -ENOENT;
found_service:
if (!rx->securities) {
read_unlock(&local->services_lock);
_leave(" = -ENOKEY");
return -ENOKEY;
trace_rxrpc_abort(0, "SVR",
sp->hdr.cid, sp->hdr.callNumber, sp->hdr.seq,
RX_INVALID_OPERATION, EKEYREJECTED);
skb->mark = RXRPC_SKB_MARK_REJECT_ABORT;
skb->priority = RX_INVALID_OPERATION;
return false;
}
/* look through the service's keyring */
kref = keyring_search(make_key_ref(rx->securities, 1UL),
&key_type_rxrpc_s, kdesc, true);
if (IS_ERR(kref)) {
read_unlock(&local->services_lock);
_leave(" = %ld [search]", PTR_ERR(kref));
return PTR_ERR(kref);
trace_rxrpc_abort(0, "SVK",
sp->hdr.cid, sp->hdr.callNumber, sp->hdr.seq,
sec->no_key_abort, EKEYREJECTED);
skb->mark = RXRPC_SKB_MARK_REJECT_ABORT;
skb->priority = sec->no_key_abort;
return false;
}
key = key_ref_to_ptr(kref);
read_unlock(&local->services_lock);
conn->server_key = key;
conn->security = sec;
_leave(" = 0");
return 0;
out:
*_sec = sec;
*_key = key_ref_to_ptr(kref);
return true;
}