2020-06-05 18:03:12

by Luiz Augusto von Dentz

Subject: [PATCH BlueZ] btio: Fix not translation mode to L2CAP mode

From: Luiz Augusto von Dentz <[email protected]>

When using L2CAP_OPTIONS legacy modes need to be used since they are
not compatible with BT_MODE.
---
btio/btio.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/btio/btio.c b/btio/btio.c
index 13c731062..844d6007f 100644
--- a/btio/btio.c
+++ b/btio/btio.c
@@ -597,6 +597,20 @@ static gboolean get_key_size(int sock, int *size, GError **err)
return FALSE;
}

+static uint8_t mode_l2mode(uint8_t mode)
+{
+ switch (mode) {
+ case BT_IO_MODE_BASIC:
+ return L2CAP_MODE_BASIC;
+ case BT_IO_MODE_ERTM:
+ return L2CAP_MODE_ERTM;
+ case BT_IO_MODE_STREAMING:
+ return L2CAP_MODE_STREAMING;
+ default:
+ return UINT8_MAX;
+ }
+}
+
static gboolean set_l2opts(int sock, uint16_t imtu, uint16_t omtu,
uint8_t mode, GError **err)
{
@@ -614,8 +628,14 @@ static gboolean set_l2opts(int sock, uint16_t imtu, uint16_t omtu,
l2o.imtu = imtu;
if (omtu)
l2o.omtu = omtu;
- if (mode)
- l2o.mode = mode;
+
+ if (mode) {
+ l2o.mode = mode_l2mode(mode);
+ if (l2o.mode == UINT8_MAX) {
+ ERROR_FAILED(err, "Unsupported mode", errno);
+ return FALSE;
+ }
+ }

if (setsockopt(sock, SOL_L2CAP, L2CAP_OPTIONS, &l2o, sizeof(l2o)) < 0) {
ERROR_FAILED(err, "setsockopt(L2CAP_OPTIONS)", errno);
--
2.25.3

2020-06-05 23:27:26

by Luiz Augusto von Dentz

Subject: Re: [PATCH BlueZ] btio: Fix not translation mode to L2CAP mode

Hi,

On Fri, Jun 5, 2020 at 10:59 AM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> From: Luiz Augusto von Dentz <[email protected]>
>
> When using L2CAP_OPTIONS legacy modes need to be used since they are
> not compatible with BT_MODE.
> ---
> btio/btio.c | 24 ++++++++++++++++++++++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/btio/btio.c b/btio/btio.c
> index 13c731062..844d6007f 100644
> --- a/btio/btio.c
> +++ b/btio/btio.c
> @@ -597,6 +597,20 @@ static gboolean get_key_size(int sock, int *size, GError **err)
> return FALSE;
> }
>
> +static uint8_t mode_l2mode(uint8_t mode)
> +{
> + switch (mode) {
> + case BT_IO_MODE_BASIC:
> + return L2CAP_MODE_BASIC;
> + case BT_IO_MODE_ERTM:
> + return L2CAP_MODE_ERTM;
> + case BT_IO_MODE_STREAMING:
> + return L2CAP_MODE_STREAMING;
> + default:
> + return UINT8_MAX;
> + }
> +}
> +
> static gboolean set_l2opts(int sock, uint16_t imtu, uint16_t omtu,
> uint8_t mode, GError **err)
> {
> @@ -614,8 +628,14 @@ static gboolean set_l2opts(int sock, uint16_t imtu, uint16_t omtu,
> l2o.imtu = imtu;
> if (omtu)
> l2o.omtu = omtu;
> - if (mode)
> - l2o.mode = mode;
> +
> + if (mode) {
> + l2o.mode = mode_l2mode(mode);
> + if (l2o.mode == UINT8_MAX) {
> + ERROR_FAILED(err, "Unsupported mode", errno);
> + return FALSE;
> + }
> + }
>
> if (setsockopt(sock, SOL_L2CAP, L2CAP_OPTIONS, &l2o, sizeof(l2o)) < 0) {
> ERROR_FAILED(err, "setsockopt(L2CAP_OPTIONS)", errno);
> --
> 2.25.3

Pushed.

--
Luiz Augusto von Dentz

2020-06-08 13:41:36

by Marijn Suijten

Subject: Re: [PATCH BlueZ] btio: Fix not translation mode to L2CAP mode

Hi Luiz,

> Hi,
>
> On Fri, Jun 5, 2020 at 10:59 AM Luiz Augusto von Dentz
> <[email protected]> wrote:
> >
> > From: Luiz Augusto von Dentz <[email protected]>
> >
> > When using L2CAP_OPTIONS legacy modes need to be used since they are
> > not compatible with BT_MODE.
> > ---
> > btio/btio.c | 24 ++++++++++++++++++++++--
> > 1 file changed, 22 insertions(+), 2 deletions(-)
> >
> > diff --git a/btio/btio.c b/btio/btio.c
> > index 13c731062..844d6007f 100644
> > --- a/btio/btio.c
> > +++ b/btio/btio.c
> > @@ -597,6 +597,20 @@ static gboolean get_key_size(int sock, int *size, GError **err)
> > return FALSE;
> > }
> >
> > +static uint8_t mode_l2mode(uint8_t mode)
> > +{
> > + switch (mode) {
> > + case BT_IO_MODE_BASIC:
> > + return L2CAP_MODE_BASIC;
> > + case BT_IO_MODE_ERTM:
> > + return L2CAP_MODE_ERTM;
> > + case BT_IO_MODE_STREAMING:
> > + return L2CAP_MODE_STREAMING;
> > + default:
> > + return UINT8_MAX;
> > + }
> > +}
> > +
> > static gboolean set_l2opts(int sock, uint16_t imtu, uint16_t omtu,
> > uint8_t mode, GError **err)
> > {
> > @@ -614,8 +628,14 @@ static gboolean set_l2opts(int sock, uint16_t imtu, uint16_t omtu,
> > l2o.imtu = imtu;
> > if (omtu)
> > l2o.omtu = omtu;
> > - if (mode)
> > - l2o.mode = mode;
> > +
> > + if (mode) {
> > + l2o.mode = mode_l2mode(mode);
> > + if (l2o.mode == UINT8_MAX) {
> > + ERROR_FAILED(err, "Unsupported mode", errno);
> > + return FALSE;
> > + }
> > + }
> >
> > if (setsockopt(sock, SOL_L2CAP, L2CAP_OPTIONS, &l2o, sizeof(l2o)) < 0) {
> > ERROR_FAILED(err, "setsockopt(L2CAP_OPTIONS)", errno);
> > --
> > 2.25.3
>
> Pushed.
>
> --
> Luiz Augusto von Dentz

This patch seems to break avctp browsing. The creation of browsing_io in
avctp_register already uses L2CAP_MODE_ERTM which is not in the
switch-case hence results in "Unsupported mode". What is the desired way
to fix this? Should all those calls use BT_IO_MODE_* instead? Not to
mention the uses of these functions should be checked for their enum
usage as well.

- Marijn Suijten

2020-06-08 15:56:24

by Luiz Augusto von Dentz

Subject: Re: [PATCH BlueZ] btio: Fix not translation mode to L2CAP mode

Hi Marijn,

On Mon, Jun 8, 2020 at 6:40 AM Marijn Suijten <[email protected]> wrote:
>
> Hi Luiz,
>
> > Hi,
> >
> > On Fri, Jun 5, 2020 at 10:59 AM Luiz Augusto von Dentz
> > <[email protected]> wrote:
> > >
> > > From: Luiz Augusto von Dentz <[email protected]>
> > >
> > > When using L2CAP_OPTIONS legacy modes need to be used since they are
> > > not compatible with BT_MODE.
> > > ---
> > > btio/btio.c | 24 ++++++++++++++++++++++--
> > > 1 file changed, 22 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/btio/btio.c b/btio/btio.c
> > > index 13c731062..844d6007f 100644
> > > --- a/btio/btio.c
> > > +++ b/btio/btio.c
> > > @@ -597,6 +597,20 @@ static gboolean get_key_size(int sock, int *size, GError **err)
> > > return FALSE;
> > > }
> > >
> > > +static uint8_t mode_l2mode(uint8_t mode)
> > > +{
> > > + switch (mode) {
> > > + case BT_IO_MODE_BASIC:
> > > + return L2CAP_MODE_BASIC;
> > > + case BT_IO_MODE_ERTM:
> > > + return L2CAP_MODE_ERTM;
> > > + case BT_IO_MODE_STREAMING:
> > > + return L2CAP_MODE_STREAMING;
> > > + default:
> > > + return UINT8_MAX;
> > > + }
> > > +}
> > > +
> > > static gboolean set_l2opts(int sock, uint16_t imtu, uint16_t omtu,
> > > uint8_t mode, GError **err)
> > > {
> > > @@ -614,8 +628,14 @@ static gboolean set_l2opts(int sock, uint16_t imtu, uint16_t omtu,
> > > l2o.imtu = imtu;
> > > if (omtu)
> > > l2o.omtu = omtu;
> > > - if (mode)
> > > - l2o.mode = mode;
> > > +
> > > + if (mode) {
> > > + l2o.mode = mode_l2mode(mode);
> > > + if (l2o.mode == UINT8_MAX) {
> > > + ERROR_FAILED(err, "Unsupported mode", errno);
> > > + return FALSE;
> > > + }
> > > + }
> > >
> > > if (setsockopt(sock, SOL_L2CAP, L2CAP_OPTIONS, &l2o, sizeof(l2o)) < 0) {
> > > ERROR_FAILED(err, "setsockopt(L2CAP_OPTIONS)", errno);
> > > --
> > > 2.25.3
> >
> > Pushed.
> >
> > --
> > Luiz Augusto von Dentz
>
> This patch seems to break avctp browsing. The creation of browsing_io in
> avctp_register already uses L2CAP_MODE_ERTM which is not in the
> switch-case hence results in "Unsupported mode". What is the desired way
> to fix this? Should all those calls use BT_IO_MODE_* instead? Not to
> mention the uses of these functions should be checked for their enum
> usage as well.

I will fix it, bt_io API shall only operate with the its mode not
L2CAP mode so it is a bug to pass them.

--
Luiz Augusto von Dentz