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

Commit 021cc4ea authored by Lena Salman's avatar Lena Salman
Browse files

USB: u_bam_data: Optimize usage of spin_lock around skb_pool



Spin lock is dropped before allocating skbs from free pool
and acquired again by the skb pool alloc/free APIs. Same is
the case when skbs are added back to the pool. Around most
of these calls USB state is not checked when spin lock is
re-acquired. This opens up a race condition where USB
might be disconnected (resulting in freeing skb pool) just
before re-acquiring spin_lock resulting in use after free
or list corruption. Fix this by moving spin_lock handling
to the caller of skb pool alloc/free APIs.

Change-Id: I28bdfb46cce93b489b669dd3fddcc74488c2e130
Signed-off-by: default avatarLena Salman <esalman@codeaurora.org>
parent 13c60e82
Loading
Loading
Loading
Loading
+6 −7
Original line number Diff line number Diff line
@@ -232,7 +232,6 @@ static void bam_data_free_skb_to_pool(
	struct sk_buff *skb)
{
	struct bam_data_ch_info *d;
	unsigned long flags;

	if (!port) {
		dev_kfree_skb_any(skb);
@@ -244,11 +243,9 @@ static void bam_data_free_skb_to_pool(
		return;
	}

	spin_lock_irqsave(&port->port_lock_ul, flags);
	skb->len = 0;
	skb_reset_tail_pointer(skb);
	__skb_queue_tail(&d->rx_skb_idle, skb);
	spin_unlock_irqrestore(&port->port_lock_ul, flags);
}

static void bam_data_write_done(void *p, struct sk_buff *skb)
@@ -260,9 +257,9 @@ static void bam_data_write_done(void *p, struct sk_buff *skb)
	if (!skb)
		return;

	spin_lock_irqsave(&port->port_lock_ul, flags);
	bam_data_free_skb_to_pool(port, skb);

	spin_lock_irqsave(&port->port_lock_ul, flags);
	d->pending_with_bam--;

	pr_debug("%s: port:%p d:%p pbam:%u, pno:%d\n", __func__,
@@ -345,10 +342,7 @@ static void bam_data_start_rx(struct bam_data_port *port)
		ret = usb_ep_queue(ep, req, GFP_ATOMIC);
		spin_lock_irqsave(&port->port_lock_ul, flags);
		if (ret) {
			spin_unlock_irqrestore(&port->port_lock_ul, flags);
			bam_data_free_skb_to_pool(port, skb);
			spin_lock_irqsave(&port->port_lock_ul, flags);


			pr_err("%s: rx queue failed %d\n", __func__, ret);

@@ -370,6 +364,7 @@ static void bam_data_epout_complete(struct usb_ep *ep, struct usb_request *req)
	struct sk_buff		*skb = req->context;
	int			status = req->status;
	int			queue = 0;
	unsigned long		flags;

	switch (status) {
	case 0:
@@ -379,14 +374,18 @@ static void bam_data_epout_complete(struct usb_ep *ep, struct usb_request *req)
	case -ECONNRESET:
	case -ESHUTDOWN:
		/* cable disconnection */
		spin_lock_irqsave(&port->port_lock_ul, flags);
		bam_data_free_skb_to_pool(port, skb);
		spin_unlock_irqrestore(&port->port_lock_ul, flags);
		req->buf = 0;
		usb_ep_free_request(ep, req);
		return;
	default:
		pr_err("%s: %s response error %d, %d/%d\n", __func__,
			ep->name, status, req->actual, req->length);
		spin_lock_irqsave(&port->port_lock_ul, flags);
		bam_data_free_skb_to_pool(port, skb);
		spin_unlock_irqrestore(&port->port_lock_ul, flags);
		break;
	}