linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Bluetooth : Errata Service Release 8, Erratum 3253
@ 2018-09-24  9:16 Mallikarjun Phulari
  2018-09-27 10:37 ` Marcel Holtmann
  0 siblings, 1 reply; 9+ messages in thread
From: Mallikarjun Phulari @ 2018-09-24  9:16 UTC (permalink / raw)
  To: linux-bluetooth

L2CAP: New result values
	0x0006 - Connection refused – Invalid Source CID
	0x0007 - Connection refused – Source CID already allocated

As per the ESR08_V1.0.0, 1.11.2 Erratum 3253, Page No. 54,
"Remote CID invalid Issue".
Applies to Core Specification versions: V5.0, V4.2, v4.1, v4.0, and v3.0 + HS
Vol 3, Part A, Section 4.2, 4.3, 4.14, 4.15.

Core Specification Version 5.0, Page No.1753, Table 4.6 and
Page No. 1767, Table 4.14

New result values are added to l2cap connect/create channel response as
0x0006 - Connection refused – Invalid Source CID
0x0007 - Connection refused – Source CID already allocated

Signed-off-by: Mallikarjun Phulari <mallikarjun.phulari@intel.com>
---
 include/net/bluetooth/l2cap.h |  6 ++++++
 net/bluetooth/l2cap_core.c    | 17 ++++++++++++++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 0697fd4..c7f97cc 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -284,6 +284,12 @@ struct l2cap_conn_rsp {
 #define L2CAP_CR_INVALID_SCID	0x0009
 #define L2CAP_CR_SCID_IN_USE	0x000A
 
+/* connect/create channel results
+ * New result codesAs per ESR08_V1.0.0, Erratum 3253
+ */
+#define L2CAP_CR_BREDR_INVALID_SCID	0x0006
+#define L2CAP_CR_BREDR_SCID_IN_USE	0x0007
+
 /* connect/create channel status */
 #define L2CAP_CS_NO_INFO	0x0000
 #define L2CAP_CS_AUTHEN_PEND	0x0001
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index d17a473..d605748 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3815,9 +3815,24 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
 
 	result = L2CAP_CR_NO_MEM;
 
+	/* As per ESR08_V1.0.0, Erratum 3253, check the CID is in valid
+	 * dynamic range and is not allocated already.
+	 * Send the new result codes accordingly
+	 */
+
+	/* Check for valid dynamic CID range */
+	if (scid < L2CAP_CID_DYN_START || scid > L2CAP_CID_DYN_END) {
+		result = L2CAP_CR_BREDR_INVALID_SCID;
+		chan = NULL;
+		goto response;
+	}
+
 	/* Check if we already have channel with that dcid */
-	if (__l2cap_get_chan_by_dcid(conn, scid))
+	if (__l2cap_get_chan_by_dcid(conn, scid)) {
+		result = L2CAP_CR_BREDR_SCID_IN_USE;
+		chan = NULL;
 		goto response;
+	}
 
 	chan = pchan->ops->new_connection(pchan);
 	if (!chan)
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] Bluetooth : Errata Service Release 8, Erratum 3253
  2018-09-24  9:16 [PATCH] Bluetooth : Errata Service Release 8, Erratum 3253 Mallikarjun Phulari
@ 2018-09-27 10:37 ` Marcel Holtmann
  2018-10-01  9:26   ` Phulari, Mallikarjun
  0 siblings, 1 reply; 9+ messages in thread
From: Marcel Holtmann @ 2018-09-27 10:37 UTC (permalink / raw)
  To: Mallikarjun Phulari; +Cc: linux-bluetooth

Hi Mallikarjun,

> L2CAP: New result values
> 	0x0006 - Connection refused – Invalid Source CID
> 	0x0007 - Connection refused – Source CID already allocated
> 
> As per the ESR08_V1.0.0, 1.11.2 Erratum 3253, Page No. 54,
> "Remote CID invalid Issue".
> Applies to Core Specification versions: V5.0, V4.2, v4.1, v4.0, and v3.0 + HS
> Vol 3, Part A, Section 4.2, 4.3, 4.14, 4.15.
> 
> Core Specification Version 5.0, Page No.1753, Table 4.6 and
> Page No. 1767, Table 4.14
> 
> New result values are added to l2cap connect/create channel response as
> 0x0006 - Connection refused – Invalid Source CID
> 0x0007 - Connection refused – Source CID already allocated
> 
> Signed-off-by: Mallikarjun Phulari <mallikarjun.phulari@intel.com>
> ---
> include/net/bluetooth/l2cap.h |  6 ++++++
> net/bluetooth/l2cap_core.c    | 17 ++++++++++++++++-
> 2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 0697fd4..c7f97cc 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -284,6 +284,12 @@ struct l2cap_conn_rsp {
> #define L2CAP_CR_INVALID_SCID	0x0009
> #define L2CAP_CR_SCID_IN_USE	0x000A
> 
> +/* connect/create channel results
> + * New result codesAs per ESR08_V1.0.0, Erratum 3253
> + */
> +#define L2CAP_CR_BREDR_INVALID_SCID	0x0006
> +#define L2CAP_CR_BREDR_SCID_IN_USE	0x0007
> +

I am confused here. We currently have this list:

/* connect/create channel results */
#define L2CAP_CR_SUCCESS        0x0000
#define L2CAP_CR_PEND           0x0001
#define L2CAP_CR_BAD_PSM        0x0002
#define L2CAP_CR_SEC_BLOCK      0x0003
#define L2CAP_CR_NO_MEM         0x0004
#define L2CAP_CR_BAD_AMP        0x0005
#define L2CAP_CR_AUTHENTICATION 0x0005
#define L2CAP_CR_AUTHORIZATION  0x0006
#define L2CAP_CR_BAD_KEY_SIZE   0x0007
#define L2CAP_CR_ENCRYPTION     0x0008
#define L2CAP_CR_INVALID_SCID   0x0009
#define L2CAP_CR_SCID_IN_USE    0x000A

Now here we have 0x0006 and 0x0007 taken. Have these been changed now. The L2CAP_CR_AUTHORIZATION we actually do use in LE and if this table is not treated with independent name spaces for BR/EDR vs LE, then we need to split this one first and get the LE only ones proper names and fix their usage.

Regards

Marcel


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] Bluetooth : Errata Service Release 8, Erratum 3253
  2018-09-27 10:37 ` Marcel Holtmann
@ 2018-10-01  9:26   ` Phulari, Mallikarjun
  2018-10-03  6:56     ` Marcel Holtmann
  0 siblings, 1 reply; 9+ messages in thread
From: Phulari, Mallikarjun @ 2018-10-01  9:26 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth

Hi Marcel,

> Hi Mallikarjun,
> 
> > L2CAP: New result values
> > 	0x0006 - Connection refused – Invalid Source CID
> > 	0x0007 - Connection refused – Source CID already allocated
> >
> > As per the ESR08_V1.0.0, 1.11.2 Erratum 3253, Page No. 54, "Remote CID
> > invalid Issue".
> > Applies to Core Specification versions: V5.0, V4.2, v4.1, v4.0, and
> > v3.0 + HS Vol 3, Part A, Section 4.2, 4.3, 4.14, 4.15.
> >
> > Core Specification Version 5.0, Page No.1753, Table 4.6 and Page No.
> > 1767, Table 4.14
> >
> > New result values are added to l2cap connect/create channel response
> > as
> > 0x0006 - Connection refused – Invalid Source CID
> > 0x0007 - Connection refused – Source CID already allocated
> >
> > Signed-off-by: Mallikarjun Phulari <mallikarjun.phulari@intel.com>
> > ---
> > include/net/bluetooth/l2cap.h |  6 ++++++
> > net/bluetooth/l2cap_core.c    | 17 ++++++++++++++++-
> > 2 files changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/net/bluetooth/l2cap.h
> > b/include/net/bluetooth/l2cap.h index 0697fd4..c7f97cc 100644
> > --- a/include/net/bluetooth/l2cap.h
> > +++ b/include/net/bluetooth/l2cap.h
> > @@ -284,6 +284,12 @@ struct l2cap_conn_rsp {
> > #define L2CAP_CR_INVALID_SCID	0x0009
> > #define L2CAP_CR_SCID_IN_USE	0x000A
> >
> > +/* connect/create channel results
> > + * New result codesAs per ESR08_V1.0.0, Erratum 3253  */
> > +#define L2CAP_CR_BREDR_INVALID_SCID	0x0006
> > +#define L2CAP_CR_BREDR_SCID_IN_USE	0x0007
> > +
> 
> I am confused here. We currently have this list:
> 
> /* connect/create channel results */
> #define L2CAP_CR_SUCCESS        0x0000
> #define L2CAP_CR_PEND           0x0001
> #define L2CAP_CR_BAD_PSM        0x0002
> #define L2CAP_CR_SEC_BLOCK      0x0003
> #define L2CAP_CR_NO_MEM         0x0004
> #define L2CAP_CR_BAD_AMP        0x0005
> #define L2CAP_CR_AUTHENTICATION 0x0005
> #define L2CAP_CR_AUTHORIZATION  0x0006
> #define L2CAP_CR_BAD_KEY_SIZE   0x0007
> #define L2CAP_CR_ENCRYPTION     0x0008
> #define L2CAP_CR_INVALID_SCID   0x0009
> #define L2CAP_CR_SCID_IN_USE    0x000A
> 
> Now here we have 0x0006 and 0x0007 taken. Have these been changed now.
> The L2CAP_CR_AUTHORIZATION we actually do use in LE and if this table is not
> treated with independent name spaces for BR/EDR vs LE, then we need to split
> this one first and get the LE only ones proper names and fix their usage.
> 
The current name spaces that you have mentioned above, have the combined result values for LE Credit base Connection (VOL 3, Part A, 4.23, Table 4.20) and for L2CAP Create/Connect (VOL3, Part A, 4.3 /4.15, Table 4.6/4.14).
To create separate name spaces for each set of error codes for L2CAP channel request, L2CAP create Connection and LE credit based create connection will add more rework to replace the error codes where these are being used.

Therefore, to avoid the rework , I had used the result values as:

#define L2CAP_CR_BREDR_INVALID_SCID	0x0006
#define L2CAP_CR_BREDR_SCID_IN_USE	0x0007

If you suggest to separate the name spaces for LE and BREDR result values, I shall re-send the v3 patch with the changes.

> Regards
> 
> Marcel

Regards,
Mallikarjun Phulari

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Bluetooth : Errata Service Release 8, Erratum 3253
  2018-10-01  9:26   ` Phulari, Mallikarjun
@ 2018-10-03  6:56     ` Marcel Holtmann
  2018-10-03 11:59       ` Phulari, Mallikarjun
  0 siblings, 1 reply; 9+ messages in thread
From: Marcel Holtmann @ 2018-10-03  6:56 UTC (permalink / raw)
  To: Phulari, Mallikarjun; +Cc: linux-bluetooth

Hi Malikarjun,

>>> L2CAP: New result values
>>> 	0x0006 - Connection refused – Invalid Source CID
>>> 	0x0007 - Connection refused – Source CID already allocated
>>> 
>>> As per the ESR08_V1.0.0, 1.11.2 Erratum 3253, Page No. 54, "Remote CID
>>> invalid Issue".
>>> Applies to Core Specification versions: V5.0, V4.2, v4.1, v4.0, and
>>> v3.0 + HS Vol 3, Part A, Section 4.2, 4.3, 4.14, 4.15.
>>> 
>>> Core Specification Version 5.0, Page No.1753, Table 4.6 and Page No.
>>> 1767, Table 4.14
>>> 
>>> New result values are added to l2cap connect/create channel response
>>> as
>>> 0x0006 - Connection refused – Invalid Source CID
>>> 0x0007 - Connection refused – Source CID already allocated
>>> 
>>> Signed-off-by: Mallikarjun Phulari <mallikarjun.phulari@intel.com>
>>> ---
>>> include/net/bluetooth/l2cap.h |  6 ++++++
>>> net/bluetooth/l2cap_core.c    | 17 ++++++++++++++++-
>>> 2 files changed, 22 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/include/net/bluetooth/l2cap.h
>>> b/include/net/bluetooth/l2cap.h index 0697fd4..c7f97cc 100644
>>> --- a/include/net/bluetooth/l2cap.h
>>> +++ b/include/net/bluetooth/l2cap.h
>>> @@ -284,6 +284,12 @@ struct l2cap_conn_rsp {
>>> #define L2CAP_CR_INVALID_SCID	0x0009
>>> #define L2CAP_CR_SCID_IN_USE	0x000A
>>> 
>>> +/* connect/create channel results
>>> + * New result codesAs per ESR08_V1.0.0, Erratum 3253  */
>>> +#define L2CAP_CR_BREDR_INVALID_SCID	0x0006
>>> +#define L2CAP_CR_BREDR_SCID_IN_USE	0x0007
>>> +
>> 
>> I am confused here. We currently have this list:
>> 
>> /* connect/create channel results */
>> #define L2CAP_CR_SUCCESS        0x0000
>> #define L2CAP_CR_PEND           0x0001
>> #define L2CAP_CR_BAD_PSM        0x0002
>> #define L2CAP_CR_SEC_BLOCK      0x0003
>> #define L2CAP_CR_NO_MEM         0x0004
>> #define L2CAP_CR_BAD_AMP        0x0005
>> #define L2CAP_CR_AUTHENTICATION 0x0005
>> #define L2CAP_CR_AUTHORIZATION  0x0006
>> #define L2CAP_CR_BAD_KEY_SIZE   0x0007
>> #define L2CAP_CR_ENCRYPTION     0x0008
>> #define L2CAP_CR_INVALID_SCID   0x0009
>> #define L2CAP_CR_SCID_IN_USE    0x000A
>> 
>> Now here we have 0x0006 and 0x0007 taken. Have these been changed now.
>> The L2CAP_CR_AUTHORIZATION we actually do use in LE and if this table is not
>> treated with independent name spaces for BR/EDR vs LE, then we need to split
>> this one first and get the LE only ones proper names and fix their usage.
>> 
> The current name spaces that you have mentioned above, have the combined result values for LE Credit base Connection (VOL 3, Part A, 4.23, Table 4.20) and for L2CAP Create/Connect (VOL3, Part A, 4.3 /4.15, Table 4.6/4.14).
> To create separate name spaces for each set of error codes for L2CAP channel request, L2CAP create Connection and LE credit based create connection will add more rework to replace the error codes where these are being used.
> 
> Therefore, to avoid the rework , I had used the result values as:
> 
> #define L2CAP_CR_BREDR_INVALID_SCID	0x0006
> #define L2CAP_CR_BREDR_SCID_IN_USE	0x0007
> 
> If you suggest to separate the name spaces for LE and BREDR result values, I shall re-send the v3 patch with the changes.

if the currently 0x0006-0x000A are LE only, then we should change their names to L2CAP_CR_LE_* and convert their usages. And then add the new BR/EDR only 0x0006-0x0007 into the list (without _BREDR in the name).

Regards

Marcel


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] Bluetooth : Errata Service Release 8, Erratum 3253
  2018-10-03  6:56     ` Marcel Holtmann
@ 2018-10-03 11:59       ` Phulari, Mallikarjun
  0 siblings, 0 replies; 9+ messages in thread
From: Phulari, Mallikarjun @ 2018-10-03 11:59 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth

Hi Marcel,

> -----Original Message-----
> From: Marcel Holtmann [mailto:marcel@holtmann.org]
> Sent: Wednesday, October 3, 2018 12:26 PM
> To: Phulari, Mallikarjun <mallikarjun.phulari@intel.com>
> Cc: linux-bluetooth@vger.kernel.org
> Subject: Re: [PATCH] Bluetooth : Errata Service Release 8, Erratum 3253
> 
> Hi Malikarjun,
> 
> >>> L2CAP: New result values
> >>> 	0x0006 - Connection refused – Invalid Source CID
> >>> 	0x0007 - Connection refused – Source CID already allocated
> >>>
> >>> As per the ESR08_V1.0.0, 1.11.2 Erratum 3253, Page No. 54, "Remote
> >>> CID invalid Issue".
> >>> Applies to Core Specification versions: V5.0, V4.2, v4.1, v4.0, and
> >>> v3.0 + HS Vol 3, Part A, Section 4.2, 4.3, 4.14, 4.15.
> >>>
> >>> Core Specification Version 5.0, Page No.1753, Table 4.6 and Page No.
> >>> 1767, Table 4.14
> >>>
> >>> New result values are added to l2cap connect/create channel response
> >>> as
> >>> 0x0006 - Connection refused – Invalid Source CID
> >>> 0x0007 - Connection refused – Source CID already allocated
> >>>
> >>> Signed-off-by: Mallikarjun Phulari <mallikarjun.phulari@intel.com>
> >>> ---
> >>> include/net/bluetooth/l2cap.h |  6 ++++++
> >>> net/bluetooth/l2cap_core.c    | 17 ++++++++++++++++-
> >>> 2 files changed, 22 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/include/net/bluetooth/l2cap.h
> >>> b/include/net/bluetooth/l2cap.h index 0697fd4..c7f97cc 100644
> >>> --- a/include/net/bluetooth/l2cap.h
> >>> +++ b/include/net/bluetooth/l2cap.h
> >>> @@ -284,6 +284,12 @@ struct l2cap_conn_rsp {
> >>> #define L2CAP_CR_INVALID_SCID	0x0009
> >>> #define L2CAP_CR_SCID_IN_USE	0x000A
> >>>
> >>> +/* connect/create channel results
> >>> + * New result codesAs per ESR08_V1.0.0, Erratum 3253  */
> >>> +#define L2CAP_CR_BREDR_INVALID_SCID	0x0006
> >>> +#define L2CAP_CR_BREDR_SCID_IN_USE	0x0007
> >>> +
> >>
> >> I am confused here. We currently have this list:
> >>
> >> /* connect/create channel results */
> >> #define L2CAP_CR_SUCCESS        0x0000
> >> #define L2CAP_CR_PEND           0x0001
> >> #define L2CAP_CR_BAD_PSM        0x0002
> >> #define L2CAP_CR_SEC_BLOCK      0x0003
> >> #define L2CAP_CR_NO_MEM         0x0004
> >> #define L2CAP_CR_BAD_AMP        0x0005
> >> #define L2CAP_CR_AUTHENTICATION 0x0005 #define
> L2CAP_CR_AUTHORIZATION
> >> 0x0006
> >> #define L2CAP_CR_BAD_KEY_SIZE   0x0007
> >> #define L2CAP_CR_ENCRYPTION     0x0008
> >> #define L2CAP_CR_INVALID_SCID   0x0009
> >> #define L2CAP_CR_SCID_IN_USE    0x000A
> >>
> >> Now here we have 0x0006 and 0x0007 taken. Have these been changed now.
> >> The L2CAP_CR_AUTHORIZATION we actually do use in LE and if this table
> >> is not treated with independent name spaces for BR/EDR vs LE, then we
> >> need to split this one first and get the LE only ones proper names and fix their
> usage.
> >>
> > The current name spaces that you have mentioned above, have the combined
> result values for LE Credit base Connection (VOL 3, Part A, 4.23, Table 4.20) and
> for L2CAP Create/Connect (VOL3, Part A, 4.3 /4.15, Table 4.6/4.14).
> > To create separate name spaces for each set of error codes for L2CAP channel
> request, L2CAP create Connection and LE credit based create connection will
> add more rework to replace the error codes where these are being used.
> >
> > Therefore, to avoid the rework , I had used the result values as:
> >
> > #define L2CAP_CR_BREDR_INVALID_SCID	0x0006
> > #define L2CAP_CR_BREDR_SCID_IN_USE	0x0007
> >
> > If you suggest to separate the name spaces for LE and BREDR result values, I
> shall re-send the v3 patch with the changes.
> 
> if the currently 0x0006-0x000A are LE only, then we should change their names
> to L2CAP_CR_LE_* and convert their usages. And then add the new BR/EDR only
> 0x0006-0x0007 into the list (without _BREDR in the name).

I will change the names of the result values and their usages as you suggested and I will send you the new patch.

> 
> Regards
> 
> Marcel

Thanks & Regards
Mallikarjun Phulari

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Bluetooth : Errata Service Release 8 Erratum 3253
  2018-09-21  4:37   ` Phulari, Mallikarjun
@ 2018-09-21 11:44     ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 9+ messages in thread
From: Luiz Augusto von Dentz @ 2018-09-21 11:44 UTC (permalink / raw)
  To: Phulari, Mallikarjun; +Cc: linux-bluetooth

Hi Mallikarjun,

On Fri, Sep 21, 2018 at 7:37 AM, Phulari, Mallikarjun
<mallikarjun.phulari@intel.com> wrote:
> Hi Luiz,
> Please find the new patch with review comments incorporated.

Please send a proper patch not as attachment.

> *I have added the Page number of Core Specification for reference in the commit message and included the "signed of by" in the patch.
>
> *L2CAP_CR_INVALID_SCID and L2CAP_CR_SCID_IN_USE are specific to LE with different result values as:
>  L2CAP_CR_INVALID_SCID  -  0x0009
>  L2CAP_CR_SCID_IN_USE   -  0x000A

Alright, so they probably need to be renamed to
L2CAP_CR_LE_INVALID_SCID and L2CAP_CR_LE_SCID_IN_USE so we don't
confuse these with BR codes.

> It is already implemented.
>
> Thanks & Regards
> Mallikarjun Phulari
>
> _____________________________________
> From: Luiz Augusto von Dentz [luiz.dentz@gmail.com]
> Sent: Wednesday, September 19, 2018 5:06 AM
> To: Phulari, Mallikarjun
> Cc: linux-bluetooth@vger.kernel.org
> Subject: Re: [PATCH] Bluetooth : Errata Service Release 8 Erratum 3253
>
> Hi Mallikarjun,
>
> On Wed, Sep 19, 2018 at 12:51 PM, Mallikarjun Phulari
> <mallikarjun.phulari@intel.com> wrote:
>> L2CAP: Changes include the new result codes for the l2cap channel
>> create/connect request. The new result code are:
>> 0x0006 - sent in the response when the CID is not in valid dynamic range.
>> 0x0007 sent in the response when the CID is already allocated.
>
> It would be a good idea to quote the erratum with page number where
> the new errors come from. Btw, you should include a signed of by in
> the kernel patches.
>
>> ---
>>  include/net/bluetooth/l2cap.h |  6 ++++++
>>  net/bluetooth/l2cap_core.c    | 12 ++++++++++++
>>  2 files changed, 18 insertions(+)
>>
>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>> index 0697fd4..eb0c8d0 100644
>> --- a/include/net/bluetooth/l2cap.h
>> +++ b/include/net/bluetooth/l2cap.h
>> @@ -284,6 +284,12 @@ struct l2cap_conn_rsp {
>>  #define L2CAP_CR_INVALID_SCID  0x0009
>>  #define L2CAP_CR_SCID_IN_USE   0x000A
>>
>> +/* connect/create channel results
>> + * As per Erratum 3253
>> + */
>> +#define L2CAP_CR_BREDR_INVALID_SCID    0x0006
>> +#define L2CAP_CR_BREDR_SCID_IN_USE     0x0007
>
> Weird, are L2CAP_CR_INVALID_SCID and L2CAP_CR_SCID_IN_USE specific to
> LE? If they are we should probably fix their names.
>
>>  /* connect/create channel status */
>>  #define L2CAP_CS_NO_INFO       0x0000
>>  #define L2CAP_CS_AUTHEN_PEND   0x0001
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index 9b7907e..85887df 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -3814,9 +3814,21 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
>>         }
>>
>>         result = L2CAP_CR_NO_MEM;
>> +       /* As per Erratum 3253, check the CID is in valid dynamic range and
>> +        *  is not allocated already. Send the new result codes accordingly
>> +        */
>> +
>> +       /* Check for valid dynamic CID range */
>> +       if (scid < L2CAP_CID_DYN_START || scid > L2CAP_CID_DYN_END) {
>> +               result = L2CAP_CR_BREDR_INVALID_SCID;
>> +               chan = NULL;
>> +               goto response;
>> +       }
>>
>>         /* Check if we already have channel with that dcid */
>>         if (__l2cap_get_chan_by_dcid(conn, scid))
>> +               result = L2CAP_CR_BREDR_SCID_IN_USE;
>> +               chan = NULL;
>>                 goto response;
>
> Missing { } above?
>
>>
>>         chan = pchan->ops->new_connection(pchan);
>> --
>> 2.7.4
>>
>
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] Bluetooth : Errata Service Release 8 Erratum 3253
  2018-09-19 12:06 ` Luiz Augusto von Dentz
@ 2018-09-21  4:37   ` Phulari, Mallikarjun
  2018-09-21 11:44     ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 9+ messages in thread
From: Phulari, Mallikarjun @ 2018-09-21  4:37 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

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

Hi Luiz,
Please find the new patch with review comments incorporated.

*I have added the Page number of Core Specification for reference in the commit message and included the "signed of by" in the patch.

*L2CAP_CR_INVALID_SCID and L2CAP_CR_SCID_IN_USE are specific to LE with different result values as:
 L2CAP_CR_INVALID_SCID  -  0x0009
 L2CAP_CR_SCID_IN_USE   -  0x000A
It is already implemented.

Thanks & Regards
Mallikarjun Phulari

_____________________________________
From: Luiz Augusto von Dentz [luiz.dentz@gmail.com]
Sent: Wednesday, September 19, 2018 5:06 AM
To: Phulari, Mallikarjun
Cc: linux-bluetooth@vger.kernel.org
Subject: Re: [PATCH] Bluetooth : Errata Service Release 8 Erratum 3253

Hi Mallikarjun,

On Wed, Sep 19, 2018 at 12:51 PM, Mallikarjun Phulari
<mallikarjun.phulari@intel.com> wrote:
> L2CAP: Changes include the new result codes for the l2cap channel
> create/connect request. The new result code are:
> 0x0006 - sent in the response when the CID is not in valid dynamic range.
> 0x0007 sent in the response when the CID is already allocated.

It would be a good idea to quote the erratum with page number where
the new errors come from. Btw, you should include a signed of by in
the kernel patches.

> ---
>  include/net/bluetooth/l2cap.h |  6 ++++++
>  net/bluetooth/l2cap_core.c    | 12 ++++++++++++
>  2 files changed, 18 insertions(+)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 0697fd4..eb0c8d0 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -284,6 +284,12 @@ struct l2cap_conn_rsp {
>  #define L2CAP_CR_INVALID_SCID  0x0009
>  #define L2CAP_CR_SCID_IN_USE   0x000A
>
> +/* connect/create channel results
> + * As per Erratum 3253
> + */
> +#define L2CAP_CR_BREDR_INVALID_SCID    0x0006
> +#define L2CAP_CR_BREDR_SCID_IN_USE     0x0007

Weird, are L2CAP_CR_INVALID_SCID and L2CAP_CR_SCID_IN_USE specific to
LE? If they are we should probably fix their names.

>  /* connect/create channel status */
>  #define L2CAP_CS_NO_INFO       0x0000
>  #define L2CAP_CS_AUTHEN_PEND   0x0001
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 9b7907e..85887df 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -3814,9 +3814,21 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
>         }
>
>         result = L2CAP_CR_NO_MEM;
> +       /* As per Erratum 3253, check the CID is in valid dynamic range and
> +        *  is not allocated already. Send the new result codes accordingly
> +        */
> +
> +       /* Check for valid dynamic CID range */
> +       if (scid < L2CAP_CID_DYN_START || scid > L2CAP_CID_DYN_END) {
> +               result = L2CAP_CR_BREDR_INVALID_SCID;
> +               chan = NULL;
> +               goto response;
> +       }
>
>         /* Check if we already have channel with that dcid */
>         if (__l2cap_get_chan_by_dcid(conn, scid))
> +               result = L2CAP_CR_BREDR_SCID_IN_USE;
> +               chan = NULL;
>                 goto response;

Missing { } above?

>
>         chan = pchan->ops->new_connection(pchan);
> --
> 2.7.4
>



--
Luiz Augusto von Dentz

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Bluetooth-Errata-Service-Release-8-Erratum-3253.patch --]
[-- Type: text/x-patch; name="0001-Bluetooth-Errata-Service-Release-8-Erratum-3253.patch", Size: 2689 bytes --]

From 3d5e7712f2d7a1d59af63c4b2aac547ad9826589 Mon Sep 17 00:00:00 2001
From: Mallikarjun Phulari <mallikarjun.phulari@intel.com>
Date: Fri, 21 Sep 2018 09:18:35 +0530
Subject: [PATCH] Bluetooth : Errata Service Release 8, Erratum 3253
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

L2CAP: New result values
	0x0006 - Connection refused – Invalid Source CID
	0x0007 - Connection refused – Source CID already allocated

As per the ESR08_V1.0.0, 1.11.2 Erratum 3253, Page No. 54,
"Remote CID invalid Issue".
Applies to Core Specification versions: V5.0, V4.2, v4.1, v4.0, and v3.0 + HS
Vol 3, Part A, Section 4.2, 4.3, 4.14, 4.15.

Core Specification Version 5.0, Page No.1753, Table 4.6 and
Page No. 1767, Table 4.14

New result values are added to l2cap connect/create channel response as
0x0006 - Connection refused – Invalid Source CID
0x0007 - Connection refused – Source CID already allocated

Signed-off-by: Mallikarjun Phulari <mallikarjun.phulari@intel.com>
---
 include/net/bluetooth/l2cap.h |  6 ++++++
 net/bluetooth/l2cap_core.c    | 17 ++++++++++++++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 0697fd4..c7f97cc 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -284,6 +284,12 @@ struct l2cap_conn_rsp {
 #define L2CAP_CR_INVALID_SCID	0x0009
 #define L2CAP_CR_SCID_IN_USE	0x000A
 
+/* connect/create channel results
+ * New result codesAs per ESR08_V1.0.0, Erratum 3253
+ */
+#define L2CAP_CR_BREDR_INVALID_SCID	0x0006
+#define L2CAP_CR_BREDR_SCID_IN_USE	0x0007
+
 /* connect/create channel status */
 #define L2CAP_CS_NO_INFO	0x0000
 #define L2CAP_CS_AUTHEN_PEND	0x0001
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index d17a473..d605748 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3815,9 +3815,24 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
 
 	result = L2CAP_CR_NO_MEM;
 
+	/* As per ESR08_V1.0.0, Erratum 3253, check the CID is in valid
+	 * dynamic range and is not allocated already.
+	 * Send the new result codes accordingly
+	 */
+
+	/* Check for valid dynamic CID range */
+	if (scid < L2CAP_CID_DYN_START || scid > L2CAP_CID_DYN_END) {
+		result = L2CAP_CR_BREDR_INVALID_SCID;
+		chan = NULL;
+		goto response;
+	}
+
 	/* Check if we already have channel with that dcid */
-	if (__l2cap_get_chan_by_dcid(conn, scid))
+	if (__l2cap_get_chan_by_dcid(conn, scid)) {
+		result = L2CAP_CR_BREDR_SCID_IN_USE;
+		chan = NULL;
 		goto response;
+	}
 
 	chan = pchan->ops->new_connection(pchan);
 	if (!chan)
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] Bluetooth : Errata Service Release 8 Erratum 3253
  2018-09-19  9:51 [PATCH] Bluetooth : Errata Service Release 8 " Mallikarjun Phulari
@ 2018-09-19 12:06 ` Luiz Augusto von Dentz
  2018-09-21  4:37   ` Phulari, Mallikarjun
  0 siblings, 1 reply; 9+ messages in thread
From: Luiz Augusto von Dentz @ 2018-09-19 12:06 UTC (permalink / raw)
  To: Mallikarjun Phulari; +Cc: linux-bluetooth

Hi Mallikarjun,

On Wed, Sep 19, 2018 at 12:51 PM, Mallikarjun Phulari
<mallikarjun.phulari@intel.com> wrote:
> L2CAP: Changes include the new result codes for the l2cap channel
> create/connect request. The new result code are:
> 0x0006 - sent in the response when the CID is not in valid dynamic range.
> 0x0007 sent in the response when the CID is already allocated.

It would be a good idea to quote the erratum with page number where
the new errors come from. Btw, you should include a signed of by in
the kernel patches.

> ---
>  include/net/bluetooth/l2cap.h |  6 ++++++
>  net/bluetooth/l2cap_core.c    | 12 ++++++++++++
>  2 files changed, 18 insertions(+)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 0697fd4..eb0c8d0 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -284,6 +284,12 @@ struct l2cap_conn_rsp {
>  #define L2CAP_CR_INVALID_SCID  0x0009
>  #define L2CAP_CR_SCID_IN_USE   0x000A
>
> +/* connect/create channel results
> + * As per Erratum 3253
> + */
> +#define L2CAP_CR_BREDR_INVALID_SCID    0x0006
> +#define L2CAP_CR_BREDR_SCID_IN_USE     0x0007

Weird, are L2CAP_CR_INVALID_SCID and L2CAP_CR_SCID_IN_USE specific to
LE? If they are we should probably fix their names.

>  /* connect/create channel status */
>  #define L2CAP_CS_NO_INFO       0x0000
>  #define L2CAP_CS_AUTHEN_PEND   0x0001
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 9b7907e..85887df 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -3814,9 +3814,21 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
>         }
>
>         result = L2CAP_CR_NO_MEM;
> +       /* As per Erratum 3253, check the CID is in valid dynamic range and
> +        *  is not allocated already. Send the new result codes accordingly
> +        */
> +
> +       /* Check for valid dynamic CID range */
> +       if (scid < L2CAP_CID_DYN_START || scid > L2CAP_CID_DYN_END) {
> +               result = L2CAP_CR_BREDR_INVALID_SCID;
> +               chan = NULL;
> +               goto response;
> +       }
>
>         /* Check if we already have channel with that dcid */
>         if (__l2cap_get_chan_by_dcid(conn, scid))
> +               result = L2CAP_CR_BREDR_SCID_IN_USE;
> +               chan = NULL;
>                 goto response;

Missing { } above?

>
>         chan = pchan->ops->new_connection(pchan);
> --
> 2.7.4
>



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] Bluetooth : Errata Service Release 8 Erratum 3253
@ 2018-09-19  9:51 Mallikarjun Phulari
  2018-09-19 12:06 ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 9+ messages in thread
From: Mallikarjun Phulari @ 2018-09-19  9:51 UTC (permalink / raw)
  To: linux-bluetooth

L2CAP: Changes include the new result codes for the l2cap channel
create/connect request. The new result code are:
0x0006 - sent in the response when the CID is not in valid dynamic range.
0x0007 sent in the response when the CID is already allocated.
---
 include/net/bluetooth/l2cap.h |  6 ++++++
 net/bluetooth/l2cap_core.c    | 12 ++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 0697fd4..eb0c8d0 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -284,6 +284,12 @@ struct l2cap_conn_rsp {
 #define L2CAP_CR_INVALID_SCID	0x0009
 #define L2CAP_CR_SCID_IN_USE	0x000A
 
+/* connect/create channel results
+ * As per Erratum 3253
+ */
+#define L2CAP_CR_BREDR_INVALID_SCID	0x0006
+#define L2CAP_CR_BREDR_SCID_IN_USE	0x0007
+
 /* connect/create channel status */
 #define L2CAP_CS_NO_INFO	0x0000
 #define L2CAP_CS_AUTHEN_PEND	0x0001
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 9b7907e..85887df 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3814,9 +3814,21 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
 	}
 
 	result = L2CAP_CR_NO_MEM;
+	/* As per Erratum 3253, check the CID is in valid dynamic range and
+	 *  is not allocated already. Send the new result codes accordingly
+	 */
+
+	/* Check for valid dynamic CID range */
+	if (scid < L2CAP_CID_DYN_START || scid > L2CAP_CID_DYN_END) {
+		result = L2CAP_CR_BREDR_INVALID_SCID;
+		chan = NULL;
+		goto response;
+	}
 
 	/* Check if we already have channel with that dcid */
 	if (__l2cap_get_chan_by_dcid(conn, scid))
+		result = L2CAP_CR_BREDR_SCID_IN_USE;
+		chan = NULL;
 		goto response;
 
 	chan = pchan->ops->new_connection(pchan);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-10-03 12:01 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-24  9:16 [PATCH] Bluetooth : Errata Service Release 8, Erratum 3253 Mallikarjun Phulari
2018-09-27 10:37 ` Marcel Holtmann
2018-10-01  9:26   ` Phulari, Mallikarjun
2018-10-03  6:56     ` Marcel Holtmann
2018-10-03 11:59       ` Phulari, Mallikarjun
  -- strict thread matches above, loose matches on Subject: below --
2018-09-19  9:51 [PATCH] Bluetooth : Errata Service Release 8 " Mallikarjun Phulari
2018-09-19 12:06 ` Luiz Augusto von Dentz
2018-09-21  4:37   ` Phulari, Mallikarjun
2018-09-21 11:44     ` Luiz Augusto von Dentz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).