[PATCH]bluetooth: rfcomm_init bug fix

[PATCH]bluetooth: rfcomm_init bug fix

[Dave Young]

rfcomm tty may be used before rfcomm_tty_driver initilized,
The problem is that now socket layer init before tty layer, if userspace
program do socket callback right here then oops will happen.

reporting in:
http://marc.info/?l=linux-bluetooth&m=124404919324542&w=2

make 3 changes:
1. remove #ifdef in rfcomm/core.c,
make it blank function when rfcomm tty not selected in rfcomm.h

2. tune the rfcomm_init error patch to ensure
tty driver initilized before rfcomm socket usage.

3. remove __exit for rfcomm_cleanup_sockets
because above change need call it in a __init function. 


CC: Alan Cox <alan@lxorguk.ukuu.org.uk>
Reported-by: Oliver Hartkopp <oliver@hartkopp.net>
Tested-by: Oliver Hartkopp <oliver@hartkopp.net>
Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
--
include/net/bluetooth/rfcomm.h |   12 +++++++++++-
net/bluetooth/rfcomm/core.c    |   29 ++++++++++++++++++++---------
net/bluetooth/rfcomm/sock.c    |    2 +-
3 files changed, 32 insertions(+), 11 deletions(-)

--- linux-2.6.orig/include/net/bluetooth/rfcomm.h	2009-08-01 13:56:53.000000000 +0800
+++ linux-2.6/include/net/bluetooth/rfcomm.h	2009-08-01 17:24:59.000000000 +0800
@@ -355,7 +355,17 @@ struct rfcomm_dev_list_req {
 };
 
 int  rfcomm_dev_ioctl(struct sock *sk, unsigned int cmd, void __user *arg);
+
+#ifdef CONFIG_BT_RFCOMM_TTY
 int  rfcomm_init_ttys(void);
 void rfcomm_cleanup_ttys(void);
-
+#else
+static inline int rfcomm_init_ttys(void)
+{
+	return 0;
+}
+static inline void rfcomm_cleanup_ttys(void)
+{
+}
+#endif
 #endif /* __RFCOMM_H */
--- linux-2.6.orig/net/bluetooth/rfcomm/core.c	2009-08-01 13:56:53.000000000 +0800
+++ linux-2.6/net/bluetooth/rfcomm/core.c	2009-08-01 13:57:18.000000000 +0800
@@ -2080,28 +2080,41 @@ static CLASS_ATTR(rfcomm_dlc, S_IRUGO, r
 /* ---- Initialization ---- */
 static int __init rfcomm_init(void)
 {
+	int ret;
+
 	l2cap_load();
 
 	hci_register_cb(&rfcomm_cb);
 
 	rfcomm_thread = kthread_run(rfcomm_run, NULL, "krfcommd");
 	if (IS_ERR(rfcomm_thread)) {
-		hci_unregister_cb(&rfcomm_cb);
-		return PTR_ERR(rfcomm_thread);
+		ret = PTR_ERR(rfcomm_thread);
+		goto out_thread;
 	}
 
 	if (class_create_file(bt_class, &class_attr_rfcomm_dlc) < 0)
 		BT_ERR("Failed to create RFCOMM info file");
 
-	rfcomm_init_sockets();
-
-#ifdef CONFIG_BT_RFCOMM_TTY
-	rfcomm_init_ttys();
-#endif
+	ret = rfcomm_init_ttys();
+	if (ret)
+		goto out_tty;
+
+	ret = rfcomm_init_sockets();
+	if (ret)
+		goto out_sock;
 
 	BT_INFO("RFCOMM ver %s", VERSION);
 
 	return 0;
+
+out_sock:
+	rfcomm_cleanup_ttys();
+out_tty:
+	kthread_stop(rfcomm_thread);
+out_thread:
+	hci_unregister_cb(&rfcomm_cb);
+
+	return ret;
 }
 
 static void __exit rfcomm_exit(void)
@@ -2112,9 +2125,7 @@ static void __exit rfcomm_exit(void)
 
 	kthread_stop(rfcomm_thread);
 
-#ifdef CONFIG_BT_RFCOMM_TTY
 	rfcomm_cleanup_ttys();
-#endif
 
 	rfcomm_cleanup_sockets();
 }
--- linux-2.6.orig/net/bluetooth/rfcomm/sock.c	2009-08-01 13:56:53.000000000 +0800
+++ linux-2.6/net/bluetooth/rfcomm/sock.c	2009-08-01 13:57:18.000000000 +0800
@@ -1132,7 +1132,7 @@ error:
 	return err;
 }
 
-void __exit rfcomm_cleanup_sockets(void)
+void rfcomm_cleanup_sockets(void)
 {
 	class_remove_file(bt_class, &class_attr_rfcomm);
 

Re: [PATCH]bluetooth: rfcomm_init bug fix

[David Miller]

From: Dave Young <hidave.darkstar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Mon, 3 Aug 2009 22:26:16 +0800

> rfcomm tty may be used before rfcomm_tty_driver initilized,
> The problem is that now socket layer init before tty layer, if userspace
> program do socket callback right here then oops will happen.
> 
> reporting in:
> http://marc.info/?l=linux-bluetooth&m=124404919324542&w=2
 ...
> Reported-by: Oliver Hartkopp <oliver-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>
> Tested-by: Oliver Hartkopp <oliver-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>
> Signed-off-by: Dave Young <hidave.darkstar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Applied, thanks.

Re: [PATCH]bluetooth: rfcomm_init bug fix

[Marcel Holtmann]

Hi Dave,

> > rfcomm tty may be used before rfcomm_tty_driver initilized,
> > The problem is that now socket layer init before tty layer, if userspace
> > program do socket callback right here then oops will happen.
> > 
> > reporting in:
> > http://marc.info/?l=linux-bluetooth&m=124404919324542&w=2
>  ...
> > Reported-by: Oliver Hartkopp <oliver-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>
> > Tested-by: Oliver Hartkopp <oliver-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>
> > Signed-off-by: Dave Young <hidave.darkstar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> Applied, thanks.

do you mind at least waiting for an ACK from my side. I haven't even
looked at the final patch.

Regards

Marcel

Re: [PATCH]bluetooth: rfcomm_init bug fix

[David Miller]

From: Marcel Holtmann <marcel@holtmann.org>
Date: Mon, 03 Aug 2009 16:54:26 -0700

> do you mind at least waiting for an ACK from my side. I haven't even
> looked at the final patch.

Sure, I haven't pushed it out yet, so now's your chance :)

Re: [PATCH]bluetooth: rfcomm_init bug fix

[David Miller]

From: David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Date: Mon, 03 Aug 2009 19:19:16 -0700 (PDT)

> From: Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
> Date: Mon, 03 Aug 2009 16:54:26 -0700
> 
>> do you mind at least waiting for an ACK from my side. I haven't even
>> looked at the final patch.
> 
> Sure, I haven't pushed it out yet, so now's your chance :)

Grumble, I accidently pushed my net-2.6 queue out before you
had a chance to reply, sorry :-(

Re: [PATCH]bluetooth: rfcomm_init bug fix

[Marcel Holtmann]

Hi Dave,

> >> do you mind at least waiting for an ACK from my side. I haven't even
> >> looked at the final patch.
> > 
> > Sure, I haven't pushed it out yet, so now's your chance :)
> 
> Grumble, I accidently pushed my net-2.6 queue out before you
> had a chance to reply, sorry :-(

not a big deal. Just have to send a cleanup patch. The patch itself is
fine, but from a style perspective it is different than everything else
in the Bluetooth subsystem and I would have liked to fix that before
pushing it. I wait with that for 2.6.32 since it is not important.

The other Dave, please send patches to linux-bluetooth only and lets
follow the normal path to get them merged into Linus' tree. Skipping
bluetooth-2.6 is not an option. You have to use the same process than
everybody else.

Regards

Marcel

Re: [PATCH]bluetooth: rfcomm_init bug fix

[Dave Young]

On Wed, Aug 5, 2009 at 1:01 AM, Marcel Holtmann<marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> wrote:
> Hi Dave,
>
>> >> do you mind at least waiting for an ACK from my side. I haven't even
>> >> looked at the final patch.
>> >
>> > Sure, I haven't pushed it out yet, so now's your chance :)
>>
>> Grumble, I accidently pushed my net-2.6 queue out before you
>> had a chance to reply, sorry :-(
>
> not a big deal. Just have to send a cleanup patch. The patch itself is
> fine, but from a style perspective it is different than everything else
> in the Bluetooth subsystem and I would have liked to fix that before
> pushing it. I wait with that for 2.6.32 since it is not important.

Marcel, could you publish the bluetooth subsystem coding style
somewhere? People including me are confused about it. I remembered I
asked it from you, but it is still not very clear for me.

>
> The other Dave, please send patches to linux-bluetooth only and lets
> follow the normal path to get them merged into Linus' tree. Skipping
> bluetooth-2.6 is not an option. You have to use the same process than
> everybody else.

Why linux-bluetooth only? IMHO there could be more people to comment
and review  if I send patches to both lkml and subsystem mailing list.

>
> Regards
>
> Marcel
>
>
>



-- 
Regards
dave

Re: [PATCH]bluetooth: rfcomm_init bug fix

[Marcel Holtmann]

Hi Dave,

> >> >> do you mind at least waiting for an ACK from my side. I haven't even
> >> >> looked at the final patch.
> >> >
> >> > Sure, I haven't pushed it out yet, so now's your chance :)
> >>
> >> Grumble, I accidently pushed my net-2.6 queue out before you
> >> had a chance to reply, sorry :-(
> >
> > not a big deal. Just have to send a cleanup patch. The patch itself is
> > fine, but from a style perspective it is different than everything else
> > in the Bluetooth subsystem and I would have liked to fix that before
> > pushing it. I wait with that for 2.6.32 since it is not important.
> 
> Marcel, could you publish the bluetooth subsystem coding style
> somewhere? People including me are confused about it. I remembered I
> asked it from you, but it is still not very clear for me.

it is kernel coding style like everything else, but some variable naming
should be consistent. And that is nothing that is published, that is
something that has been done since Max wrote it. Check the rest of the
code.

> > The other Dave, please send patches to linux-bluetooth only and lets
> > follow the normal path to get them merged into Linus' tree. Skipping
> > bluetooth-2.6 is not an option. You have to use the same process than
> > everybody else.
> 
> Why linux-bluetooth only? IMHO there could be more people to comment
> and review  if I send patches to both lkml and subsystem mailing list.

Lets me make this perfectly clear here. Bluetooth patches are sent to
the linux-bluetooth mailing list. There is no need to send them to
netdev and/or LKML at the same time. Stop this cross-posting. They don't
get more review there anyway. I take care of sending them for net-2.6
inclusion if I feel that they are appropriate.

Regards

Marcel

Re: [PATCH]bluetooth: rfcomm_init bug fix

[David Miller]

From: Marcel Holtmann <marcel@holtmann.org>
Date: Wed, 05 Aug 2009 08:35:55 -0700

> Lets me make this perfectly clear here. Bluetooth patches are sent to
> the linux-bluetooth mailing list. There is no need to send them to
> netdev and/or LKML at the same time. Stop this cross-posting. They don't
> get more review there anyway. I take care of sending them for net-2.6
> inclusion if I feel that they are appropriate.

Some of that isn't true, I was reading over his patches as he
posted them.  So cross-posting to netdev is absolutely not a
waste of time IMHO.

Re: [PATCH]bluetooth: rfcomm_init bug fix

[Marcel Holtmann]

Hi Dave,

> > Lets me make this perfectly clear here. Bluetooth patches are sent to
> > the linux-bluetooth mailing list. There is no need to send them to
> > netdev and/or LKML at the same time. Stop this cross-posting. They don't
> > get more review there anyway. I take care of sending them for net-2.6
> > inclusion if I feel that they are appropriate.
> 
> Some of that isn't true, I was reading over his patches as he
> posted them.  So cross-posting to netdev is absolutely not a
> waste of time IMHO.

this is great and netdev might make sense from time to time, but for
sure posting them to LKML is just noise. To be honest, if you wanna read
Bluetooth patches before they are acked by me, you could subscribe to
linux-bluetooth.

Regards

Marcel