On 07/17/2013 01:05 PM, Gianluca Anzolin wrote:
On Wed, Jul 17, 2013 at 10:02:31AM -0400, Peter Hurley wrote:
@@ -687,7 +665,9 @@ static void rfcomm_dev_state_change(struct rfcomm_dlc *dlc, int err)
return;
}
- rfcomm_dev_del(dev);
+ set_bit(RFCOMM_TTY_RELEASED, &dev->flags);
+ tty_port_put(&dev->port);
+
tty_port_put(&dev->port);
rfcomm_dlc_lock(dlc);
}
While this is functionally correct, it ignores the larger issue in
rfcomm_dev_state_change(); namely, what prevents the rfcomm_dev from being
destructed immediately after
struct rfcomm_dev *dev = dlc->owner;
If the answer to that question is the dlc lock, then the whole function is
_broken_.
No amount of reference counting will prevent the rfcomm_dev destructor
from completing once the dlc lock is dropped. (Presumably the dlc is not
subject to destruction once the lock is dropped. Is this true?)
This means:
1. Holding the dlc lock from the caller is pointless and should be dropped.
2. Some other solution is required to either preserve rfcomm_dev lifetime
or determine that destruction is already in progress.
I'm afraid I lied in the commit message: there are three places where the
tty_port may be released and the code above is the third one.
I wrote that message because at first I wanted to remove that code path but
then noticed I shouldn't.
Maybe we can simply save the dev->id before releasing the lock and then feed
that integer to rfcomm_dev_get? After all if the destruction is in progress
rfcomm_dev_get(id) == NULL and we return immediately. Otherwise we release the
tty_port.
I had forgotten about your earlier change to remove the rfcomm_dev node
from the list as the first op of the destructor. That does ensure
rfcomm_dev_get(id) will not return a rfcomm_dev which is destructing.
That said, preventing rfcomm_dev destruction by holding the dlc lock
is poor design (not that I'm suggesting you should be required to fix it though)
and something that at least needs documenting.
Regarding acquiring a snapshot of dev->id is fine, provided that the id
cannot be reallocated in between dropping the dlc lock and subsequently
scanning the rfcomm_dev_list for that id.
Regards,
Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
