From patchwork Fri Jan 8 09:45:04 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kiszka X-Patchwork-Id: 43546 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 80C33B7CD8 for ; Sat, 23 Jan 2010 10:29:52 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755432Ab0AVXZ7 (ORCPT ); Fri, 22 Jan 2010 18:25:59 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754248Ab0AVXYM (ORCPT ); Fri, 22 Jan 2010 18:24:12 -0500 Received: from fmmailgate03.web.de ([217.72.192.234]:60631 "EHLO fmmailgate03.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755280Ab0AVXYE (ORCPT ); Fri, 22 Jan 2010 18:24:04 -0500 Received: from smtp08.web.de (fmsmtp08.dlan.cinetic.de [172.20.5.216]) by fmmailgate03.web.de (Postfix) with ESMTP id 8F76113C783E9; Sat, 23 Jan 2010 00:24:03 +0100 (CET) Received: from [92.75.141.69] (helo=[192.168.1.10]) by smtp08.web.de with asmtp (TLSv1:AES256-SHA:256) (WEB.DE 4.110 #314) id 1NYSrD-0006RT-00; Sat, 23 Jan 2010 00:24:03 +0100 Message-Id: In-Reply-To: References: From: Jan Kiszka To: David Miller , Karsten Keil Cc: linux-kernel@vger.kernel.org, i4ldeveloper@listserv.isdn4linux.de, isdn4linux@listserv.isdn4linux.de, netdev@vger.kernel.org Date: Fri, 8 Jan 2010 10:45:04 +0100 Subject: [PATCH 19/31] CAPI: Fix locking around glueing capiminor and capidev X-Sender: jan.kiszka@web.de X-Provags-ID: V01U2FsdGVkX1/UH/mULMbu9ty2sN8T7PCzSEasRRORZwph5kmt 0BGGuIMejFUOwrxZ919OBwdJHpNcDFGvYfV0YPjFpkjrfxCRsr vOGzV/eAg= Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org The CAPI user interface comes with two types of devices: /dev/capi20 is a character device, instances are represented by capidev objects. Each established network control connection triggers the creation of a TTY device (/dev/capi/), open instances are managed via capiminor objects. capincci objects are representing the connections, and they link the corresponding capiminor to its capidev. Unfortunately, the lifetimes of an NCCI is not identical to that of a capiminor instance. So we need proper locking to glue things and also unbind them again. This patch fixes the totally broken attempts to achieve this via a rwlock and some atomic counter. It converts the rwlock into a mutex and applies it to all related critical sections (capinc_tty_open/close as well as capiminor_alloc/release). Signed-off-by: Jan Kiszka --- drivers/isdn/capi/capi.c | 82 +++++++++++++++++++++++---------------------- 1 files changed, 42 insertions(+), 40 deletions(-) diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c index 249ff13..b53cead 100644 --- a/drivers/isdn/capi/capi.c +++ b/drivers/isdn/capi/capi.c @@ -150,7 +150,7 @@ static LIST_HEAD(capidev_list); #ifdef CONFIG_ISDN_CAPI_MIDDLEWARE -static DEFINE_RWLOCK(capiminor_list_lock); +static DEFINE_MUTEX(glue_lock); static LIST_HEAD(capiminor_list); /* -------- datahandles --------------------------------------------- */ @@ -214,7 +214,6 @@ static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci) { struct capiminor *mp, *p; unsigned int minor = 0; - unsigned long flags; mp = kzalloc(sizeof(*mp), GFP_KERNEL); if (!mp) { @@ -234,7 +233,6 @@ static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci) /* Allocate the least unused minor number. */ - write_lock_irqsave(&capiminor_list_lock, flags); if (list_empty(&capiminor_list)) list_add(&mp->list, &capiminor_list); else { @@ -249,7 +247,6 @@ static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci) list_add(&mp->list, p->list.prev); } } - write_unlock_irqrestore(&capiminor_list_lock, flags); if (!(minor < capi_ttyminors)) { printk(KERN_NOTICE "capi: out of minorsn"); @@ -262,36 +259,25 @@ static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci) static void capiminor_free(struct capiminor *mp) { - unsigned long flags; - - write_lock_irqsave(&capiminor_list_lock, flags); list_del(&mp->list); - write_unlock_irqrestore(&capiminor_list_lock, flags); kfree_skb(mp->ttyskb); - mp->ttyskb = NULL; skb_queue_purge(&mp->inqueue); skb_queue_purge(&mp->outqueue); + capiminor_del_all_ack(mp); + kfree(mp); } static struct capiminor *capiminor_find(unsigned int minor) { - struct list_head *l; - struct capiminor *p = NULL; - - read_lock(&capiminor_list_lock); - list_for_each(l, &capiminor_list) { - p = list_entry(l, struct capiminor, list); - if (p->minor == minor) - break; - } - read_unlock(&capiminor_list_lock); - if (l == &capiminor_list) - return NULL; + struct capiminor *mp; - return p; + list_for_each_entry(mp, &capiminor_list, list) + if (mp->minor == minor) + return mp; + return NULL; } /* -------- struct capincci ----------------------------------------- */ @@ -303,6 +289,8 @@ static void capincci_alloc_minor(struct capidev *cdev, struct capincci *np) if (!(cdev->userflags & CAPIFLAG_HIGHJACKING)) return; + mutex_lock(&glue_lock); + mp = np->minorp = capiminor_alloc(&cdev->ap, np->ncci); if (mp) { mp->nccip = np; @@ -313,12 +301,17 @@ static void capincci_alloc_minor(struct capidev *cdev, struct capincci *np) capifs_new_ncci(mp->minor, MKDEV(capi_ttymajor, mp->minor)); } + + mutex_unlock(&glue_lock); } static void capincci_free_minor(struct capincci *np) { - struct capiminor *mp = np->minorp; + struct capiminor *mp; + mutex_lock(&glue_lock); + + mp = np->minorp; if (mp) { capifs_free_ncci(mp->capifs_dentry); if (mp->tty) { @@ -331,6 +324,8 @@ static void capincci_free_minor(struct capincci *np) capiminor_free(mp); } } + + mutex_unlock(&glue_lock); } static inline unsigned int capincci_minor_opencount(struct capincci *np) @@ -992,13 +987,17 @@ static int capinc_tty_open(struct tty_struct * tty, struct file * file) { struct capiminor *mp; unsigned long flags; + int err = 0; - if ((mp = capiminor_find(iminor(file->f_path.dentry->d_inode))) == NULL) - return -ENXIO; - if (mp->nccip == NULL) - return -ENXIO; + mutex_lock(&glue_lock); - tty->driver_data = (void *)mp; + mp = capiminor_find(iminor(file->f_path.dentry->d_inode)); + if (!mp || !mp->nccip) { + err = -ENXIO; + goto unlock_out; + } + + tty->driver_data = mp; spin_lock_irqsave(&workaround_lock, flags); if (atomic_read(&mp->ttyopencount) == 0) @@ -1009,28 +1008,31 @@ static int capinc_tty_open(struct tty_struct * tty, struct file * file) #endif handle_minor_recv(mp); spin_unlock_irqrestore(&workaround_lock, flags); - return 0; + +unlock_out: + mutex_unlock(&glue_lock); + return err; } static void capinc_tty_close(struct tty_struct * tty, struct file * file) { - struct capiminor *mp; + struct capiminor *mp = tty->driver_data; - mp = (struct capiminor *)tty->driver_data; - if (mp) { - if (atomic_dec_and_test(&mp->ttyopencount)) { + mutex_lock(&glue_lock); + + if (atomic_dec_and_test(&mp->ttyopencount)) { #ifdef _DEBUG_REFCOUNT - printk(KERN_DEBUG "capinc_tty_close lastclosen"); + printk(KERN_DEBUG "capinc_tty_close lastclosen"); #endif - tty->driver_data = NULL; - mp->tty = NULL; - } + mp->tty = NULL; + } #ifdef _DEBUG_REFCOUNT printk(KERN_DEBUG "capinc_tty_close ocount=%dn", atomic_read(&mp->ttyopencount)); #endif - if (mp->nccip == NULL) - capiminor_free(mp); - } + if (!mp->nccip) + capiminor_free(mp); + + mutex_unlock(&glue_lock); #ifdef _DEBUG_REFCOUNT printk(KERN_DEBUG "capinc_tty_closen");