On Friday 05 June 2020 00:11:54 Luiz Augusto von Dentz wrote:
> Hi Pali,
>
> On Thu, Jun 4, 2020 at 4:18 PM Pali Rohár <pali@xxxxxxxxxx> wrote:
> >
> > When bluetoothd daemon is starting, it prints following error messages:
> >
> > bluetoothd[19117]: src/profile.c:ext_start_servers() L2CAP server failed for Message Notification: setsockopt(L2CAP_OPTIONS): Invalid argument (22)
> > bluetoothd[19117]: src/profile.c:ext_start_servers() L2CAP server failed for Message Access: setsockopt(L2CAP_OPTIONS): Invalid argument (22)
> > bluetoothd[19117]: src/profile.c:ext_start_servers() L2CAP server failed for Phone Book Access: setsockopt(L2CAP_OPTIONS): Invalid argument (22)
> > bluetoothd[19117]: src/profile.c:ext_start_servers() L2CAP server failed for File Transfer: setsockopt(L2CAP_OPTIONS): Invalid argument (22)
> > bluetoothd[19117]: src/profile.c:ext_start_servers() L2CAP server failed for Object Push: setsockopt(L2CAP_OPTIONS): Invalid argument (22)
> >
> > Result is that Object Push and File Transfer profiles do not work at all.
> >
> > Debugging set_l2opts() function showed me that src/profile.c calls it with
> > L2CAP mode 0x01 (L2CAP_MODE_RETRANS). But kernel bluetooth code in function
> > l2cap_sock_setsockopt_old() for L2CAP_OPTIONS option disallows to set L2CAP
> > method to 0x01 (L2CAP_MODE_RETRANS) and just returns -EINVAL (22).
> >
> > These bluetooth profiles have in src/profile.c file defined L2CAP mode to
> > BT_IO_MODE_ERTM and not to RETRANS. So it means that BT_IO_MODE_ERTM value
> > defined in 'enum BtIOMode' must be incorrect.
> >
> > Digging into git history, it appears that 'enum BtIOMode' was broken in
> > commit f2418bf97d ("btio: Add mode to for Enhanced Credit Mode") which
> > basically broke all those Object Push, File Transfer, Phone Book Access,
> > Message Access and Message Notification L2CAP profiles. That commit removed
> > some values from 'enum BtIOMode' and therefore broke all bluetooth code
> > which uses 'enum BtIOMode' for communication with kernel.
> >
> > This patch fixes 'enum BtIOMode' by reverting back BT_IO_MODE_RETRANS and
> > BT_IO_MODE_FLOWCTL modes, so BT_IO_MODE_ERTM has again correct value 0x03.
> >
> > After applying this patch, Object Push and File Transfer profiles work
> > again.
> >
> > Fixes: f2418bf97d ("btio: Add mode to for Enhanced Credit Mode")
> > ---
> >
> > Hello Luiz, could you please review this patch? As that problematic commit
> > f2418bf97d was introduced by you.
> >
> > I'm curious why nobody else reported this issue about broken Object Push
> > and File Transfer profile as it should print those error messages... Or
> > maybe error message is visible only in debug build and nobody is using
> > Bluetooth File Transfer anymore?
>
> Looks like you are right but, see bellow.
>
> > ---
> > btio/btio.h | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/btio/btio.h b/btio/btio.h
> > index 23e0ef72b..0d183e3ce 100644
> > --- a/btio/btio.h
> > +++ b/btio/btio.h
> > @@ -68,6 +68,8 @@ typedef enum {
> >
> > typedef enum {
> > BT_IO_MODE_BASIC = 0,
> > + BT_IO_MODE_RETRANS,
> > + BT_IO_MODE_FLOWCTL,
> > BT_IO_MODE_ERTM,
> > BT_IO_MODE_STREAMING,
> > BT_IO_MODE_LE_FLOWCTL,
> > --
> > 2.20.1
>
> These modes were never supported by the kernel thus why Ive dropped
> them, so might want to translate them to the old L2CAP modes when
> set_l2opts is called.
Hello Luiz! In commit 81629d982c672e4b3288c4499f9912f60f7040e9 ("btio:
Fix not translation mode to L2CAP mode") you have introduced following
code:
+ l2o.mode = mode_l2mode(mode);
+ if (l2o.mode == UINT8_MAX) {
+ ERROR_FAILED(err, "Unsupported mode", errno);
+ return FALSE;
+ }
But function mode_l2mode() does not set errno when it fails. Therefore
it reports bogus error number (probably zero).
So probably you want to use EINVAL (or some other constant):
+ ERROR_FAILED(err, "Unsupported mode", EINVAL);
