[Bluez-devel] rfcomm_sock_sendmsg with len==0 (in Linux 2.6.18)

[Bluez-devel] rfcomm_sock_sendmsg with len==0 (in Linux 2.6.18)

[Tuomas Suutari]

Hello.

I've made a buffer for socket connections to use with C++ iostreams. 
It's quite simple; just uses send and recv to fill and empty the buffer 
when needed.

It worked fine for a while, but yesterday some strange errors occured 
with it. After few hours of debugging I found that code responsible was 
using syscall send() to Bluetooth socket with buffer length set to 0. 
Problem was that I assumed it to return either -1 on error or 0 when 
success, but instead it returned positive values sometimes. 

Ok, it was stupid calling send() with len==0 anyway, but still (at least 
according to manual) send shouldn't return anything positive then. So I 
traced what kernel code is responsible and found it's 
rfcomm_sock_sendmsg() in net/bluetooth/rfcomm/sock.c. It returns 
uninitialized variable err, if called with len==0.

Simple fix is to initialize err to 0.

-- 
Tuomas Suutari | +358 50 3806983 | thsuut@utu.fi

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] rfcomm_sock_sendmsg with len==0 (in Linux 2.6.18)

[Marcel Holtmann]

Hi Tuomas,

> I've made a buffer for socket connections to use with C++ iostreams. 
> It's quite simple; just uses send and recv to fill and empty the buffer 
> when needed.
> 
> It worked fine for a while, but yesterday some strange errors occured 
> with it. After few hours of debugging I found that code responsible was 
> using syscall send() to Bluetooth socket with buffer length set to 0. 
> Problem was that I assumed it to return either -1 on error or 0 when 
> success, but instead it returned positive values sometimes. 
> 
> Ok, it was stupid calling send() with len==0 anyway, but still (at least 
> according to manual) send shouldn't return anything positive then. So I 
> traced what kernel code is responsible and found it's 
> rfcomm_sock_sendmsg() in net/bluetooth/rfcomm/sock.c. It returns 
> uninitialized variable err, if called with len==0.
> 
> Simple fix is to initialize err to 0.

this would only hide the real problem. It should only return err if the
sent is still 0. The return statement is

	return sent ? sent : err;

And sent is initialized with 0 and if len is also zero it will never
enter the while loop and thus not modify sent at all.

Please add some printk to the code before and after the loop. Something
is really wrong on your side. I would suspect a compiler error.

Regards

Marcel



-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] rfcomm_sock_sendmsg with len==0 (in Linux 2.6.18)

[Tuomas Suutari]

On 2006-12-19 Tuesday 11:39, Marcel Holtmann wrote:
> > rfcomm_sock_sendmsg() in net/bluetooth/rfcomm/sock.c. It returns
> > uninitialized variable err, if called with len==0.
> >
> > Simple fix is to initialize err to 0.
>
> this would only hide the real problem. It should only return err if
> the sent is still 0. The return statement is
>
> 	return sent ? sent : err;

Yep, exactly. It returns err, which isn't initialized, so it could be 
positive.

Am I missing something?

-- 
Tuomas Suutari | +358 50 3806983 | thsuut@utu.fi

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] rfcomm_sock_sendmsg with len==0 (in Linux 2.6.18)

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 631 bytes --]

Hi Tuomas,

> > > rfcomm_sock_sendmsg() in net/bluetooth/rfcomm/sock.c. It returns
> > > uninitialized variable err, if called with len==0.
> > >
> > > Simple fix is to initialize err to 0.
> >
> > this would only hide the real problem. It should only return err if
> > the sent is still 0. The return statement is
> >
> > 	return sent ? sent : err;
> 
> Yep, exactly. It returns err, which isn't initialized, so it could be 
> positive.
> 
> Am I missing something?

that is really strange. A recent compiler should detect that err can be
used uninitialized.

How about the attached patch. Does it work for you?

Regards

Marcel


[-- Attachment #2: patch --]
[-- Type: text/x-patch, Size: 1120 bytes --]

diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 544d65b..4297ff6 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -557,7 +557,6 @@ static int rfcomm_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 	struct sock *sk = sock->sk;
 	struct rfcomm_dlc *d = rfcomm_pi(sk)->dlc;
 	struct sk_buff *skb;
-	int err;
 	int sent = 0;
 
 	if (msg->msg_flags & MSG_OOB)
@@ -572,6 +571,7 @@ static int rfcomm_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 
 	while (len) {
 		size_t size = min_t(size_t, len, d->mtu);
+		int err;
 		
 		skb = sock_alloc_send_skb(sk, size + RFCOMM_SKB_RESERVE,
 				msg->msg_flags & MSG_DONTWAIT, &err);
@@ -589,6 +589,7 @@ static int rfcomm_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 		err = rfcomm_dlc_send(d, skb);
 		if (err < 0) {
 			kfree_skb(skb);
+			sent = err;
 			break;
 		}
 
@@ -598,7 +599,7 @@ static int rfcomm_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 
 	release_sock(sk);
 
-	return sent ? sent : err;
+	return sent;
 }
 
 static long rfcomm_sock_data_wait(struct sock *sk, long timeo)

[-- Attachment #3: Type: text/plain, Size: 347 bytes --]

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

[-- Attachment #4: Type: text/plain, Size: 164 bytes --]

_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] rfcomm_sock_sendmsg with len==0 (in Linux 2.6.18)

[Luciano Coelho]

ext Marcel Holtmann wrote:
> that is really strange. A recent compiler should detect that err can be
> used uninitialized.

Yes, it *should* ;-) But I have noticed at least one case in which GCC 
(version 3.4.4) doesn't recognize the use of uninitialized values... :-( 
It happened when compiling an ugly piece of code with gotos and stuff 
like that, but still...

Cheers,
Luca

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] rfcomm_sock_sendmsg with len==0 (in Linux 2.6.18)

[Tuomas Suutari]

On 2006-12-19 Tuesday 12:31, Marcel Holtmann wrote:
> that is really strange. A recent compiler should detect that err can
> be used uninitialized.

It (gcc 4.1.1) didn't, maybe that's because it can't know if len is 
always >0. Though it should have warned anyway.


> How about the attached patch. Does it work for you?

Yes. Now it returns 0 when called with len==0.

Thanks.

-- 
Tuomas Suutari | +358 50 3806983 | thsuut@utu.fi

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel