Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 49b538e7 authored by David S. Miller's avatar David S. Miller
Browse files


David Howells says:

====================
rxrpc: Fix packet reception code

Here are a set of patches that prepares for and fix problems in rxrpc's
package reception code.  There serious problems are:

 (A) There's a window between binding the socket and setting the data_ready
     hook in which packets can find their way into the UDP socket's receive
     queues.

 (B) The skb_recv_udp() will return an error (and clear the error state) if
     there was an error on the Tx side.  rxrpc doesn't handle this.

 (C) The rxrpc data_ready handler doesn't fully drain the UDP receive
     queue.

 (D) The rxrpc data_ready handler assumes it is called in a non-reentrant
 state.

The second patch fixes (A) - (C); the third patch renders (B) and (C)
non-issues by using the recap_rcv hook instead of data_ready - and the
final patch fixes (D).  That last is the most complex.

The preparatory patches are:

 (1) Fix some places that are doing things in the wrong net namespace.

 (2) Stop taking the rcu read lock as it's held by the IP input routine in
     the call chain.

 (3) Only end the Tx phase if *we* rotated the final packet out of the Tx
     buffer.

 (4) Don't assume that the call state won't change after dropping the
     call_state lock.

 (5) Only take receive window and MTU suze parameters from an ACK packet if
     it's the latest ACK packet.

 (6) Record connection-level abort information correctly.

 (7) Fix a trace line.

And then there are three main patches - note that these are mixed in with
the preparatory patches somewhat:

 (1) Fix the setup window (A), skb_recv_udp() error check (B) and packet
     drainage (C).

 (2) Switch to using the encap_rcv instead of data_ready to cut out the
     effects of the UDP read queues and get the packets delivered directly.

 (3) Add more locking into the various packet input paths to defend against
     re-entrance (D).
====================

Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 9a4890bd c1e15b49
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -931,6 +931,7 @@ TRACE_EVENT(rxrpc_tx_packet,
	    TP_fast_assign(
		    __entry->call = call_id;
		    memcpy(&__entry->whdr, whdr, sizeof(__entry->whdr));
		    __entry->where = where;
			   ),

	    TP_printk("c=%08x %08x:%08x:%08x:%04x %08x %08x %02x %02x %s %s",
+1 −0
Original line number Diff line number Diff line
@@ -40,5 +40,6 @@ struct udphdr {
#define UDP_ENCAP_L2TPINUDP	3 /* rfc2661 */
#define UDP_ENCAP_GTP0		4 /* GSM TS 09.60 */
#define UDP_ENCAP_GTP1U		5 /* 3GPP TS 29.060 */
#define UDP_ENCAP_RXRPC		6

#endif /* _UAPI_LINUX_UDP_H */
+13 −10
Original line number Diff line number Diff line
@@ -302,6 +302,7 @@ struct rxrpc_peer {

	/* calculated RTT cache */
#define RXRPC_RTT_CACHE_SIZE 32
	spinlock_t		rtt_input_lock;	/* RTT lock for input routine */
	ktime_t			rtt_last_req;	/* Time of last RTT request */
	u64			rtt;		/* Current RTT estimate (in nS) */
	u64			rtt_sum;	/* Sum of cache contents */
@@ -442,17 +443,17 @@ struct rxrpc_connection {
	spinlock_t		state_lock;	/* state-change lock */
	enum rxrpc_conn_cache_state cache_state;
	enum rxrpc_conn_proto_state state;	/* current state of connection */
	u32			local_abort;	/* local abort code */
	u32			remote_abort;	/* remote abort code */
	u32			abort_code;	/* Abort code of connection abort */
	int			debug_id;	/* debug ID for printks */
	atomic_t		serial;		/* packet serial number counter */
	unsigned int		hi_serial;	/* highest serial number received */
	u32			security_nonce;	/* response re-use preventer */
	u16			service_id;	/* Service ID, possibly upgraded */
	u32			service_id;	/* Service ID, possibly upgraded */
	u8			size_align;	/* data size alignment (for security) */
	u8			security_size;	/* security header size */
	u8			security_ix;	/* security type */
	u8			out_clientflag;	/* RXRPC_CLIENT_INITIATED if we are client */
	short			error;		/* Local error code */
};

static inline bool rxrpc_to_server(const struct rxrpc_skb_priv *sp)
@@ -635,6 +636,8 @@ struct rxrpc_call {
	bool			tx_phase;	/* T if transmission phase, F if receive phase */
	u8			nr_jumbo_bad;	/* Number of jumbo dups/exceeds-windows */

	spinlock_t		input_lock;	/* Lock for packet input to this call */

	/* receive-phase ACK management */
	u8			ackr_reason;	/* reason to ACK */
	u16			ackr_skew;	/* skew on packet being ACK'd */
@@ -720,8 +723,6 @@ int rxrpc_service_prealloc(struct rxrpc_sock *, gfp_t);
void rxrpc_discard_prealloc(struct rxrpc_sock *);
struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *,
					   struct rxrpc_sock *,
					   struct rxrpc_peer *,
					   struct rxrpc_connection *,
					   struct sk_buff *);
void rxrpc_accept_incoming_calls(struct rxrpc_local *);
struct rxrpc_call *rxrpc_accept_call(struct rxrpc_sock *, unsigned long,
@@ -891,8 +892,9 @@ extern unsigned long rxrpc_conn_idle_client_fast_expiry;
extern struct idr rxrpc_client_conn_ids;

void rxrpc_destroy_client_conn_ids(void);
int rxrpc_connect_call(struct rxrpc_call *, struct rxrpc_conn_parameters *,
		       struct sockaddr_rxrpc *, gfp_t);
int rxrpc_connect_call(struct rxrpc_sock *, struct rxrpc_call *,
		       struct rxrpc_conn_parameters *, struct sockaddr_rxrpc *,
		       gfp_t);
void rxrpc_expose_client_call(struct rxrpc_call *);
void rxrpc_disconnect_client_call(struct rxrpc_call *);
void rxrpc_put_client_conn(struct rxrpc_connection *);
@@ -965,7 +967,7 @@ void rxrpc_unpublish_service_conn(struct rxrpc_connection *);
/*
 * input.c
 */
void rxrpc_data_ready(struct sock *);
int rxrpc_input_packet(struct sock *, struct sk_buff *);

/*
 * insecure.c
@@ -1045,10 +1047,11 @@ void rxrpc_peer_keepalive_worker(struct work_struct *);
 */
struct rxrpc_peer *rxrpc_lookup_peer_rcu(struct rxrpc_local *,
					 const struct sockaddr_rxrpc *);
struct rxrpc_peer *rxrpc_lookup_peer(struct rxrpc_local *,
struct rxrpc_peer *rxrpc_lookup_peer(struct rxrpc_sock *, struct rxrpc_local *,
				     struct sockaddr_rxrpc *, gfp_t);
struct rxrpc_peer *rxrpc_alloc_peer(struct rxrpc_local *, gfp_t);
void rxrpc_new_incoming_peer(struct rxrpc_local *, struct rxrpc_peer *);
void rxrpc_new_incoming_peer(struct rxrpc_sock *, struct rxrpc_local *,
			     struct rxrpc_peer *);
void rxrpc_destroy_all_peers(struct rxrpc_net *);
struct rxrpc_peer *rxrpc_get_peer(struct rxrpc_peer *);
struct rxrpc_peer *rxrpc_get_peer_maybe(struct rxrpc_peer *);
+18 −9
Original line number Diff line number Diff line
@@ -287,7 +287,7 @@ static struct rxrpc_call *rxrpc_alloc_incoming_call(struct rxrpc_sock *rx,
					  (peer_tail + 1) &
					  (RXRPC_BACKLOG_MAX - 1));

			rxrpc_new_incoming_peer(local, peer);
			rxrpc_new_incoming_peer(rx, local, peer);
		}

		/* Now allocate and set up the connection */
@@ -333,11 +333,11 @@ static struct rxrpc_call *rxrpc_alloc_incoming_call(struct rxrpc_sock *rx,
 */
struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
					   struct rxrpc_sock *rx,
					   struct rxrpc_peer *peer,
					   struct rxrpc_connection *conn,
					   struct sk_buff *skb)
{
	struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
	struct rxrpc_connection *conn;
	struct rxrpc_peer *peer;
	struct rxrpc_call *call;

	_enter("");
@@ -354,6 +354,13 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
		goto out;
	}

	/* The peer, connection and call may all have sprung into existence due
	 * to a duplicate packet being handled on another CPU in parallel, so
	 * we have to recheck the routing.  However, we're now holding
	 * rx->incoming_lock, so the values should remain stable.
	 */
	conn = rxrpc_find_connection_rcu(local, skb, &peer);

	call = rxrpc_alloc_incoming_call(rx, local, peer, conn, skb);
	if (!call) {
		skb->mark = RXRPC_SKB_MARK_REJECT_BUSY;
@@ -396,20 +403,22 @@ 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;
		}
		write_unlock(&call->state_lock);
		break;

	case RXRPC_CONN_REMOTELY_ABORTED:
		rxrpc_set_call_completion(call, RXRPC_CALL_REMOTELY_ABORTED,
					  conn->remote_abort, -ECONNABORTED);
					  conn->abort_code, conn->error);
		break;
	case RXRPC_CONN_LOCALLY_ABORTED:
		rxrpc_abort_call("CON", call, sp->hdr.seq,
				 conn->local_abort, -ECONNABORTED);
				 conn->abort_code, conn->error);
		break;
	default:
		BUG();
+3 −2
Original line number Diff line number Diff line
@@ -138,6 +138,7 @@ struct rxrpc_call *rxrpc_alloc_call(struct rxrpc_sock *rx, gfp_t gfp,
	init_waitqueue_head(&call->waitq);
	spin_lock_init(&call->lock);
	spin_lock_init(&call->notify_lock);
	spin_lock_init(&call->input_lock);
	rwlock_init(&call->state_lock);
	atomic_set(&call->usage, 1);
	call->debug_id = debug_id;
@@ -287,7 +288,7 @@ struct rxrpc_call *rxrpc_new_client_call(struct rxrpc_sock *rx,
	/* Set up or get a connection record and set the protocol parameters,
	 * including channel number and call ID.
	 */
	ret = rxrpc_connect_call(call, cp, srx, gfp);
	ret = rxrpc_connect_call(rx, call, cp, srx, gfp);
	if (ret < 0)
		goto error;

@@ -339,7 +340,7 @@ int rxrpc_retry_client_call(struct rxrpc_sock *rx,
	/* Set up or get a connection record and set the protocol parameters,
	 * including channel number and call ID.
	 */
	ret = rxrpc_connect_call(call, cp, srx, gfp);
	ret = rxrpc_connect_call(rx, call, cp, srx, gfp);
	if (ret < 0)
		goto error;

Loading