2019-12-03 01:05:23

by Abhishek Pandit-Subedi

[permalink] [raw]
Subject: [BlueZ PATCH v2 1/4] lib: Add ba2strlc to match kernel printk format

When the kernel prints the bluetooth address (via %pMR), it prints the
address in lower case. ba2strlc should be used in cases where we should
match the kernel casing (i.e. addresses assigned to /dev/uhid and
/dev/uinput)

---

Changes in v2:
- Split into its own commit

lib/bluetooth.c | 7 +++++++
lib/bluetooth.h | 1 +
2 files changed, 8 insertions(+)

diff --git a/lib/bluetooth.c b/lib/bluetooth.c
index effc7f49d..7cba509d8 100644
--- a/lib/bluetooth.c
+++ b/lib/bluetooth.c
@@ -81,6 +81,13 @@ int ba2str(const bdaddr_t *ba, char *str)
ba->b[5], ba->b[4], ba->b[3], ba->b[2], ba->b[1], ba->b[0]);
}

+/* Match kernel's lowercase printing of mac address (%pMR) */
+int ba2strlc(const bdaddr_t *ba, char *str)
+{
+ return sprintf(str, "%2.2x:%2.2x:%2.2x:%2.2x:%2.2x:%2.2x",
+ ba->b[5], ba->b[4], ba->b[3], ba->b[2], ba->b[1], ba->b[0]);
+}
+
int str2ba(const char *str, bdaddr_t *ba)
{
int i;
diff --git a/lib/bluetooth.h b/lib/bluetooth.h
index eb279260e..756dce164 100644
--- a/lib/bluetooth.h
+++ b/lib/bluetooth.h
@@ -325,6 +325,7 @@ void baswap(bdaddr_t *dst, const bdaddr_t *src);
bdaddr_t *strtoba(const char *str);
char *batostr(const bdaddr_t *ba);
int ba2str(const bdaddr_t *ba, char *str);
+int ba2strlc(const bdaddr_t *ba, char *str);
int str2ba(const char *str, bdaddr_t *ba);
int ba2oui(const bdaddr_t *ba, char *oui);
int bachk(const char *str);
--
2.24.0.393.g34dc348eaf-goog


2019-12-03 01:05:23

by Abhishek Pandit-Subedi

[permalink] [raw]
Subject: [BlueZ PATCH v2 2/4] input: Update virtual input devices with correct info

Update uhid and uinput devices with lowercase addresses (to match how
kernel prints it via %pMR). Also update uinput to include the phys
attribute and correctly set the vendor/product/version during init.

---

Changes in v2:
- Split into two commits

profiles/audio/avctp.c | 21 ++++++++++++++-------
profiles/input/device.c | 4 ++--
profiles/input/hog-lib.c | 10 ++++++++++
3 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index 70a3e40b2..42136f94b 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -1161,7 +1161,8 @@ failed:
return FALSE;
}

-static int uinput_create(char *name)
+static int uinput_create(struct btd_device *device, const char *name,
+ const char *phys)
{
struct uinput_dev dev;
int fd, err, i;
@@ -1185,9 +1186,9 @@ static int uinput_create(char *name)
strncpy(dev.name, name, UINPUT_MAX_NAME_SIZE - 1);

dev.id.bustype = BUS_BLUETOOTH;
- dev.id.vendor = 0x0000;
- dev.id.product = 0x0000;
- dev.id.version = 0x0000;
+ dev.id.vendor = btd_device_get_vendor(device);
+ dev.id.product = btd_device_get_product(device);
+ dev.id.version = btd_device_get_version(device);

if (write(fd, &dev, sizeof(dev)) < 0) {
err = -errno;
@@ -1202,6 +1203,9 @@ static int uinput_create(char *name)
ioctl(fd, UI_SET_EVBIT, EV_REP);
ioctl(fd, UI_SET_EVBIT, EV_SYN);

+ /* Also set the phys */
+ ioctl(fd, UI_SET_PHYS, phys);
+
for (i = 0; key_map[i].name != NULL; i++)
ioctl(fd, UI_SET_KEYBIT, key_map[i].uinput);

@@ -1220,7 +1224,7 @@ static int uinput_create(char *name)

static void init_uinput(struct avctp *session)
{
- char address[18], name[248 + 1];
+ char address[18], phys[18], name[248 + 1];

device_get_name(session->device, name, sizeof(name));
if (g_str_equal(name, "Nokia CK-20W")) {
@@ -1230,8 +1234,11 @@ static void init_uinput(struct avctp *session)
session->key_quirks[AVC_PAUSE] |= QUIRK_NO_RELEASE;
}

- ba2str(device_get_address(session->device), address);
- session->uinput = uinput_create(address);
+ ba2strlc(device_get_address(session->device), address);
+ ba2strlc(btd_adapter_get_address(device_get_adapter(session->device)),
+ phys);
+
+ session->uinput = uinput_create(session->device, address, phys);
if (session->uinput < 0)
error("AVRCP: failed to init uinput for %s", address);
else
diff --git a/profiles/input/device.c b/profiles/input/device.c
index a711ef527..2cb3811c8 100644
--- a/profiles/input/device.c
+++ b/profiles/input/device.c
@@ -855,8 +855,8 @@ static int uhid_connadd(struct input_device *idev, struct hidp_connadd_req *req)
memset(&ev, 0, sizeof(ev));
ev.type = UHID_CREATE;
strncpy((char *) ev.u.create.name, req->name, sizeof(ev.u.create.name));
- ba2str(&idev->src, (char *) ev.u.create.phys);
- ba2str(&idev->dst, (char *) ev.u.create.uniq);
+ ba2strlc(&idev->src, (char *) ev.u.create.phys);
+ ba2strlc(&idev->dst, (char *) ev.u.create.uniq);
ev.u.create.vendor = req->vendor;
ev.u.create.product = req->product;
ev.u.create.version = req->version;
diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c
index d9ed80689..9c5c814a7 100644
--- a/profiles/input/hog-lib.c
+++ b/profiles/input/hog-lib.c
@@ -32,6 +32,7 @@
#include <stdbool.h>
#include <errno.h>
#include <unistd.h>
+#include <ctype.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
@@ -992,6 +993,15 @@ static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
BT_IO_OPT_SOURCE, ev.u.create.phys,
BT_IO_OPT_DEST, ev.u.create.uniq,
BT_IO_OPT_INVALID);
+
+ /* Phys + uniq are the same size (hw address type) */
+ for (i = 0;
+ i < (int)sizeof(ev.u.create.phys) && ev.u.create.phys[i] != 0;
+ ++i) {
+ ev.u.create.phys[i] = tolower(ev.u.create.phys[i]);
+ ev.u.create.uniq[i] = tolower(ev.u.create.uniq[i]);
+ }
+
if (gerr) {
error("Failed to connection details: %s", gerr->message);
g_error_free(gerr);
--
2.24.0.393.g34dc348eaf-goog

2019-12-03 01:05:54

by Abhishek Pandit-Subedi

[permalink] [raw]
Subject: [BlueZ PATCH v2 3/4] input: Change uinput name and add AVRCP suffix

When creating the uinput device, change the name to the peer device
name and add a "(AVRCP)" suffix.

The resulting uinput device will look like this:

$ udevadm info -a -p /sys/devices/virtual/input/input17
...
looking at device '/devices/virtual/input/input17':
KERNEL=="input17"
SUBSYSTEM=="input"
DRIVER==""
ATTR{inhibited}=="0"
ATTR{name}=="BeatsStudio Wireless (AVRCP)"
ATTR{phys}=="00:00:00:6e:d0:74"
ATTR{properties}=="0"
ATTR{uniq}==""

---

Changes in v2:
- Added "(AVRCP)" suffix and moved src address into uinput_create

profiles/audio/avctp.c | 36 +++++++++++++++++++++++++-----------
1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index 42136f94b..5116a5cde 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -1162,10 +1162,11 @@ failed:
}

static int uinput_create(struct btd_device *device, const char *name,
- const char *phys)
+ const char *suffix);
{
struct uinput_dev dev;
int fd, err, i;
+ char src[18];

fd = open("/dev/uinput", O_RDWR);
if (fd < 0) {
@@ -1185,6 +1186,23 @@ static int uinput_create(struct btd_device *device, const char *name,
if (name)
strncpy(dev.name, name, UINPUT_MAX_NAME_SIZE - 1);

+ if (suffix) {
+ int len, slen;
+
+ len = strlen(dev.name);
+ slen = strlen(suffix);
+
+ /* If name + suffix don't fit, truncate the name, then add the
+ * suffix.
+ */
+ if (len + slen < UINPUT_MAX_NAME_SIZE - 1) {
+ strncpy(dev.name + len, suffix, slen);
+ } else {
+ len = UINPUT_MAX_NAME_SIZE - slen - 1;
+ strncpy(dev.name + len, suffix, slen);
+ }
+ }
+
dev.id.bustype = BUS_BLUETOOTH;
dev.id.vendor = btd_device_get_vendor(device);
dev.id.product = btd_device_get_product(device);
@@ -1203,8 +1221,8 @@ static int uinput_create(struct btd_device *device, const char *name,
ioctl(fd, UI_SET_EVBIT, EV_REP);
ioctl(fd, UI_SET_EVBIT, EV_SYN);

- /* Also set the phys */
- ioctl(fd, UI_SET_PHYS, phys);
+ ba2strlc(btd_adapter_get_address(device_get_adapter(device)), src);
+ ioctl(fd, UI_SET_PHYS, src);

for (i = 0; key_map[i].name != NULL; i++)
ioctl(fd, UI_SET_KEYBIT, key_map[i].uinput);
@@ -1224,7 +1242,7 @@ static int uinput_create(struct btd_device *device, const char *name,

static void init_uinput(struct avctp *session)
{
- char address[18], phys[18], name[248 + 1];
+ char name[248 + 1];

device_get_name(session->device, name, sizeof(name));
if (g_str_equal(name, "Nokia CK-20W")) {
@@ -1234,15 +1252,11 @@ static void init_uinput(struct avctp *session)
session->key_quirks[AVC_PAUSE] |= QUIRK_NO_RELEASE;
}

- ba2strlc(device_get_address(session->device), address);
- ba2strlc(btd_adapter_get_address(device_get_adapter(session->device)),
- phys);
-
- session->uinput = uinput_create(session->device, address, phys);
+ session->uinput = uinput_create(session->device, name, " (AVRCP)");
if (session->uinput < 0)
- error("AVRCP: failed to init uinput for %s", address);
+ error("AVRCP: failed to init uinput for %s", name);
else
- DBG("AVRCP: uinput initialized for %s", address);
+ DBG("AVRCP: uinput initialized for %s", name);
}

static struct avctp_queue *avctp_queue_create(struct avctp_channel *chan)
--
2.24.0.393.g34dc348eaf-goog

2019-12-03 01:07:16

by Abhishek Pandit-Subedi

[permalink] [raw]
Subject: [BlueZ PATCH v2 4/4] input: Set uniq attribute

Set the uniq attribute of /dev/uinput with the peer device address.

The resulting uinput device will look like this:

$ udevadm info -a -p /sys/devices/virtual/input/input17
...
looking at device '/devices/virtual/input/input17':
KERNEL=="input17"
SUBSYSTEM=="input"
DRIVER==""
ATTR{inhibited}=="0"
ATTR{name}=="BeatsStudio Wireless (AVRCP)"
ATTR{phys}=="00:00:00:6e:d0:74"
ATTR{properties}=="0"
ATTR{uniq}=="00:00:00:1a:33:21"

---

This change requires an accompanying change in the kernel that adds the
set uniq ioctl. The change is available here:

https://lore.kernel.org/linux-bluetooth/[email protected]/T/#u


Changes in v2:
- Split setting uniq to its own commit

profiles/audio/avctp.c | 4 +++-
src/uinput.h | 2 ++
2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index 5116a5cde..544bc640c 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -1166,7 +1166,7 @@ static int uinput_create(struct btd_device *device, const char *name,
{
struct uinput_dev dev;
int fd, err, i;
- char src[18];
+ char dest[18], src[18];

fd = open("/dev/uinput", O_RDWR);
if (fd < 0) {
@@ -1222,7 +1222,9 @@ static int uinput_create(struct btd_device *device, const char *name,
ioctl(fd, UI_SET_EVBIT, EV_SYN);

ba2strlc(btd_adapter_get_address(device_get_adapter(device)), src);
+ ba2strlc(device_get_address(device), dest);
ioctl(fd, UI_SET_PHYS, src);
+ ioctl(fd, UI_SET_UNIQ, dest);

for (i = 0; key_map[i].name != NULL; i++)
ioctl(fd, UI_SET_KEYBIT, key_map[i].uinput);
diff --git a/src/uinput.h b/src/uinput.h
index 20e0941d1..589c22528 100644
--- a/src/uinput.h
+++ b/src/uinput.h
@@ -686,6 +686,8 @@ extern "C" {
#define UI_SET_FFBIT _IOW(UINPUT_IOCTL_BASE, 107, int)
#define UI_SET_PHYS _IOW(UINPUT_IOCTL_BASE, 108, char*)
#define UI_SET_SWBIT _IOW(UINPUT_IOCTL_BASE, 109, int)
+#define UI_SET_PROPBIT _IOW(UINPUT_IOCTL_BASE, 110, int)
+#define UI_SET_UNIQ _IOW(UINPUT_IOCTL_BASE, 111, char*)

#ifndef NBITS
#define NBITS(x) ((((x) - 1) / (sizeof(long) * 8)) + 1)
--
2.24.0.393.g34dc348eaf-goog

2019-12-03 17:19:10

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [BlueZ PATCH v2 1/4] lib: Add ba2strlc to match kernel printk format

Hi Abhishek,

On Tue, Dec 3, 2019 at 3:04 AM Abhishek Pandit-Subedi
<[email protected]> wrote:
>
> When the kernel prints the bluetooth address (via %pMR), it prints the
> address in lower case. ba2strlc should be used in cases where we should
> match the kernel casing (i.e. addresses assigned to /dev/uhid and
> /dev/uinput)
>
> ---
>
> Changes in v2:
> - Split into its own commit
>
> lib/bluetooth.c | 7 +++++++
> lib/bluetooth.h | 1 +
> 2 files changed, 8 insertions(+)
>
> diff --git a/lib/bluetooth.c b/lib/bluetooth.c
> index effc7f49d..7cba509d8 100644
> --- a/lib/bluetooth.c
> +++ b/lib/bluetooth.c
> @@ -81,6 +81,13 @@ int ba2str(const bdaddr_t *ba, char *str)
> ba->b[5], ba->b[4], ba->b[3], ba->b[2], ba->b[1], ba->b[0]);
> }
>
> +/* Match kernel's lowercase printing of mac address (%pMR) */
> +int ba2strlc(const bdaddr_t *ba, char *str)
> +{
> + return sprintf(str, "%2.2x:%2.2x:%2.2x:%2.2x:%2.2x:%2.2x",
> + ba->b[5], ba->b[4], ba->b[3], ba->b[2], ba->b[1], ba->b[0]);
> +}
> +
> int str2ba(const char *str, bdaddr_t *ba)
> {
> int i;
> diff --git a/lib/bluetooth.h b/lib/bluetooth.h
> index eb279260e..756dce164 100644
> --- a/lib/bluetooth.h
> +++ b/lib/bluetooth.h
> @@ -325,6 +325,7 @@ void baswap(bdaddr_t *dst, const bdaddr_t *src);
> bdaddr_t *strtoba(const char *str);
> char *batostr(const bdaddr_t *ba);
> int ba2str(const bdaddr_t *ba, char *str);
> +int ba2strlc(const bdaddr_t *ba, char *str);
> int str2ba(const char *str, bdaddr_t *ba);
> int ba2oui(const bdaddr_t *ba, char *oui);
> int bachk(const char *str);
> --
> 2.24.0.393.g34dc348eaf-goog

Applied, thanks. Note that I did change a little bit the code in
uinput_create since strncpy generates some warnings with gcc8 see:

https://developers.redhat.com/blog/2018/05/24/detecting-string-truncation-with-gcc-8/

What I did was to assign '\0' after the use of strncpy otherwise it
would generate warnings, also replaced the use of strncpy when we know
it would not need to be truncated.

--
Luiz Augusto von Dentz