[RFC] Bluetooth: Fix compile warning in l2cap_core.c

[RFC] Bluetooth: Fix compile warning in l2cap_core.c

[Andre Guedes]

This patch fixes the following warning reported by gcc 4.7.0:

net/bluetooth/l2cap_core.c: In function 'l2cap_config_rsp':
net/bluetooth/l2cap_core.c:3302:13: warning: 'rfc.max_pdu_size' may be used uninitialized in this function [-Wmaybe-uninitialized]
net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.max_pdu_size' was declared here
net/bluetooth/l2cap_core.c:3298:25: warning: 'rfc.monitor_timeout' may be used uninitialized in this function [-Wmaybe-uninitialized]
net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.monitor_timeout' was declared here
net/bluetooth/l2cap_core.c:3297:25: warning: 'rfc.retrans_timeout' may be used uninitialized in this function [-Wmaybe-uninitialized]
net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.retrans_timeout' was declared here
net/bluetooth/l2cap_core.c:3295:2: warning: 'rfc.mode' may be used uninitialized in this function [-Wmaybe-uninitialized]
net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.mode' was declared here

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/l2cap_core.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index f9bffe3..6df27cd 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3270,6 +3270,14 @@ static void l2cap_conf_rfc_get(struct l2cap_chan *chan, void *rsp, int len)
 	if ((chan->mode != L2CAP_MODE_ERTM) && (chan->mode != L2CAP_MODE_STREAMING))
 		return;
 
+	/* Use sane default values in case a misbehaving remote device
+	 * do not send an RFC option.
+	 */
+	rfc.mode = chan->mode;
+	rfc.retrans_timeout = __constant_cpu_to_le16(L2CAP_DEFAULT_RETRANS_TO);
+	rfc.monitor_timeout = __constant_cpu_to_le16(L2CAP_DEFAULT_MONITOR_TO);
+	rfc.max_pdu_size = cpu_to_le16(chan->imtu);
+
 	while (len >= L2CAP_CONF_OPT_SIZE) {
 		len -= l2cap_get_conf_opt(&rsp, &type, &olen, &val);
 
@@ -3281,14 +3289,6 @@ static void l2cap_conf_rfc_get(struct l2cap_chan *chan, void *rsp, int len)
 		}
 	}
 
-	/* Use sane default values in case a misbehaving remote device
-	 * did not send an RFC option.
-	 */
-	rfc.mode = chan->mode;
-	rfc.retrans_timeout = __constant_cpu_to_le16(L2CAP_DEFAULT_RETRANS_TO);
-	rfc.monitor_timeout = __constant_cpu_to_le16(L2CAP_DEFAULT_MONITOR_TO);
-	rfc.max_pdu_size = cpu_to_le16(chan->imtu);
-
 	BT_ERR("Expected RFC option was not found, using defaults");
 
 done:
-- 
1.7.10.2

Re: [RFC] Bluetooth: Fix compile warning in l2cap_core.c

[Andrei Emeltchenko]

Hi Andre,

On Thu, May 31, 2012 at 08:39:16PM -0300, Andre Guedes wrote:
> This patch fixes the following warning reported by gcc 4.7.0:
> 
> net/bluetooth/l2cap_core.c: In function 'l2cap_config_rsp':
> net/bluetooth/l2cap_core.c:3302:13: warning: 'rfc.max_pdu_size' may be used uninitialized in this function [-Wmaybe-uninitialized]
> net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.max_pdu_size' was declared here
> net/bluetooth/l2cap_core.c:3298:25: warning: 'rfc.monitor_timeout' may be used uninitialized in this function [-Wmaybe-uninitialized]
> net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.monitor_timeout' was declared here
> net/bluetooth/l2cap_core.c:3297:25: warning: 'rfc.retrans_timeout' may be used uninitialized in this function [-Wmaybe-uninitialized]
> net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.retrans_timeout' was declared here
> net/bluetooth/l2cap_core.c:3295:2: warning: 'rfc.mode' may be used uninitialized in this function [-Wmaybe-uninitialized]
> net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.mode' was declared here
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  net/bluetooth/l2cap_core.c |   16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index f9bffe3..6df27cd 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -3270,6 +3270,14 @@ static void l2cap_conf_rfc_get(struct l2cap_chan *chan, void *rsp, int len)
>  	if ((chan->mode != L2CAP_MODE_ERTM) && (chan->mode != L2CAP_MODE_STREAMING))
>  		return;
>  
> +	/* Use sane default values in case a misbehaving remote device
> +	 * do not send an RFC option.
> +	 */
> +	rfc.mode = chan->mode;
> +	rfc.retrans_timeout = __constant_cpu_to_le16(L2CAP_DEFAULT_RETRANS_TO);
> +	rfc.monitor_timeout = __constant_cpu_to_le16(L2CAP_DEFAULT_MONITOR_TO);
> +	rfc.max_pdu_size = cpu_to_le16(chan->imtu);
> +

I feel that this should be reworked better. If we do not get rfc we assign
to rfc those values from chan and make conversion cpu_to_le16 and then we
assign those values again to chan making reverse conversion.

Best regards 
Andrei Emeltchenko 

Re: [RFC] Bluetooth: Fix compile warning in l2cap_core.c

[Gustavo Padovan]

Hi Andre,

* Andre Guedes <andre.guedes@openbossa.org> [2012-05-31 20:39:16 -0300]:

> This patch fixes the following warning reported by gcc 4.7.0:
> 
> net/bluetooth/l2cap_core.c: In function 'l2cap_config_rsp':
> net/bluetooth/l2cap_core.c:3302:13: warning: 'rfc.max_pdu_size' may be used uninitialized in this function [-Wmaybe-uninitialized]
> net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.max_pdu_size' was declared here
> net/bluetooth/l2cap_core.c:3298:25: warning: 'rfc.monitor_timeout' may be used uninitialized in this function [-Wmaybe-uninitialized]
> net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.monitor_timeout' was declared here
> net/bluetooth/l2cap_core.c:3297:25: warning: 'rfc.retrans_timeout' may be used uninitialized in this function [-Wmaybe-uninitialized]
> net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.retrans_timeout' was declared here
> net/bluetooth/l2cap_core.c:3295:2: warning: 'rfc.mode' may be used uninitialized in this function [-Wmaybe-uninitialized]
> net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.mode' was declared here

So I think this is a false positive, I've seen this warning here for more than
a month, since I updated to fedora 17.
At some people will disable this warning in the kernel compile process if this
appear in others places in the kernel as false positive too.

	Gustavo

Re: [RFC] Bluetooth: Fix compile warning in l2cap_core.c

[Szymon Janc]

Hi,

> * Andre Guedes <andre.guedes@openbossa.org> [2012-05-31 20:39:16 -0300]:
> 
> > This patch fixes the following warning reported by gcc 4.7.0:
> > 
> > net/bluetooth/l2cap_core.c: In function 'l2cap_config_rsp':
> > net/bluetooth/l2cap_core.c:3302:13: warning: 'rfc.max_pdu_size' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.max_pdu_size' was declared here
> > net/bluetooth/l2cap_core.c:3298:25: warning: 'rfc.monitor_timeout' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.monitor_timeout' was declared here
> > net/bluetooth/l2cap_core.c:3297:25: warning: 'rfc.retrans_timeout' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.retrans_timeout' was declared here
> > net/bluetooth/l2cap_core.c:3295:2: warning: 'rfc.mode' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.mode' was declared here
> 
> So I think this is a false positive, I've seen this warning here for more than
> a month, since I updated to fedora 17.
> At some people will disable this warning in the kernel compile process if this
> appear in others places in the kernel as false positive too.

What if remote device misbehaves and sends bogus L2CAP_CONF_RFC i.e. with wrong length?


-- 
BR
Szymon

Re: [RFC] Bluetooth: Fix compile warning in l2cap_core.c

[Andre Guedes]

Hi Gustavo,

On Mon, Jun 4, 2012 at 4:17 AM, Szymon Janc <szymon.janc@tieto.com> wrote:
> Hi,
>
>> * Andre Guedes <andre.guedes@openbossa.org> [2012-05-31 20:39:16 -0300]:
>>
>> > This patch fixes the following warning reported by gcc 4.7.0:
>> >
>> > net/bluetooth/l2cap_core.c: In function 'l2cap_config_rsp':
>> > net/bluetooth/l2cap_core.c:3302:13: warning: 'rfc.max_pdu_size' may be used uninitialized in this function [-Wmaybe-uninitialized]
>> > net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.max_pdu_size' was declared here
>> > net/bluetooth/l2cap_core.c:3298:25: warning: 'rfc.monitor_timeout' may be used uninitialized in this function [-Wmaybe-uninitialized]
>> > net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.monitor_timeout' was declared here
>> > net/bluetooth/l2cap_core.c:3297:25: warning: 'rfc.retrans_timeout' may be used uninitialized in this function [-Wmaybe-uninitialized]
>> > net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.retrans_timeout' was declared here
>> > net/bluetooth/l2cap_core.c:3295:2: warning: 'rfc.mode' may be used uninitialized in this function [-Wmaybe-uninitialized]
>> > net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.mode' was declared here
>>
>> So I think this is a false positive, I've seen this warning here for more than
>> a month, since I updated to fedora 17.
>> At some people will disable this warning in the kernel compile process if this
>> appear in others places in the kernel as false positive too.
>
> What if remote device misbehaves and sends bogus L2CAP_CONF_RFC i.e. with wrong length?

I'm not sure this is a false positive. If remote device misbehaves and
sends bogus L2CAP_CONF_RFC with wrong length (as Szymon said) we go to
"done" label and 'rfc' is used uninitialized.

BR,

Andre

Re: [RFC] Bluetooth: Fix compile warning in l2cap_core.c

[Gustavo Padovan]

Hi Andre,

* Andre Guedes <andre.guedes@openbossa.org> [2012-06-05 16:01:42 -0300]:

> Hi Gustavo,
> 
> On Mon, Jun 4, 2012 at 4:17 AM, Szymon Janc <szymon.janc@tieto.com> wrote:
> > Hi,
> >
> >> * Andre Guedes <andre.guedes@openbossa.org> [2012-05-31 20:39:16 -0300]:
> >>
> >> > This patch fixes the following warning reported by gcc 4.7.0:
> >> >
> >> > net/bluetooth/l2cap_core.c: In function 'l2cap_config_rsp':
> >> > net/bluetooth/l2cap_core.c:3302:13: warning: 'rfc.max_pdu_size' may be used uninitialized in this function [-Wmaybe-uninitialized]
> >> > net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.max_pdu_size' was declared here
> >> > net/bluetooth/l2cap_core.c:3298:25: warning: 'rfc.monitor_timeout' may be used uninitialized in this function [-Wmaybe-uninitialized]
> >> > net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.monitor_timeout' was declared here
> >> > net/bluetooth/l2cap_core.c:3297:25: warning: 'rfc.retrans_timeout' may be used uninitialized in this function [-Wmaybe-uninitialized]
> >> > net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.retrans_timeout' was declared here
> >> > net/bluetooth/l2cap_core.c:3295:2: warning: 'rfc.mode' may be used uninitialized in this function [-Wmaybe-uninitialized]
> >> > net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.mode' was declared here
> >>
> >> So I think this is a false positive, I've seen this warning here for more than
> >> a month, since I updated to fedora 17.
> >> At some people will disable this warning in the kernel compile process if this
> >> appear in others places in the kernel as false positive too.
> >
> > What if remote device misbehaves and sends bogus L2CAP_CONF_RFC i.e. with wrong length?
> 
> I'm not sure this is a false positive. If remote device misbehaves and
> sends bogus L2CAP_CONF_RFC with wrong length (as Szymon said) we go to
> "done" label and 'rfc' is used uninitialized.

Yes, I agree with you guys, but please rebase this against the bluetooth tree
so we can put this in 3.5 and also rename the patch title as we are not fixing
a simple compile warning.

	Gustavo

Re: [RFC] Bluetooth: Fix compile warning in l2cap_core.c

[Andrei Emeltchenko]

On Wed, Jun 06, 2012 at 01:38:59AM -0300, Gustavo Padovan wrote:
> Hi Andre,
> 
> * Andre Guedes <andre.guedes@openbossa.org> [2012-06-05 16:01:42 -0300]:
> 
> > Hi Gustavo,
> > 
> > On Mon, Jun 4, 2012 at 4:17 AM, Szymon Janc <szymon.janc@tieto.com> wrote:
> > > Hi,
> > >
> > >> * Andre Guedes <andre.guedes@openbossa.org> [2012-05-31 20:39:16 -0300]:
> > >>
> > >> > This patch fixes the following warning reported by gcc 4.7.0:
> > >> >
> > >> > net/bluetooth/l2cap_core.c: In function 'l2cap_config_rsp':
> > >> > net/bluetooth/l2cap_core.c:3302:13: warning: 'rfc.max_pdu_size' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > >> > net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.max_pdu_size' was declared here
> > >> > net/bluetooth/l2cap_core.c:3298:25: warning: 'rfc.monitor_timeout' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > >> > net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.monitor_timeout' was declared here
> > >> > net/bluetooth/l2cap_core.c:3297:25: warning: 'rfc.retrans_timeout' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > >> > net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.retrans_timeout' was declared here
> > >> > net/bluetooth/l2cap_core.c:3295:2: warning: 'rfc.mode' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > >> > net/bluetooth/l2cap_core.c:3266:24: note: 'rfc.mode' was declared here
> > >>
> > >> So I think this is a false positive, I've seen this warning here for more than
> > >> a month, since I updated to fedora 17.
> > >> At some people will disable this warning in the kernel compile process if this
> > >> appear in others places in the kernel as false positive too.
> > >
> > > What if remote device misbehaves and sends bogus L2CAP_CONF_RFC i.e. with wrong length?
> > 
> > I'm not sure this is a false positive. If remote device misbehaves and
> > sends bogus L2CAP_CONF_RFC with wrong length (as Szymon said) we go to
> > "done" label and 'rfc' is used uninitialized.

what is not OK is that double conversion.

Best regards 
Andrei Emeltchenko 

Re: [RFC] Bluetooth: Fix compile warning in l2cap_core.c

[Szymon Janc]

Hi,

(as plain text this time...)

> > > I'm not sure this is a false positive. If remote device misbehaves and
> > > sends bogus L2CAP_CONF_RFC with wrong length (as Szymon said) we go to
> > > "done" label and 'rfc' is used uninitialized.
> 
> what is not OK is that double conversion.

Maybe sth like that? This function is expected to extract only conf_rfc anyway..

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index f9bffe3..42b8af6 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3273,12 +3273,14 @@ static void l2cap_conf_rfc_get(struct l2cap_chan *chan, void *rsp, int len)
        while (len >= L2CAP_CONF_OPT_SIZE) {
                len -= l2cap_get_conf_opt(&rsp, &type, &olen, &val);
 
-               switch (type) {
-               case L2CAP_CONF_RFC:
-                       if (olen == sizeof(rfc))
-                               memcpy(&rfc, (void *)val, olen);
-                       goto done;
-               }
+               if (type != L2CAP_CONF_RFC)
+                       continue;
+
+               if (olen != sizeof(rfc))
+                       break;
+
+               memcpy(&rfc, (void *)val, olen);
+               goto done;
        }

        /* Use sane default values in case a misbehaving remote device

-- 
BR
Szymon Janc