diff -u -p linux/include/net/irda/irmod.d2.h linux/include/net/irda/irmod.h --- linux/include/net/irda/irmod.d2.h Wed Nov 7 13:21:31 2001 +++ linux/include/net/irda/irmod.h Wed Nov 7 13:57:41 2001 @@ -10,6 +10,7 @@ * Modified by: Dag Brattli * * Copyright (c) 1998-2000 Dag Brattli, All Rights Reserved. + * Copyright (c) 2000-2001 Jean Tourrilhes * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as @@ -25,108 +26,19 @@ #ifndef IRMOD_H #define IRMOD_H -#include -#include +#include /* Notify stuff */ -#include +/* Nothing much here anymore - Maybe this header should be merged in + * another header like net/irda/irda.h... - Jean II */ -#define IRMGR_IOC_MAGIC 'm' -#define IRMGR_IOCTNPC _IO(IRMGR_IOC_MAGIC, 1) -#define IRMGR_IOC_MAXNR 1 - -/* - * Events that we pass to the user space manager - */ -typedef enum { - EVENT_DEVICE_DISCOVERED = 0, - EVENT_REQUEST_MODULE, - EVENT_IRLAN_START, - EVENT_IRLAN_STOP, - EVENT_IRLPT_START, /* Obsolete */ - EVENT_IRLPT_STOP, /* Obsolete */ - EVENT_IROBEX_START, /* Obsolete */ - EVENT_IROBEX_STOP, /* Obsolete */ - EVENT_IRDA_STOP, - EVENT_NEED_PROCESS_CONTEXT, -} IRMGR_EVENT; - -/* - * Event information passed to the IrManager daemon process - */ -struct irmanager_event { - IRMGR_EVENT event; - char devname[10]; - char info[32]; - int service; - __u32 saddr; - __u32 daddr; -}; - -typedef void (*TODO_CALLBACK)( void *self, __u32 param); - -/* - * Same as irmanager_event but this one can be queued and inclueds some - * addtional information - */ -struct irda_event { - irda_queue_t q; /* Must be first */ - - struct irmanager_event event; -}; - -/* - * Funtions with needs to be called with a process context - */ -struct irda_todo { - irda_queue_t q; /* Must be first */ - - void *self; - TODO_CALLBACK callback; - __u32 param; -}; - -/* - * Main structure for the IrDA device (not much here :-) - */ -struct irda_cb { - struct miscdevice dev; - wait_queue_head_t wait_queue; - - int in_use; - - irda_queue_t *event_queue; /* Events queued for the irmanager */ - irda_queue_t *todo_queue; /* Todo list */ -}; - -int irmod_init_module(void); -void irmod_cleanup_module(void); - -/* - * Function irda_lock (lock) - * - * Lock variable. Returns false if the lock is already set. - * - */ -static inline int irda_lock(int *lock) -{ - if (test_and_set_bit( 0, (void *) lock)) { - IRDA_DEBUG(3, __FUNCTION__ - "(), Trying to lock, already locked variable!\n"); - return FALSE; - } - return TRUE; -} - -inline int irda_unlock(int *lock); +/* Locking wrapper - Note the inverted logic on irda_lock(). + * Those function basically return false if the lock is already in the + * position you want to set it. - Jean II */ +#define irda_lock(lock) (! test_and_set_bit(0, (void *) (lock))) +#define irda_unlock(lock) (test_and_clear_bit(0, (void *) (lock))) +/* Zero the notify structure */ void irda_notify_init(notify_t *notify); - -void irda_execute_as_process(void *self, TODO_CALLBACK callback, __u32 param); -void irmanager_notify(struct irmanager_event *event); - -extern void irda_proc_modcount(struct inode *, int); -void irda_mod_inc_use_count(void); -void irda_mod_dec_use_count(void); #endif /* IRMOD_H */ diff -u -p linux/net/irda/irsyms.d2.c linux/net/irda/irsyms.c --- linux/net/irda/irsyms.d2.c Wed Nov 7 13:16:00 2001 +++ linux/net/irda/irsyms.c Wed Nov 7 13:42:19 2001 @@ -10,6 +10,7 @@ * Modified by: Dag Brattli * * Copyright (c) 1997, 1999-2000 Dag Brattli, All Rights Reserved. + * Copyright (c) 2000-2001 Jean Tourrilhes * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as @@ -79,7 +80,6 @@ EXPORT_SYMBOL(irttp_dup); EXPORT_SYMBOL(irda_debug); #endif EXPORT_SYMBOL(irda_notify_init); -EXPORT_SYMBOL(irda_lock); #ifdef CONFIG_PROC_FS EXPORT_SYMBOL(proc_irda); #endif @@ -217,21 +217,6 @@ static void __exit irda_cleanup(void) /* Remove middle layer */ irlmp_cleanup(); -} - -/* - * Function irda_unlock (lock) - * - * Unlock variable. Returns false if lock is already unlocked - * - */ -inline int irda_unlock(int *lock) -{ - if (!test_and_clear_bit(0, (void *) lock)) { - printk("Trying to unlock already unlocked variable!\n"); - return FALSE; - } - return TRUE; } /* diff -u -p linux/net/irda/irttp.d2.c linux/net/irda/irttp.c --- linux/net/irda/irttp.d2.c Tue Oct 30 18:14:58 2001 +++ linux/net/irda/irttp.c Wed Nov 7 13:33:09 2001 @@ -11,6 +11,7 @@ * * Copyright (c) 1998-2000 Dag Brattli , * All Rights Reserved. + * Copyright (c) 2000-2001 Jean Tourrilhes * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as @@ -223,6 +224,11 @@ static void __irttp_close_tsap(struct ts del_timer(&self->todo_timer); + /* This one won't be cleaned up if we are diconnect_pend + close_pend + * and we receive a disconnect_indication */ + if (self->disconnect_skb) + dev_kfree_skb(self->disconnect_skb); + self->connected = FALSE; self->magic = ~TTP_TSAP_MAGIC; @@ -235,6 +241,9 @@ static void __irttp_close_tsap(struct ts * Remove TSAP from list of all TSAPs and then deallocate all resources * associated with this TSAP * + * Note : because we *free* the tsap structure, it is the responsability + * of the caller to make sure we are called only once and to deal with + * possible race conditions. - Jean II */ int irttp_close_tsap(struct tsap_cb *self) { @@ -248,8 +257,8 @@ int irttp_close_tsap(struct tsap_cb *sel /* Make sure tsap has been disconnected */ if (self->connected) { /* Check if disconnect is not pending */ - if (!self->disconnect_pend) { - IRDA_DEBUG(0, __FUNCTION__ "(), TSAP still connected!\n"); + if (!test_bit(0, &self->disconnect_pend)) { + WARNING(__FUNCTION__ "(), TSAP still connected!\n"); irttp_disconnect_request(self, NULL, P_NORMAL); } self->close_pend = TRUE; @@ -407,6 +416,7 @@ static void irttp_run_tx_queue(struct ts unsigned long flags; int n; + /* Get exclusive access to the tx queue, otherwise don't touch it */ if (irda_lock(&self->tx_queue_lock) == FALSE) return; @@ -473,27 +483,17 @@ static void irttp_run_tx_queue(struct ts * close the socket, we are dead ! * Jean II */ if (skb->sk != NULL) { - struct sk_buff *tx_skb; - /* IrSOCK application, IrOBEX, ... */ IRDA_DEBUG(4, __FUNCTION__ "() : Detaching SKB from socket.\n"); - /* Note : still looking for a more efficient way - * to do that - Jean II */ - /* Get another skb on the same buffer, but without - * a reference to the socket (skb->sk = NULL) */ - tx_skb = skb_clone(skb, GFP_ATOMIC); - if (tx_skb != NULL) { - /* Release the skb associated with the - * socket, and use the new skb insted */ - kfree_skb(skb); - skb = tx_skb; - } + /* That's the right way to do it - Jean II */ + skb_orphan(skb); } else { /* IrCOMM over IrTTP, IrLAN, ... */ IRDA_DEBUG(4, __FUNCTION__ "() : Got SKB not attached to a socket.\n"); } + /* Pass the skb to IrLMP - done */ irlmp_data_request(self->lsap, skb); self->stats.tx_packets++; @@ -1105,18 +1105,23 @@ int irttp_disconnect_request(struct tsap /* Already disconnected? */ if (!self->connected) { IRDA_DEBUG(4, __FUNCTION__ "(), already disconnected!\n"); + if (userdata) + dev_kfree_skb(userdata); return -1; } - /* Disconnect already pending? */ - if (self->disconnect_pend) { - IRDA_DEBUG(1, __FUNCTION__ "(), disconnect already pending\n"); - if (userdata) { + /* Disconnect already pending ? + * We need to use an atomic operation to prevent reentry. This + * function may be called from various context, like user, timer + * for following a disconnect_indication() (i.e. net_bh). + * Jean II */ + if(test_and_set_bit(0, &self->disconnect_pend)) { + IRDA_DEBUG(0, __FUNCTION__ "(), disconnect already pending\n"); + if (userdata) dev_kfree_skb(userdata); - } /* Try to make some progress */ - irttp_run_rx_queue(self); + irttp_run_tx_queue(self); return -1; } @@ -1125,25 +1130,20 @@ int irttp_disconnect_request(struct tsap */ if (skb_queue_len(&self->tx_queue) > 0) { if (priority == P_HIGH) { - IRDA_DEBUG(1, __FUNCTION__ "High priority!!()\n" ); - /* * No need to send the queued data, if we are * disconnecting right now since the data will * not have any usable connection to be sent on */ + IRDA_DEBUG(1, __FUNCTION__ "High priority!!()\n" ); irttp_flush_queues(self); } else if (priority == P_NORMAL) { /* - * Must delay disconnect til after all data segments - * have been sent an the tx_queue is empty + * Must delay disconnect until after all data segments + * have been sent and the tx_queue is empty */ - if (userdata) - self->disconnect_skb = userdata; - else - self->disconnect_skb = NULL; - - self->disconnect_pend = TRUE; + /* We'll reuse this one later for the disconnect */ + self->disconnect_skb = userdata; /* May be NULL */ irttp_run_tx_queue(self); @@ -1152,9 +1152,8 @@ int irttp_disconnect_request(struct tsap } } IRDA_DEBUG(1, __FUNCTION__ "(), Disconnecting ...\n"); - self->connected = FALSE; - + if (!userdata) { skb = dev_alloc_skb(64); if (!skb) @@ -1169,6 +1168,9 @@ int irttp_disconnect_request(struct tsap } ret = irlmp_disconnect_request(self->lsap, userdata); + /* The disconnect is no longer pending */ + clear_bit(0, &self->disconnect_pend); /* FALSE */ + return ret; } @@ -1190,19 +1192,27 @@ void irttp_disconnect_indication(void *i ASSERT(self != NULL, return;); ASSERT(self->magic == TTP_TSAP_MAGIC, return;); + /* Prevent higher layer to send more data */ self->connected = FALSE; /* Check if client has already tried to close the TSAP */ if (self->close_pend) { + /* In this case, the higher layer is probably gone. Don't + * bother it and clean up the remains - Jean II */ + if (skb) + dev_kfree_skb(skb); irttp_close_tsap(self); return; } + /* If we are here, we assume that is the higher layer is still + * waiting for the disconnect notification and able to process it, + * even if he tried to disconnect. Otherwise, it would have already + * attempted to close the tsap and self->close_pend would be TRUE. + * Jean II */ + /* No need to notify the client if has already tried to disconnect */ - if (self->disconnect_pend) - return; - - if (self->notify.disconnect_indication) + if(self->notify.disconnect_indication) self->notify.disconnect_indication(self->notify.instance, self, reason, skb); else @@ -1222,7 +1232,7 @@ void irttp_do_data_indication(struct tsa int err; /* Check if client has already tried to close the TSAP */ - if (self->close_pend || self->disconnect_pend) { + if (self->close_pend) { dev_kfree_skb(skb); return; } @@ -1263,6 +1273,7 @@ void irttp_run_rx_queue(struct tsap_cb * IRDA_DEBUG(2, __FUNCTION__ "() send=%d,avail=%d,remote=%d\n", self->send_credit, self->avail_credit, self->remote_credit); + /* Get exclusive access to the rx queue, otherwise don't touch it */ if (irda_lock(&self->rx_queue_lock) == FALSE) return; @@ -1500,7 +1511,7 @@ static int irttp_param_max_sdu_size(void else self->tx_max_sdu_size = param->pv.i; - IRDA_DEBUG(0, __FUNCTION__ "(), MaxSduSize=%d\n", param->pv.i); + IRDA_DEBUG(1, __FUNCTION__ "(), MaxSduSize=%d\n", param->pv.i); return 0; } @@ -1530,18 +1541,16 @@ static void irttp_todo_expired(unsigned } /* Check if time for disconnect */ - if (self->disconnect_pend) { + if (test_bit(0, &self->disconnect_pend)) { /* Check if it's possible to disconnect yet */ if (skb_queue_empty(&self->tx_queue)) { - /* Make sure disconnect is not pending anymore */ - self->disconnect_pend = FALSE; - if (self->disconnect_skb) { - irttp_disconnect_request( - self, self->disconnect_skb, P_NORMAL); - self->disconnect_skb = NULL; - } else - irttp_disconnect_request(self, NULL, P_NORMAL); + clear_bit(0, &self->disconnect_pend); /* FALSE */ + + /* Note : self->disconnect_skb may be NULL */ + irttp_disconnect_request(self, self->disconnect_skb, + P_NORMAL); + self->disconnect_skb = NULL; } else { /* Try again later */ irttp_start_todo_timer(self, 1*HZ); diff -u -p linux/net/irda/af_irda.d2.c linux/net/irda/af_irda.c --- linux/net/irda/af_irda.d2.c Fri Nov 2 11:09:12 2001 +++ linux/net/irda/af_irda.c Mon Nov 5 18:40:11 2001 @@ -11,7 +11,7 @@ * Sources: af_netroom.c, af_ax25.c, af_rose.c, af_x25.c etc. * * Copyright (c) 1999 Dag Brattli - * Copyright (c) 1999 Jean Tourrilhes + * Copyright (c) 1999-2001 Jean Tourrilhes * All Rights Reserved. * * This program is free software; you can redistribute it and/or @@ -134,33 +134,41 @@ static void irda_disconnect_indication(v IRDA_DEBUG(2, __FUNCTION__ "(%p)\n", self); + /* Don't care about it, but let's not leak it */ + if(skb) + dev_kfree_skb(skb); + sk = self->sk; if (sk == NULL) return; - sk->state = TCP_CLOSE; - sk->err = ECONNRESET; - sk->shutdown |= SEND_SHUTDOWN; - if (!sk->dead) { + /* Prevent race conditions with irda_release() and irda_shutdown() */ + if ((!sk->dead) && (sk->state != TCP_CLOSE)) { + sk->state = TCP_CLOSE; + sk->err = ECONNRESET; + sk->shutdown |= SEND_SHUTDOWN; + sk->state_change(sk); - sk->dead = 1; - } + sk->dead = 1; /* Uh-oh... Should use sock_orphan ? */ - /* Close our TSAP. - * If we leave it open, IrLMP put it back into the list of - * unconnected LSAPs. The problem is that any incoming request - * can then be matched to this socket (and it will be, because - * it is at the head of the list). This would prevent any - * listening socket waiting on the same TSAP to get those requests. - * Some apps forget to close sockets, or hang to it a bit too long, - * so we may stay in this dead state long enough to be noticed... - * Note : all socket function do check sk->state, so we are safe... - * Jean II - */ - if (self->tsap) { - irttp_close_tsap(self->tsap); - self->tsap = NULL; - } + /* Close our TSAP. + * If we leave it open, IrLMP put it back into the list of + * unconnected LSAPs. The problem is that any incoming request + * can then be matched to this socket (and it will be, because + * it is at the head of the list). This would prevent any + * listening socket waiting on the same TSAP to get those + * requests. Some apps forget to close sockets, or hang to it + * a bit too long, so we may stay in this dead state long + * enough to be noticed... + * Note : all socket function do check sk->state, so we are + * safe... + * Jean II + */ + if (self->tsap) { + irttp_close_tsap(self->tsap); + self->tsap = NULL; + } + } /* Note : once we are there, there is not much you want to do * with the socket anymore, apart from closing it. @@ -222,7 +230,8 @@ static void irda_connect_confirm(void *i self->max_data_size); memcpy(&self->qos_tx, qos, sizeof(struct qos_info)); - kfree_skb(skb); + dev_kfree_skb(skb); + // Should be ??? skb_queue_tail(&sk->receive_queue, skb); /* We are now connected! */ sk->state = TCP_ESTABLISHED; @@ -1205,7 +1214,7 @@ static int irda_release(struct socket *s sk->protinfo.irda = NULL; sock_orphan(sk); - sock->sk = NULL; + sock->sk = NULL; /* Purge queues (see sock_init_data()) */ skb_queue_purge(&sk->receive_queue); diff -u -p linux/net/irda/iriap.d2.c linux/net/irda/iriap.c --- linux/net/irda/iriap.d2.c Fri Nov 2 11:14:13 2001 +++ linux/net/irda/iriap.c Mon Nov 5 18:41:12 2001 @@ -11,6 +11,7 @@ * * Copyright (c) 1998-1999 Dag Brattli , * All Rights Reserved. + * Copyright (c) 2000-2001 Jean Tourrilhes * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as @@ -773,7 +774,7 @@ static void iriap_connect_indication(voi { struct iriap_cb *self, *new; - IRDA_DEBUG(0, __FUNCTION__ "()\n"); + IRDA_DEBUG(1, __FUNCTION__ "()\n"); self = (struct iriap_cb *) instance; diff -u -p linux/net/irda/irnet/irnet.d2.h linux/net/irda/irnet/irnet.h --- linux/net/irda/irnet/irnet.d2.h Wed Oct 31 16:09:19 2001 +++ linux/net/irda/irnet/irnet.h Wed Nov 7 14:02:34 2001 @@ -126,17 +126,17 @@ * History : * ------- * - * v1 - 15/5/00 - Jean II + * v1 - 15.5.00 - Jean II * o Basic IrNET (hook to ppp_generic & IrTTP - incl. multipoint) * o control channel on /dev/irnet (set name/address) * o event channel on /dev/irnet (for user space daemon) * - * v2 - 5/6/00 - Jean II + * v2 - 5.6.00 - Jean II * o Enable DROP_NOT_READY to avoid PPP timeouts & other weirdness... * o Add DISCONNECT_TO event and rename DISCONNECT_FROM. * o Set official device number alloaction on /dev/irnet * - * v3 - 30/8/00 - Jean II + * v3 - 30.8.00 - Jean II * o Update to latest Linux-IrDA changes : * - queue_t => irda_queue_t * o Update to ppp-2.4.0 : @@ -148,17 +148,17 @@ * another multilink bug (darn !) * o Remove LINKNAME_IOCTL cruft * - * v3b - 31/8/00 - Jean II + * v3b - 31.8.00 - Jean II * o Dump discovery log at event channel startup * - * v4 - 28/9/00 - Jean II + * v4 - 28.9.00 - Jean II * o Fix interaction between poll/select and dump discovery log * o Add IRNET_BLOCKED_LINK event (depend on new IrDA-Linux patch) * o Add IRNET_NOANSWER_FROM event (mostly to help support) * o Release flow control in disconnect_indication * o Block packets while connecting (speed up connections) * - * v5 - 11/01/01 - Jean II + * v5 - 11.01.01 - Jean II * o Init self->max_header_size, just in case... * o Set up ap->chan.hdrlen, to get zero copy on tx side working. * o avoid tx->ttp->flow->ppp->tx->... loop, by checking flow state @@ -169,7 +169,7 @@ * o Declare hashbin HB_NOLOCK instead of HB_LOCAL to avoid * disabling and enabling irq twice * - * v6 - 31/05/01 - Jean II + * v6 - 31.05.01 - Jean II * o Print source address in Found, Discovery, Expiry & Request events * o Print requested source address in /proc/net/irnet * o Change control channel input. Allow multiple commands in one line. @@ -186,12 +186,19 @@ * o Add ttp_connect flag to prevent rentry on the connect procedure * o Test and fixups to eliminate side effects of retries * - * v7 - 22/08/01 - Jean II + * v7 - 22.08.01 - Jean II * o Cleanup : Change "saddr = 0x0" to "saddr = DEV_ADDR_ANY" * o Fix bug in BLOCK_WHEN_CONNECT introduced in v6 : due to the * asynchronous IAS query, self->tsap is NULL when PPP send the * first packet. This was preventing "connect-delay 0" to work. * Change the test in ppp_irnet_send() to self->ttp_connect. + * + * v8 - 1.11.01 - Jean II + * o Tighten the use of self->ttp_connect and self->ttp_open to + * prevent various race conditions. + * o Avoid leaking discovery log and skb + * o Replace "self" with "server" in irnet_connect_indication() to + * better detect cut'n'paste error ;-) */ /***************************** INCLUDES *****************************/ @@ -204,6 +211,7 @@ #include #include #include +#include #include #include #include /* isspace() */ diff -u -p linux/net/irda/irnet/irnet_irda.d2.c linux/net/irda/irnet/irnet_irda.c --- linux/net/irda/irnet/irnet_irda.d2.c Wed Oct 31 16:00:53 2001 +++ linux/net/irda/irnet/irnet_irda.c Wed Nov 7 10:48:58 2001 @@ -272,7 +272,7 @@ irnet_connect_tsap(irnet_socket * self) err = irnet_open_tsap(self); if(err != 0) { - self->ttp_connect = 0; + clear_bit(0, &self->ttp_connect); DERROR(IRDA_SR_ERROR, "connect aborted!\n"); return(err); } @@ -283,7 +283,7 @@ irnet_connect_tsap(irnet_socket * self) self->max_sdu_size_rx, NULL); if(err != 0) { - self->ttp_connect = 0; + clear_bit(0, &self->ttp_connect); DERROR(IRDA_SR_ERROR, "connect aborted!\n"); return(err); } @@ -377,7 +377,7 @@ irnet_discover_daddr_and_lsap_sel(irnet_ if(self->discoveries == NULL) { self->disco_number = -1; - self->ttp_connect = 0; + clear_bit(0, &self->ttp_connect); DRETURN(-ENETUNREACH, IRDA_SR_INFO, "No Cachelog...\n"); } DEBUG(IRDA_SR_INFO, "Got the log (0x%X), size is %d\n", @@ -399,7 +399,7 @@ irnet_discover_daddr_and_lsap_sel(irnet_ kfree(self->discoveries); self->discoveries = NULL; - self->ttp_connect = 0; + clear_bit(0, &self->ttp_connect); DRETURN(-ENETUNREACH, IRDA_SR_INFO, "Cachelog empty...\n"); } @@ -518,12 +518,12 @@ irda_irnet_connect(irnet_socket * self) DENTER(IRDA_SOCK_TRACE, "(self=0x%X)\n", (unsigned int) self); - /* Check if we have opened a local TSAP : - * If we have already opened a TSAP, it means that either we are already - * connected or in the process of doing so... */ - if(self->ttp_connect) + /* Check if we are already trying to connect. + * Because irda_irnet_connect() can be called directly by pppd plus + * packet retries in ppp_generic and connect may take time, plus we may + * race with irnet_connect_indication(), we need to be careful there... */ + if(test_and_set_bit(0, &self->ttp_connect)) DRETURN(-EBUSY, IRDA_SOCK_INFO, "Already connecting...\n"); - self->ttp_connect = 1; if((self->iriap != NULL) || (self->tsap != NULL)) DERROR(IRDA_SOCK_ERROR, "Socket not cleaned up...\n"); @@ -579,6 +579,7 @@ irda_irnet_connect(irnet_socket * self) * * Destroy irnet instance * + * Note : this need to be called from a process context. */ void irda_irnet_destroy(irnet_socket * self) @@ -601,6 +602,23 @@ irda_irnet_destroy(irnet_socket * self) DASSERT(entry == self, , IRDA_SOCK_ERROR, "Can't remove from hash.\n"); } + /* If we were connected, post a message */ + if(test_bit(0, &self->ttp_open)) + { + /* Note : as the disconnect comes from ppp_generic, the unit number + * doesn't exist anymore when we post the event, so we need to pass + * NULL as the first arg... */ + irnet_post_event(NULL, IRNET_DISCONNECT_TO, + self->saddr, self->daddr, self->rname); + } + + /* Prevent various IrDA callbacks from messing up things + * Need to be first */ + clear_bit(0, &self->ttp_connect); + + /* Prevent higher layer from accessing IrTTP */ + clear_bit(0, &self->ttp_open); + /* Unregister with IrLMP */ irlmp_unregister_client(self->ckey); @@ -611,19 +629,14 @@ irda_irnet_destroy(irnet_socket * self) self->iriap = NULL; } - /* If we were connected, post a message */ - if(self->ttp_open) + /* Cleanup eventual discoveries from connection attempt */ + if(self->discoveries != NULL) { - /* Note : as the disconnect comes from ppp_generic, the unit number - * doesn't exist anymore when we post the event, so we need to pass - * NULL as the first arg... */ - irnet_post_event(NULL, IRNET_DISCONNECT_TO, - self->saddr, self->daddr, self->rname); + /* Cleanup our copy of the discovery log */ + kfree(self->discoveries); + self->discoveries = NULL; } - /* Prevent higher layer from accessing IrTTP */ - self->ttp_open = 0; - /* Close our IrTTP connection */ if(self->tsap) { @@ -761,7 +774,7 @@ irnet_find_socket(irnet_socket * self) while(new !=(irnet_socket *) NULL) { /* Is it available ? */ - if(!(new->ttp_open) && (new->rdaddr == DEV_ADDR_ANY) && + if(!(test_bit(0, &new->ttp_open)) && (new->rdaddr == DEV_ADDR_ANY) && (new->rname[0] == '\0') && (new->ppp_open)) { /* Yes !!! Get it.. */ @@ -788,17 +801,17 @@ irnet_find_socket(irnet_socket * self) * */ static inline int -irnet_connect_socket(irnet_socket * self, +irnet_connect_socket(irnet_socket * server, irnet_socket * new, struct qos_info * qos, __u32 max_sdu_size, __u8 max_header_size) { - DENTER(IRDA_SERV_TRACE, "(self=0x%X, new=0x%X)\n", - (unsigned int) self, (unsigned int) new); + DENTER(IRDA_SERV_TRACE, "(server=0x%X, new=0x%X)\n", + (unsigned int) server, (unsigned int) new); /* Now attach up the new socket */ - new->tsap = irttp_dup(self->tsap, new); + new->tsap = irttp_dup(server->tsap, new); DABORT(new->tsap == NULL, -1, IRDA_SERV_ERROR, "dup failed!\n"); /* Set up all the relevant parameters on the new socket */ @@ -817,17 +830,32 @@ irnet_connect_socket(irnet_socket * self #endif /* STREAM_COMPAT */ /* Clean up the original one to keep it in listen state */ - self->tsap->dtsap_sel = self->tsap->lsap->dlsap_sel = LSAP_ANY; - self->tsap->lsap->lsap_state = LSAP_DISCONNECTED; + server->tsap->dtsap_sel = server->tsap->lsap->dlsap_sel = LSAP_ANY; + server->tsap->lsap->lsap_state = LSAP_DISCONNECTED; /* Send a connection response on the new socket */ irttp_connect_response(new->tsap, new->max_sdu_size_rx, NULL); /* Allow PPP to send its junk over the new socket... */ - new->ttp_open = 1; - new->ttp_connect = 0; + set_bit(0, &new->ttp_open); + + /* Not connecting anymore, and clean up last possible remains + * of connection attempts on the socket */ + clear_bit(0, &new->ttp_connect); + if(new->iriap) + { + iriap_close(new->iriap); + new->iriap = NULL; + } + if(new->discoveries != NULL) + { + kfree(new->discoveries); + new->discoveries = NULL; + } + #ifdef CONNECT_INDIC_KICK - /* As currently we don't packets in ppp_irnet_send(), this is not needed... + /* As currently we don't block packets in ppp_irnet_send() while passive, + * this is not really needed... * Also, not doing it give IrDA a chance to finish the setup properly * before beeing swamped with packets... */ ppp_output_wakeup(&new->chan); @@ -835,7 +863,7 @@ irnet_connect_socket(irnet_socket * self /* Notify the control channel */ irnet_post_event(new, IRNET_CONNECT_FROM, - new->saddr, new->daddr, self->rname); + new->saddr, new->daddr, server->rname); DEXIT(IRDA_SERV_TRACE, "\n"); return 0; @@ -1053,12 +1081,33 @@ irnet_disconnect_indication(void * insta struct sk_buff *skb) { irnet_socket * self = (irnet_socket *) instance; + int test = 0; DENTER(IRDA_TCB_TRACE, "(self=0x%X)\n", (unsigned int) self); DASSERT(self != NULL, , IRDA_CB_ERROR, "Self is NULL !!!\n"); + /* Don't care about it, but let's not leak it */ + if(skb) + dev_kfree_skb(skb); + + /* Prevent higher layer from accessing IrTTP */ + test = test_and_clear_bit(0, &self->ttp_open); + /* Not connecting anymore... + * (note : TSAP is open, so IAP callbacks are no longer pending...) */ + test |= test_and_clear_bit(0, &self->ttp_connect); + + /* If both self->ttp_open and self->ttp_connect are NULL, it mean that we + * have a race condition with irda_irnet_destroy() or + * irnet_connect_indication(), so don't mess up tsap... + */ + if(!test) + { + DERROR(IRDA_CB_ERROR, "Race condition detected...\n"); + return; + } + /* If we were active, notify the control channel */ - if(self->ttp_open) + if(test_bit(0, &self->ttp_open)) irnet_post_event(self, IRNET_DISCONNECT_FROM, self->saddr, self->daddr, self->rname); else @@ -1067,15 +1116,10 @@ irnet_disconnect_indication(void * insta irnet_post_event(self, IRNET_NOANSWER_FROM, self->saddr, self->daddr, self->rname); - /* Prevent higher layer from accessing IrTTP */ - self->ttp_open = 0; - self->ttp_connect = 0; - - /* Close our IrTTP connection */ + /* Close our IrTTP connection, cleanup tsap */ if((self->tsap) && (self != &irnet_server.s)) { DEBUG(IRDA_CB_INFO, "Closing our TTP connection.\n"); - irttp_disconnect_request(self->tsap, NULL, P_NORMAL); irttp_close_tsap(self->tsap); self->tsap = NULL; @@ -1114,6 +1158,13 @@ irnet_connect_confirm(void * instance, DENTER(IRDA_TCB_TRACE, "(self=0x%X)\n", (unsigned int) self); + /* Check if socket is closing down (via irda_irnet_destroy()) */ + if(! test_bit(0, &self->ttp_connect)) + { + DERROR(IRDA_CB_ERROR, "Socket no longer connecting. Ouch !\n"); + return; + } + /* How much header space do we need to reserve */ self->max_header_size = max_header_size; @@ -1129,8 +1180,8 @@ irnet_connect_confirm(void * instance, self->saddr = irttp_get_saddr(self->tsap); /* Allow higher layer to access IrTTP */ - self->ttp_connect = 0; - self->ttp_open = 1; + set_bit(0, &self->ttp_open); + clear_bit(0, &self->ttp_connect); /* Not racy, IrDA traffic is serial */ /* Give a kick in the ass of ppp_generic so that he sends us some data */ ppp_output_wakeup(&self->chan); @@ -1251,56 +1302,76 @@ irnet_connect_indication(void * instanc __u8 max_header_size, struct sk_buff *skb) { - irnet_socket * self = &irnet_server.s; + irnet_socket * server = &irnet_server.s; irnet_socket * new = (irnet_socket *) NULL; - DENTER(IRDA_TCB_TRACE, "(self=0x%X)\n", (unsigned int) self); + DENTER(IRDA_TCB_TRACE, "(server=0x%X)\n", (unsigned int) server); DASSERT(instance == &irnet_server, , IRDA_CB_ERROR, "Invalid instance (0x%X) !!!\n", (unsigned int) instance); DASSERT(sap == irnet_server.s.tsap, , IRDA_CB_ERROR, "Invalid sap !!!\n"); /* Try to find the most appropriate IrNET socket */ - new = irnet_find_socket(self); + new = irnet_find_socket(server); /* After all this hard work, do we have an socket ? */ if(new == (irnet_socket *) NULL) { DEXIT(IRDA_CB_INFO, ": No socket waiting for this connection.\n"); - irnet_disconnect_server(self, skb); + irnet_disconnect_server(server, skb); return; } /* Is the socket already busy ? */ - if(new->ttp_open) + if(test_bit(0, &new->ttp_open)) { DEXIT(IRDA_CB_INFO, ": Socket already connected.\n"); - irnet_disconnect_server(self, skb); + irnet_disconnect_server(server, skb); return; } - /* Socket connecting */ - if(new->tsap != NULL) - { - /* The socket has sent a IrTTP connection request and is waiting for - * a connection response (that may never come). - * Now, the pain is that the socket has open a tsap and is waiting on it, - * while the other end is trying to connect to it on another tsap. - * Argh ! We will deal with that later... + /* Socket connecting ? + * Clear up flag : prevent irnet_disconnect_indication() to mess up tsap */ + if(test_and_clear_bit(0, &new->ttp_connect)) + { + /* The socket is trying to connect to the other end and may have sent + * a IrTTP connection request and is waiting for a connection response + * (that may never come). + * Now, the pain is that the socket may have opened a tsap and is + * waiting on it, while the other end is trying to connect to it on + * another tsap. */ DERROR(IRDA_CB_ERROR, "Socket already connecting. Ouch !\n"); #ifdef ALLOW_SIMULT_CONNECT - /* Close the connection the new socket was attempting. - * WARNING : This need more testing ! */ - irttp_close_tsap(new->tsap); + /* Cleanup the TSAP if necessary - IrIAP will be cleaned up later */ + if(new->tsap != NULL) + { + /* Close the connection the new socket was attempting. + * This seems to be safe... */ + irttp_close_tsap(new->tsap); + new->tsap = NULL; + } /* Note : no return, fall through... */ #else /* ALLOW_SIMULT_CONNECT */ - irnet_disconnect_server(self, skb); + irnet_disconnect_server(server, skb); return; #endif /* ALLOW_SIMULT_CONNECT */ } + else + /* If socket is not connecting or connected, tsap should be NULL */ + if(new->tsap != NULL) + { + /* If we are here, we are also in irnet_disconnect_indication(), + * and it's a nice race condition... On the other hand, we can't be + * in irda_irnet_destroy() otherwise we would not have found the + * socket in the hashbin. */ + /* Better get out of here, otherwise we will mess up tsaps ! */ + DERROR(IRDA_CB_ERROR, "Race condition detected, abort connect...\n"); + irnet_disconnect_server(server, skb); + return; + } /* So : at this point, we have a socket, and it is idle. Good ! */ - irnet_connect_socket(self, new, qos, max_sdu_size, max_header_size); + irnet_connect_socket(server, new, qos, max_sdu_size, max_header_size); /* Check size of received packet */ if(skb->len > 0) @@ -1349,24 +1420,25 @@ irnet_getvalue_confirm(int result, DENTER(IRDA_OCB_TRACE, "(self=0x%X)\n", (unsigned int) self); DASSERT(self != NULL, , IRDA_OCB_ERROR, "Self is NULL !!!\n"); - /* We probably don't need to make any more queries */ - iriap_close(self->iriap); - self->iriap = NULL; - - /* Check if already connected (via irnet_connect_socket()) */ - if(self->ttp_open) + /* Check if already connected (via irnet_connect_socket()) + * or socket is closing down (via irda_irnet_destroy()) */ + if(! test_bit(0, &self->ttp_connect)) { - DERROR(IRDA_OCB_ERROR, "Socket already connected. Ouch !\n"); + DERROR(IRDA_OCB_ERROR, "Socket no longer connecting. Ouch !\n"); return; } + /* We probably don't need to make any more queries */ + iriap_close(self->iriap); + self->iriap = NULL; + /* Post process the IAS reply */ self->dtsap_sel = irnet_ias_to_tsap(self, result, value); /* If error, just go out */ if(self->errno) { - self->ttp_connect = 0; + clear_bit(0, &self->ttp_connect); DERROR(IRDA_OCB_ERROR, "IAS connect failed ! (0x%X)\n", self->errno); return; } @@ -1412,6 +1484,14 @@ irnet_discovervalue_confirm(int result, DENTER(IRDA_OCB_TRACE, "(self=0x%X)\n", (unsigned int) self); DASSERT(self != NULL, , IRDA_OCB_ERROR, "Self is NULL !!!\n"); + /* Check if already connected (via irnet_connect_socket()) + * or socket is closing down (via irda_irnet_destroy()) */ + if(! test_bit(0, &self->ttp_connect)) + { + DERROR(IRDA_OCB_ERROR, "Socket no longer connecting. Ouch !\n"); + return; + } + /* Post process the IAS reply */ dtsap_sel = irnet_ias_to_tsap(self, result, value); @@ -1468,15 +1548,8 @@ irnet_discovervalue_confirm(int result, if(self->daddr == DEV_ADDR_ANY) { self->daddr = DEV_ADDR_ANY; - self->ttp_connect = 0; + clear_bit(0, &self->ttp_connect); DEXIT(IRDA_OCB_TRACE, ": cannot discover IrNET in any device !!!\n"); - return; - } - - /* Check if already connected (via irnet_connect_socket()) */ - if(self->ttp_open) - { - DERROR(IRDA_OCB_ERROR, "Socket already connected. Ouch !\n"); return; } diff -u -p linux/net/irda/irnet/irnet_ppp.d2.c linux/net/irda/irnet/irnet_ppp.c --- linux/net/irda/irnet/irnet_ppp.d2.c Thu Nov 1 12:14:43 2001 +++ linux/net/irda/irnet/irnet_ppp.c Thu Nov 1 14:11:53 2001 @@ -850,7 +850,7 @@ ppp_irnet_send(struct ppp_channel * chan DASSERT(self != NULL, 0, PPP_ERROR, "Self is NULL !!!\n"); /* Check if we are connected */ - if(self->ttp_open == 0) + if(!(test_bit(0, &self->ttp_open))) { #ifdef CONNECT_IN_SEND /* Let's try to connect one more time... */ @@ -884,7 +884,7 @@ ppp_irnet_send(struct ppp_channel * chan */ #ifdef BLOCK_WHEN_CONNECT /* If we are attempting to connect */ - if(self->ttp_connect) + if(test_bit(0, &self->ttp_connect)) { /* Blocking packet, ppp_generic will retry later */ return 0;