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

Commit e653181d authored by James Chapman's avatar James Chapman Committed by David S. Miller
Browse files

[PPPOL2TP]: Fix SMP issues in skb reorder queue handling 



When walking a session's packet reorder queue, use
skb_queue_walk_safe() since the list could be modified inside the
loop.

Rearrange the unlinking skbs from the reorder queue such that it is
done while the queue lock is held in pppol2tp_recv_dequeue() when
walking the skb list.

A version of this patch was suggested by Jarek Poplawski.

Signed-off-by: default avatarJames Chapman <jchapman@katalix.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent cf3752e2

drivers/net/pppol2tp.c

+8 −3
Original line number Original line Diff line number Diff line
@@ -342,10 +342,11 @@ static struct pppol2tp_tunnel *pppol2tp_tunnel_find(u16 tunnel_id) 
static void pppol2tp_recv_queue_skb(struct pppol2tp_session *session, struct sk_buff *skb)

static void pppol2tp_recv_queue_skb(struct pppol2tp_session *session, struct sk_buff *skb)

{

{

	struct sk_buff *skbp;

	struct sk_buff *skbp;

	struct sk_buff *tmp;

	u16 ns = PPPOL2TP_SKB_CB(skb)->ns;

	u16 ns = PPPOL2TP_SKB_CB(skb)->ns;





	spin_lock_bh(&session->reorder_q.lock);

	spin_lock_bh(&session->reorder_q.lock);

	skb_queue_walk(&session->reorder_q, skbp) {

	skb_queue_walk_safe(&session->reorder_q, skbp, tmp) {

		if (PPPOL2TP_SKB_CB(skbp)->ns > ns) {

		if (PPPOL2TP_SKB_CB(skbp)->ns > ns) {

			__skb_insert(skb, skbp->prev, skbp, &session->reorder_q);

			__skb_insert(skb, skbp->prev, skbp, &session->reorder_q);

			PRINTK(session->debug, PPPOL2TP_MSG_SEQ, KERN_DEBUG,

			PRINTK(session->debug, PPPOL2TP_MSG_SEQ, KERN_DEBUG,
@@ -371,10 +372,9 @@ static void pppol2tp_recv_dequeue_skb(struct pppol2tp_session *session, struct s 
	int length = PPPOL2TP_SKB_CB(skb)->length;

	int length = PPPOL2TP_SKB_CB(skb)->length;

	struct sock *session_sock = NULL;

	struct sock *session_sock = NULL;





	/* We're about to requeue the skb, so unlink it and return resources

	/* We're about to requeue the skb, so return resources

	 * to its current owner (a socket receive buffer).

	 * to its current owner (a socket receive buffer).

	 */

	 */

	skb_unlink(skb, &session->reorder_q);

	skb_orphan(skb);

	skb_orphan(skb);





	tunnel->stats.rx_packets++;

	tunnel->stats.rx_packets++;
@@ -470,6 +470,11 @@ static void pppol2tp_recv_dequeue(struct pppol2tp_session *session) 
				goto out;

				goto out;

			}

			}

		}

		}

		__skb_unlink(skb, &session->reorder_q);



		/* Process the skb. We release the queue lock while we

		 * do so to let other contexts process the queue.

		 */

		spin_unlock_bh(&session->reorder_q.lock);

		spin_unlock_bh(&session->reorder_q.lock);

		pppol2tp_recv_dequeue_skb(session, skb);

		pppol2tp_recv_dequeue_skb(session, skb);

		spin_lock_bh(&session->reorder_q.lock);

		spin_lock_bh(&session->reorder_q.lock);