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 <[email protected]>
---
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
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
On Wed, Jun 06, 2012 at 01:38:59AM -0300, Gustavo Padovan wrote:
> Hi Andre,
>
> * Andre Guedes <[email protected]> [2012-06-05 16:01:42 -0300]:
>
> > Hi Gustavo,
> >
> > On Mon, Jun 4, 2012 at 4:17 AM, Szymon Janc <[email protected]> wrote:
> > > Hi,
> > >
> > >> * Andre Guedes <[email protected]> [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
Hi Andre,
* Andre Guedes <[email protected]> [2012-06-05 16:01:42 -0300]:
> Hi Gustavo,
>
> On Mon, Jun 4, 2012 at 4:17 AM, Szymon Janc <[email protected]> wrote:
> > Hi,
> >
> >> * Andre Guedes <[email protected]> [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
Hi Gustavo,
On Mon, Jun 4, 2012 at 4:17 AM, Szymon Janc <[email protected]> wrote:
> Hi,
>
>> * Andre Guedes <[email protected]> [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
Hi,
> * Andre Guedes <[email protected]> [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
Hi Andre,
* Andre Guedes <[email protected]> [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
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 <[email protected]>
> ---
> 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