Commit dfbb84ff authored by Jan Kiszka's avatar Jan Kiszka Committed by David S. Miller
Browse files

CAPI: Fix locking around capiminor's output queue and drop workaround_lock 



Introduce outlock as a spin lock that protects capiminor's outqueue,
outbytes and outskb (formerly known as ttyskb). outlock can be acquired
from soft-IRQ context via capinc_write, so make it bh-safe.

This finally removes the last reason for keeping the workaround lock
around (which was incomplete and partly broken anyway). And as we no
longer call handle_recv_skb in atomic context, gen_data_b3_resp_for can
use non-atomic allocation now.

Signed-off-by: default avatarJan Kiszka <jan.kiszka@web.de>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 68d7347b

drivers/isdn/capi/capi.c

+67 −59
Original line number Diff line number Diff line
@@ -95,11 +95,13 @@ struct capiminor { 
	struct tty_port port;

	int                ttyinstop;

	int                ttyoutstop;

	struct sk_buff    *ttyskb;



	struct sk_buff_head	inqueue;



	struct sk_buff_head	outqueue;

	int			outbytes;

	struct sk_buff		*outskb;

	spinlock_t		outlock;



	/* transmit path */

	struct list_head ackqueue;
@@ -107,15 +109,6 @@ struct capiminor { 
	spinlock_t ackqlock;

};



/* FIXME: The following lock is a sledgehammer-workaround to a

 * locking issue with the capiminor (and maybe other) data structure(s).

 * Access to this data is done in a racy way and crashes the machine with

 * a FritzCard DSL driver; sooner or later. This is a workaround

 * which trades scalability vs stability, so it doesn't crash the kernel anymore.

 * The correct (and scalable) fix for the issue seems to require

 * an API change to the drivers... . */

static DEFINE_SPINLOCK(workaround_lock);



struct capincci {

	struct list_head list;

	u32		 ncci;
@@ -231,6 +224,7 @@ static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci) 


	skb_queue_head_init(&mp->inqueue);

	skb_queue_head_init(&mp->outqueue);

	spin_lock_init(&mp->outlock);



	tty_port_init(&mp->port);

	mp->port.ops = &capiminor_port_ops;
@@ -271,7 +265,7 @@ static void capiminor_destroy(struct kref *kref) 
{

	struct capiminor *mp = container_of(kref, struct capiminor, kref);



	kfree_skb(mp->ttyskb);

	kfree_skb(mp->outskb);

	skb_queue_purge(&mp->inqueue);

	skb_queue_purge(&mp->outqueue);

	capiminor_del_all_ack(mp);
@@ -417,7 +411,7 @@ static struct sk_buff * 
gen_data_b3_resp_for(struct capiminor *mp, struct sk_buff *skb)

{

	struct sk_buff *nskb;

	nskb = alloc_skb(CAPI_DATA_B3_RESP_LEN, GFP_ATOMIC);

	nskb = alloc_skb(CAPI_DATA_B3_RESP_LEN, GFP_KERNEL);

	if (nskb) {

		u16 datahandle = CAPIMSG_U16(skb->data,CAPIMSG_BASELEN+4+4+2);

		unsigned char *s = skb_put(nskb, CAPI_DATA_B3_RESP_LEN);
@@ -548,9 +542,18 @@ static int handle_minor_send(struct capiminor *mp) 
		return 0;

	}



	while ((skb = skb_dequeue(&mp->outqueue)) != NULL) {

		datahandle = atomic_inc_return(&mp->datahandle);

	while (1) {

		spin_lock_bh(&mp->outlock);

		skb = __skb_dequeue(&mp->outqueue);

		if (!skb) {

			spin_unlock_bh(&mp->outlock);

			break;

		}

		len = (u16)skb->len;

		mp->outbytes -= len;

		spin_unlock_bh(&mp->outlock);



		datahandle = atomic_inc_return(&mp->datahandle);

		skb_push(skb, CAPI_DATA_B3_REQ_LEN);

		memset(skb->data, 0, CAPI_DATA_B3_REQ_LEN);

		capimsg_setu16(skb->data, 0, CAPI_DATA_B3_REQ_LEN);
@@ -566,14 +569,18 @@ static int handle_minor_send(struct capiminor *mp) 


		if (capiminor_add_ack(mp, datahandle) < 0) {

			skb_pull(skb, CAPI_DATA_B3_REQ_LEN);

			skb_queue_head(&mp->outqueue, skb);



			spin_lock_bh(&mp->outlock);

			__skb_queue_head(&mp->outqueue, skb);

			mp->outbytes += len;

			spin_unlock_bh(&mp->outlock);



			tty_kref_put(tty);

			return count;

		}

		errcode = capi20_put_message(mp->ap, skb);

		if (errcode == CAPI_NOERROR) {

			count++;

			mp->outbytes -= len;

#ifdef _DEBUG_DATAFLOW

			printk(KERN_DEBUG "capi: DATA_B3_REQ %u len=%u\n",

							datahandle, len);
@@ -584,13 +591,17 @@ static int handle_minor_send(struct capiminor *mp) 


		if (errcode == CAPI_SENDQUEUEFULL) {

			skb_pull(skb, CAPI_DATA_B3_REQ_LEN);

			skb_queue_head(&mp->outqueue, skb);



			spin_lock_bh(&mp->outlock);

			__skb_queue_head(&mp->outqueue, skb);

			mp->outbytes += len;

			spin_unlock_bh(&mp->outlock);



			break;

		}



		/* ups, drop packet */

		printk(KERN_ERR "capi: put_message = %x\n", errcode);

		mp->outbytes -= len;

		kfree_skb(skb);

	}

	tty_kref_put(tty);
@@ -609,7 +620,6 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb) 
	u16 datahandle;

#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */

	struct capincci *np;

	unsigned long flags;



	mutex_lock(&cdev->lock);
@@ -621,7 +631,6 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb) 
	if (CAPIMSG_CMD(skb->data) == CAPI_CONNECT_B3_IND)

		capincci_alloc(cdev, CAPIMSG_NCCI(skb->data));



	spin_lock_irqsave(&workaround_lock, flags);

	if (CAPIMSG_COMMAND(skb->data) != CAPI_DATA_B3) {

		skb_queue_tail(&cdev->recvqueue, skb);

		wake_up_interruptible(&cdev->recvwait);
@@ -683,7 +692,6 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb) 
#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */



unlock_out:

	spin_unlock_irqrestore(&workaround_lock, flags);

	mutex_unlock(&cdev->lock);

}
@@ -1062,16 +1070,13 @@ static void capinc_tty_cleanup(struct tty_struct *tty) 
static int capinc_tty_open(struct tty_struct *tty, struct file *filp)

{

	struct capiminor *mp = tty->driver_data;

	unsigned long flags;

	int err;



	err = tty_port_open(&mp->port, tty, filp);

	if (err)

		return err;



	spin_lock_irqsave(&workaround_lock, flags);

	handle_minor_recv(mp);

	spin_unlock_irqrestore(&workaround_lock, flags);

	return 0;

}
@@ -1087,71 +1092,78 @@ static int capinc_tty_write(struct tty_struct *tty, 
{

	struct capiminor *mp = tty->driver_data;

	struct sk_buff *skb;

	unsigned long flags;



#ifdef _DEBUG_TTYFUNCS

	printk(KERN_DEBUG "capinc_tty_write(count=%d)\n", count);

#endif



	spin_lock_irqsave(&workaround_lock, flags);

	skb = mp->ttyskb;

	spin_lock_bh(&mp->outlock);

	skb = mp->outskb;

	if (skb) {

		mp->ttyskb = NULL;

		skb_queue_tail(&mp->outqueue, skb);

		mp->outskb = NULL;

		__skb_queue_tail(&mp->outqueue, skb);

		mp->outbytes += skb->len;

	}



	skb = alloc_skb(CAPI_DATA_B3_REQ_LEN+count, GFP_ATOMIC);

	if (!skb) {

		printk(KERN_ERR "capinc_tty_write: alloc_skb failed\n");

		spin_unlock_irqrestore(&workaround_lock, flags);

		spin_unlock_bh(&mp->outlock);

		return -ENOMEM;

	}



	skb_reserve(skb, CAPI_DATA_B3_REQ_LEN);

	memcpy(skb_put(skb, count), buf, count);



	skb_queue_tail(&mp->outqueue, skb);

	__skb_queue_tail(&mp->outqueue, skb);

	mp->outbytes += skb->len;

	spin_unlock_bh(&mp->outlock);



	(void)handle_minor_send(mp);

	spin_unlock_irqrestore(&workaround_lock, flags);



	return count;

}



static int capinc_tty_put_char(struct tty_struct *tty, unsigned char ch)

{

	struct capiminor *mp = tty->driver_data;

	bool invoke_send = false;

	struct sk_buff *skb;

	unsigned long flags;

	int ret = 1;



#ifdef _DEBUG_TTYFUNCS

	printk(KERN_DEBUG "capinc_put_char(%u)\n", ch);

#endif



	spin_lock_irqsave(&workaround_lock, flags);

	skb = mp->ttyskb;

	spin_lock_bh(&mp->outlock);

	skb = mp->outskb;

	if (skb) {

		if (skb_tailroom(skb) > 0) {

			*(skb_put(skb, 1)) = ch;

			spin_unlock_irqrestore(&workaround_lock, flags);

			return 1;

			goto unlock_out;

		}

		mp->ttyskb = NULL;

		skb_queue_tail(&mp->outqueue, skb);

		mp->outskb = NULL;

		__skb_queue_tail(&mp->outqueue, skb);

		mp->outbytes += skb->len;

		(void)handle_minor_send(mp);

		invoke_send = true;

	}



	skb = alloc_skb(CAPI_DATA_B3_REQ_LEN+CAPI_MAX_BLKSIZE, GFP_ATOMIC);

	if (skb) {

		skb_reserve(skb, CAPI_DATA_B3_REQ_LEN);

		*(skb_put(skb, 1)) = ch;

		mp->ttyskb = skb;

		mp->outskb = skb;

	} else {

		printk(KERN_ERR "capinc_put_char: char %u lost\n", ch);

		ret = 0;

	}

	spin_unlock_irqrestore(&workaround_lock, flags);



unlock_out:

	spin_unlock_bh(&mp->outlock);



	if (invoke_send)

		(void)handle_minor_send(mp);



	return ret;

}
@@ -1159,22 +1171,24 @@ static void capinc_tty_flush_chars(struct tty_struct *tty) 
{

	struct capiminor *mp = tty->driver_data;

	struct sk_buff *skb;

	unsigned long flags;



#ifdef _DEBUG_TTYFUNCS

	printk(KERN_DEBUG "capinc_tty_flush_chars\n");

#endif



	spin_lock_irqsave(&workaround_lock, flags);

	skb = mp->ttyskb;

	spin_lock_bh(&mp->outlock);

	skb = mp->outskb;

	if (skb) {

		mp->ttyskb = NULL;

		skb_queue_tail(&mp->outqueue, skb);

		mp->outskb = NULL;

		__skb_queue_tail(&mp->outqueue, skb);

		mp->outbytes += skb->len;

		spin_unlock_bh(&mp->outlock);



		(void)handle_minor_send(mp);

	}

	(void)handle_minor_recv(mp);

	spin_unlock_irqrestore(&workaround_lock, flags);

	} else

		spin_unlock_bh(&mp->outlock);



	handle_minor_recv(mp);

}



static int capinc_tty_write_room(struct tty_struct *tty)
@@ -1234,15 +1248,12 @@ static void capinc_tty_throttle(struct tty_struct *tty) 
static void capinc_tty_unthrottle(struct tty_struct *tty)

{

	struct capiminor *mp = tty->driver_data;

	unsigned long flags;



#ifdef _DEBUG_TTYFUNCS

	printk(KERN_DEBUG "capinc_tty_unthrottle\n");

#endif

	spin_lock_irqsave(&workaround_lock, flags);

	mp->ttyinstop = 0;

	handle_minor_recv(mp);

	spin_unlock_irqrestore(&workaround_lock, flags);

}



static void capinc_tty_stop(struct tty_struct *tty)
@@ -1258,15 +1269,12 @@ static void capinc_tty_stop(struct tty_struct *tty) 
static void capinc_tty_start(struct tty_struct *tty)

{

	struct capiminor *mp = tty->driver_data;

	unsigned long flags;



#ifdef _DEBUG_TTYFUNCS

	printk(KERN_DEBUG "capinc_tty_start\n");

#endif

	spin_lock_irqsave(&workaround_lock, flags);

	mp->ttyoutstop = 0;

	(void)handle_minor_send(mp);

	spin_unlock_irqrestore(&workaround_lock, flags);

}



static void capinc_tty_hangup(struct tty_struct *tty)

CRA Git | Maintained and supported by SUSTech CRA and CCSE