Skip to content

Commit

Permalink
rxrpc: Fix accept on a connection that need securing
Browse files Browse the repository at this point in the history 
When a new incoming call arrives at an userspace rxrpc socket on a new
connection that has a security class set, the code currently pushes it onto
the accept queue to hold a ref on it for the socket.  This doesn't work,
however, as recvmsg() pops it off, notices that it's in the SERVER_SECURING
state and discards the ref.  This means that the call runs out of refs too
early and the kernel oopses.

By contrast, a kernel rxrpc socket manually pre-charges the incoming call
pool with calls that already have user call IDs assigned, so they are ref'd
by the call tree on the socket.

Change the mode of operation for userspace rxrpc server sockets to work
like this too.  Although this is a UAPI change, server sockets aren't
currently functional.

Fixes: 248f219 ("rxrpc: Rewrite the data and ack handling code")
Signed-off-by: David Howells <dhowells@redhat.com>
  • Loading branch information
David Howells committed Oct 5, 2020
1 parent fa1d113 commit 2d914c1
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 281 deletions.
2 changes: 1 addition & 1 deletion include/uapi/linux/rxrpc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,11 @@ enum rxrpc_cmsg_type {
RXRPC_BUSY = 6, /* -r: server busy received [terminal] */
RXRPC_LOCAL_ERROR = 7, /* -r: local error generated [terminal] */
RXRPC_NEW_CALL = 8, /* -r: [Service] new incoming call notification */
RXRPC_ACCEPT = 9, /* s-: [Service] accept request */
RXRPC_EXCLUSIVE_CALL = 10, /* s-: Call should be on exclusive connection */
RXRPC_UPGRADE_SERVICE = 11, /* s-: Request service upgrade for client call */
RXRPC_TX_LENGTH = 12, /* s-: Total length of Tx data */
RXRPC_SET_CALL_TIMEOUT = 13, /* s-: Set one or more call timeouts */
RXRPC_CHARGE_ACCEPT = 14, /* s-: Charge the accept pool with a user call ID */
RXRPC__SUPPORTED
};

Expand Down
7 changes: 2 additions & 5 deletions net/rxrpc/ar-internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,6 @@ enum rxrpc_call_state {
RXRPC_CALL_CLIENT_RECV_REPLY, /* - client receiving reply phase */
RXRPC_CALL_SERVER_PREALLOC, /* - service preallocation */
RXRPC_CALL_SERVER_SECURING, /* - server securing request connection */
RXRPC_CALL_SERVER_ACCEPTING, /* - server accepting request */
RXRPC_CALL_SERVER_RECV_REQUEST, /* - server receiving request */
RXRPC_CALL_SERVER_ACK_REQUEST, /* - server pending ACK of request */
RXRPC_CALL_SERVER_SEND_REPLY, /* - server sending reply */
Expand Down Expand Up @@ -714,8 +713,8 @@ struct rxrpc_ack_summary {
enum rxrpc_command {
RXRPC_CMD_SEND_DATA, /* send data message */
RXRPC_CMD_SEND_ABORT, /* request abort generation */
RXRPC_CMD_ACCEPT, /* [server] accept incoming call */
RXRPC_CMD_REJECT_BUSY, /* [server] reject a call as busy */
RXRPC_CMD_CHARGE_ACCEPT, /* [server] charge accept preallocation */
};

struct rxrpc_call_params {
Expand Down Expand Up @@ -755,9 +754,7 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *,
struct rxrpc_sock *,
struct sk_buff *);
void rxrpc_accept_incoming_calls(struct rxrpc_local *);
struct rxrpc_call *rxrpc_accept_call(struct rxrpc_sock *, unsigned long,
rxrpc_notify_rx_t);
int rxrpc_reject_call(struct rxrpc_sock *);
int rxrpc_user_charge_accept(struct rxrpc_sock *, unsigned long);

/*
* call_event.c
Expand Down
263 changes: 38 additions & 225 deletions net/rxrpc/call_accept.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,9 @@ static int rxrpc_service_prealloc_one(struct rxrpc_sock *rx,
unsigned int debug_id)
{
const void *here = __builtin_return_address(0);
struct rxrpc_call *call;
struct rxrpc_call *call, *xcall;
struct rxrpc_net *rxnet = rxrpc_net(sock_net(&rx->sk));
struct rb_node *parent, **pp;
int max, tmp;
unsigned int size = RXRPC_BACKLOG_MAX;
unsigned int head, tail, call_head, call_tail;
Expand Down Expand Up @@ -94,7 +95,7 @@ static int rxrpc_service_prealloc_one(struct rxrpc_sock *rx,
}

/* Now it gets complicated, because calls get registered with the
* socket here, particularly if a user ID is preassigned by the user.
* socket here, with a user ID preassigned by the user.
*/
call = rxrpc_alloc_call(rx, gfp, debug_id);
if (!call)
Expand All @@ -107,34 +108,33 @@ static int rxrpc_service_prealloc_one(struct rxrpc_sock *rx,
here, (const void *)user_call_ID);

write_lock(&rx->call_lock);
if (user_attach_call) {
struct rxrpc_call *xcall;
struct rb_node *parent, **pp;

/* Check the user ID isn't already in use */
pp = &rx->calls.rb_node;
parent = NULL;
while (*pp) {
parent = *pp;
xcall = rb_entry(parent, struct rxrpc_call, sock_node);
if (user_call_ID < xcall->user_call_ID)
pp = &(*pp)->rb_left;
else if (user_call_ID > xcall->user_call_ID)
pp = &(*pp)->rb_right;
else
goto id_in_use;
}

call->user_call_ID = user_call_ID;
call->notify_rx = notify_rx;
/* Check the user ID isn't already in use */
pp = &rx->calls.rb_node;
parent = NULL;
while (*pp) {
parent = *pp;
xcall = rb_entry(parent, struct rxrpc_call, sock_node);
if (user_call_ID < xcall->user_call_ID)
pp = &(*pp)->rb_left;
else if (user_call_ID > xcall->user_call_ID)
pp = &(*pp)->rb_right;
else
goto id_in_use;
}

call->user_call_ID = user_call_ID;
call->notify_rx = notify_rx;
if (user_attach_call) {
rxrpc_get_call(call, rxrpc_call_got_kernel);
user_attach_call(call, user_call_ID);
rxrpc_get_call(call, rxrpc_call_got_userid);
rb_link_node(&call->sock_node, parent, pp);
rb_insert_color(&call->sock_node, &rx->calls);
set_bit(RXRPC_CALL_HAS_USERID, &call->flags);
}

rxrpc_get_call(call, rxrpc_call_got_userid);
rb_link_node(&call->sock_node, parent, pp);
rb_insert_color(&call->sock_node, &rx->calls);
set_bit(RXRPC_CALL_HAS_USERID, &call->flags);

list_add(&call->sock_link, &rx->sock_calls);

write_unlock(&rx->call_lock);
Expand All @@ -157,11 +157,8 @@ static int rxrpc_service_prealloc_one(struct rxrpc_sock *rx,
}

/*
* Preallocate sufficient service connections, calls and peers to cover the
* entire backlog of a socket. When a new call comes in, if we don't have
* sufficient of each available, the call gets rejected as busy or ignored.
*
* The backlog is replenished when a connection is accepted or rejected.
* Allocate the preallocation buffers for incoming service calls. These must
* be charged manually.
*/
int rxrpc_service_prealloc(struct rxrpc_sock *rx, gfp_t gfp)
{
Expand All @@ -174,13 +171,6 @@ int rxrpc_service_prealloc(struct rxrpc_sock *rx, gfp_t gfp)
rx->backlog = b;
}

if (rx->discard_new_call)
return 0;

while (rxrpc_service_prealloc_one(rx, b, NULL, NULL, 0, gfp,
atomic_inc_return(&rxrpc_debug_id)) == 0)
;

return 0;
}

Expand Down Expand Up @@ -333,6 +323,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->security_ix = conn->security_ix;
call->peer = rxrpc_get_peer(conn->params.peer);
call->cong_cwnd = call->peer->cong_cwnd;
return call;
Expand Down Expand Up @@ -402,8 +393,6 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,

if (rx->notify_new_call)
rx->notify_new_call(&rx->sk, call, call->user_call_ID);
else
sk_acceptq_added(&rx->sk);

spin_lock(&conn->state_lock);
switch (conn->state) {
Expand All @@ -415,12 +404,8 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,

case RXRPC_CONN_SERVICE:
write_lock(&call->state_lock);
if (call->state < RXRPC_CALL_COMPLETE) {
if (rx->discard_new_call)
call->state = RXRPC_CALL_SERVER_RECV_REQUEST;
else
call->state = RXRPC_CALL_SERVER_ACCEPTING;
}
if (call->state < RXRPC_CALL_COMPLETE)
call->state = RXRPC_CALL_SERVER_RECV_REQUEST;
write_unlock(&call->state_lock);
break;

Expand All @@ -440,9 +425,6 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,

rxrpc_send_ping(call, skb);

if (call->state == RXRPC_CALL_SERVER_ACCEPTING)
rxrpc_notify_socket(call);

/* We have to discard the prealloc queue's ref here and rely on a
* combination of the RCU read lock and refs held either by the socket
* (recvmsg queue, to-be-accepted queue or user ID tree) or the kernel
Expand All @@ -460,187 +442,18 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
}

/*
* handle acceptance of a call by userspace
* - assign the user call ID to the call at the front of the queue
* - called with the socket locked.
* Charge up socket with preallocated calls, attaching user call IDs.
*/
struct rxrpc_call *rxrpc_accept_call(struct rxrpc_sock *rx,
unsigned long user_call_ID,
rxrpc_notify_rx_t notify_rx)
__releases(&rx->sk.sk_lock.slock)
__acquires(call->user_mutex)
int rxrpc_user_charge_accept(struct rxrpc_sock *rx, unsigned long user_call_ID)
{
struct rxrpc_call *call;
struct rb_node *parent, **pp;
int ret;

_enter(",%lx", user_call_ID);

ASSERT(!irqs_disabled());

write_lock(&rx->call_lock);

if (list_empty(&rx->to_be_accepted)) {
write_unlock(&rx->call_lock);
release_sock(&rx->sk);
kleave(" = -ENODATA [empty]");
return ERR_PTR(-ENODATA);
}

/* check the user ID isn't already in use */
pp = &rx->calls.rb_node;
parent = NULL;
while (*pp) {
parent = *pp;
call = rb_entry(parent, struct rxrpc_call, sock_node);

if (user_call_ID < call->user_call_ID)
pp = &(*pp)->rb_left;
else if (user_call_ID > call->user_call_ID)
pp = &(*pp)->rb_right;
else
goto id_in_use;
}

/* Dequeue the first call and check it's still valid. We gain
* responsibility for the queue's reference.
*/
call = list_entry(rx->to_be_accepted.next,
struct rxrpc_call, accept_link);
write_unlock(&rx->call_lock);

/* We need to gain the mutex from the interrupt handler without
* upsetting lockdep, so we have to release it there and take it here.
* We are, however, still holding the socket lock, so other accepts
* must wait for us and no one can add the user ID behind our backs.
*/
if (mutex_lock_interruptible(&call->user_mutex) < 0) {
release_sock(&rx->sk);
kleave(" = -ERESTARTSYS");
return ERR_PTR(-ERESTARTSYS);
}

write_lock(&rx->call_lock);
list_del_init(&call->accept_link);
sk_acceptq_removed(&rx->sk);
rxrpc_see_call(call);

/* Find the user ID insertion point. */
pp = &rx->calls.rb_node;
parent = NULL;
while (*pp) {
parent = *pp;
call = rb_entry(parent, struct rxrpc_call, sock_node);

if (user_call_ID < call->user_call_ID)
pp = &(*pp)->rb_left;
else if (user_call_ID > call->user_call_ID)
pp = &(*pp)->rb_right;
else
BUG();
}

write_lock_bh(&call->state_lock);
switch (call->state) {
case RXRPC_CALL_SERVER_ACCEPTING:
call->state = RXRPC_CALL_SERVER_RECV_REQUEST;
break;
case RXRPC_CALL_COMPLETE:
ret = call->error;
goto out_release;
default:
BUG();
}

/* formalise the acceptance */
call->notify_rx = notify_rx;
call->user_call_ID = user_call_ID;
rxrpc_get_call(call, rxrpc_call_got_userid);
rb_link_node(&call->sock_node, parent, pp);
rb_insert_color(&call->sock_node, &rx->calls);
if (test_and_set_bit(RXRPC_CALL_HAS_USERID, &call->flags))
BUG();

write_unlock_bh(&call->state_lock);
write_unlock(&rx->call_lock);
rxrpc_notify_socket(call);
rxrpc_service_prealloc(rx, GFP_KERNEL);
release_sock(&rx->sk);
_leave(" = %p{%d}", call, call->debug_id);
return call;

out_release:
_debug("release %p", call);
write_unlock_bh(&call->state_lock);
write_unlock(&rx->call_lock);
rxrpc_release_call(rx, call);
rxrpc_put_call(call, rxrpc_call_put);
goto out;

id_in_use:
ret = -EBADSLT;
write_unlock(&rx->call_lock);
out:
rxrpc_service_prealloc(rx, GFP_KERNEL);
release_sock(&rx->sk);
_leave(" = %d", ret);
return ERR_PTR(ret);
}

/*
* Handle rejection of a call by userspace
* - reject the call at the front of the queue
*/
int rxrpc_reject_call(struct rxrpc_sock *rx)
{
struct rxrpc_call *call;
bool abort = false;
int ret;

_enter("");

ASSERT(!irqs_disabled());

write_lock(&rx->call_lock);

if (list_empty(&rx->to_be_accepted)) {
write_unlock(&rx->call_lock);
return -ENODATA;
}

/* Dequeue the first call and check it's still valid. We gain
* responsibility for the queue's reference.
*/
call = list_entry(rx->to_be_accepted.next,
struct rxrpc_call, accept_link);
list_del_init(&call->accept_link);
sk_acceptq_removed(&rx->sk);
rxrpc_see_call(call);
struct rxrpc_backlog *b = rx->backlog;

write_lock_bh(&call->state_lock);
switch (call->state) {
case RXRPC_CALL_SERVER_ACCEPTING:
__rxrpc_abort_call("REJ", call, 1, RX_USER_ABORT, -ECONNABORTED);
abort = true;
fallthrough;
case RXRPC_CALL_COMPLETE:
ret = call->error;
goto out_discard;
default:
BUG();
}
if (rx->sk.sk_state == RXRPC_CLOSE)
return -ESHUTDOWN;

out_discard:
write_unlock_bh(&call->state_lock);
write_unlock(&rx->call_lock);
if (abort) {
rxrpc_send_abort_packet(call);
rxrpc_release_call(rx, call);
rxrpc_put_call(call, rxrpc_call_put);
}
rxrpc_service_prealloc(rx, GFP_KERNEL);
_leave(" = %d", ret);
return ret;
return rxrpc_service_prealloc_one(rx, b, NULL, NULL, user_call_ID,
GFP_KERNEL,
atomic_inc_return(&rxrpc_debug_id));
}

/*
Expand Down
Loading

0 comments on commit 2d914c1

Please sign in to comment.