* [PATCH v2 2/5] doc/gatt-api: Restrict supported file descriptors
2018-11-20 10:39 [PATCH v2 1/5] gatt: Fix invalid read when disconnecting Luiz Augusto von Dentz
@ 2018-11-20 10:39 ` Luiz Augusto von Dentz
2018-11-20 10:39 ` [PATCH v2 3/5] gatt: Switch from pipe2 to sockepair for Acquire* Luiz Augusto von Dentz
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2018-11-20 10:39 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Only support sockets with AcquireWrite/AcquireNotify since pipe don't
work with sendmsg therefore MSG_NOSIGNAL cannot be used.
---
doc/gatt-api.txt | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
index be4c60c6f..8dda60b8a 100644
--- a/doc/gatt-api.txt
+++ b/doc/gatt-api.txt
@@ -100,9 +100,9 @@ Methods array{byte} ReadValue(dict options)
fd, uint16 AcquireWrite(dict options) [optional]
- Acquire file descriptor and MTU for writing. Usage of
- WriteValue will be locked causing it to return
- NotPermitted error.
+ Acquire file descriptor and MTU for writing. Only
+ sockets are supported. Usage of WriteValue will be
+ locked causing it to return NotPermitted error.
For server the MTU returned shall be equal or smaller
than the negotiated MTU.
@@ -130,9 +130,9 @@ Methods array{byte} ReadValue(dict options)
fd, uint16 AcquireNotify(dict options) [optional]
- Acquire file descriptor and MTU for notify. Usage of
- StartNotify will be locked causing it to return
- NotPermitted error.
+ Acquire file descriptor and MTU for notify. Only
+ sockets are support. Usage of StartNotify will be locked
+ causing it to return NotPermitted error.
For server the MTU returned shall be equal or smaller
than the negotiated MTU.
--
2.17.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 3/5] gatt: Switch from pipe2 to sockepair for Acquire*
2018-11-20 10:39 [PATCH v2 1/5] gatt: Fix invalid read when disconnecting Luiz Augusto von Dentz
2018-11-20 10:39 ` [PATCH v2 2/5] doc/gatt-api: Restrict supported file descriptors Luiz Augusto von Dentz
@ 2018-11-20 10:39 ` Luiz Augusto von Dentz
2018-11-20 10:39 ` [PATCH v2 4/5] client: Switch from write to sendmsg " Luiz Augusto von Dentz
2018-11-20 10:39 ` [PATCH v2 5/5] meshctl: " Luiz Augusto von Dentz
3 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2018-11-20 10:39 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This enables to use sendmsg with MSG_NOSIGNAL.
---
src/gatt-client.c | 99 ++++++++++++++++++++++++++-------------------
src/gatt-database.c | 26 +++++++-----
2 files changed, 72 insertions(+), 53 deletions(-)
diff --git a/src/gatt-client.c b/src/gatt-client.c
index 55aa5e423..45ed3b170 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -90,7 +90,7 @@ struct async_dbus_op {
async_dbus_op_complete_t complete;
};
-struct pipe_io {
+struct sock_io {
DBusMessage *msg;
struct io *io;
void (*destroy)(void *data);
@@ -109,8 +109,8 @@ struct characteristic {
char *path;
unsigned int ready_id;
- struct pipe_io *write_io;
- struct pipe_io *notify_io;
+ struct sock_io *write_io;
+ struct sock_io *notify_io;
struct async_dbus_op *read_op;
struct async_dbus_op *write_op;
@@ -1054,19 +1054,30 @@ fail:
return btd_error_not_supported(msg);
}
-static bool chrc_pipe_read(struct io *io, void *user_data)
+static bool sock_read(struct io *io, void *user_data)
{
struct characteristic *chrc = user_data;
struct bt_gatt_client *gatt = chrc->service->client->gatt;
+ struct msghdr msg;
uint8_t buf[512];
+ struct iovec iov;
int fd = io_get_fd(io);
ssize_t bytes_read;
- bytes_read = read(fd, buf, sizeof(buf));
- if (bytes_read < 0)
+ iov.iov_base = buf;
+ iov.iov_len = sizeof(buf);
+
+ memset(&msg, 0, sizeof(msg));
+ msg.msg_iov = &iov;
+ msg.msg_iovlen = 1;
+
+ bytes_read = recvmsg(fd, &msg, MSG_DONTWAIT);
+ if (bytes_read < 0) {
+ error("recvmsg: %s", strerror(errno));
return false;
+ }
- if (!gatt)
+ if (!gatt || bytes_read == 0)
return false;
bt_gatt_client_write_without_response(gatt, chrc->value_handle,
@@ -1076,7 +1087,7 @@ static bool chrc_pipe_read(struct io *io, void *user_data)
return true;
}
-static void pipe_io_destroy(struct pipe_io *io)
+static void sock_io_destroy(struct sock_io *io)
{
if (io->destroy)
io->destroy(io->data);
@@ -1088,20 +1099,19 @@ static void pipe_io_destroy(struct pipe_io *io)
free(io);
}
-static void characteristic_destroy_pipe(struct characteristic *chrc,
- struct io *io)
+static void destroy_sock(struct characteristic *chrc, struct io *io)
{
queue_remove(chrc->service->client->ios, io);
if (chrc->write_io && io == chrc->write_io->io) {
- pipe_io_destroy(chrc->write_io);
+ sock_io_destroy(chrc->write_io);
chrc->write_io = NULL;
g_dbus_emit_property_changed(btd_get_dbus_connection(),
chrc->path,
GATT_CHARACTERISTIC_IFACE,
"WriteAcquired");
} else if (chrc->notify_io) {
- pipe_io_destroy(chrc->notify_io);
+ sock_io_destroy(chrc->notify_io);
chrc->notify_io = NULL;
g_dbus_emit_property_changed(btd_get_dbus_connection(),
chrc->path,
@@ -1110,22 +1120,21 @@ static void characteristic_destroy_pipe(struct characteristic *chrc,
}
}
-static bool characteristic_pipe_hup(struct io *io, void *user_data)
+static bool sock_hup(struct io *io, void *user_data)
{
struct characteristic *chrc = user_data;
DBG("%s: io %p", chrc->path, io);
- characteristic_destroy_pipe(chrc, io);
+ destroy_sock(chrc, io);
return false;
}
-static DBusMessage *characteristic_create_pipe(struct characteristic *chrc,
- DBusMessage *msg)
+static DBusMessage *create_sock(struct characteristic *chrc, DBusMessage *msg)
{
struct bt_gatt_client *gatt = chrc->service->client->gatt;
- int pipefd[2];
+ int fds[2];
struct io *io;
bool dir;
uint16_t mtu;
@@ -1134,33 +1143,34 @@ static DBusMessage *characteristic_create_pipe(struct characteristic *chrc,
if (!gatt || !bt_gatt_client_is_ready(gatt))
return btd_error_failed(msg, "Not connected");
- if (pipe2(pipefd, O_DIRECT | O_NONBLOCK | O_CLOEXEC) < 0)
+ if (socketpair(AF_LOCAL, SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC,
+ 0, fds) < 0)
return btd_error_failed(msg, strerror(errno));
dir = dbus_message_has_member(msg, "AcquireWrite");
- io = io_new(pipefd[!dir]);
+ io = io_new(fds[!dir]);
if (!io) {
- close(pipefd[0]);
- close(pipefd[1]);
+ close(fds[0]);
+ close(fds[1]);
return btd_error_failed(msg, strerror(EIO));
}
io_set_close_on_destroy(io, true);
- if (!io_set_read_handler(io, chrc_pipe_read, chrc, NULL))
+ if (!io_set_read_handler(io, sock_read, chrc, NULL))
goto fail;
- if (!io_set_disconnect_handler(io, characteristic_pipe_hup, chrc, NULL))
+ if (!io_set_disconnect_handler(io, sock_hup, chrc, NULL))
goto fail;
mtu = bt_gatt_client_get_mtu(gatt);
- reply = g_dbus_create_reply(msg, DBUS_TYPE_UNIX_FD, &pipefd[dir],
+ reply = g_dbus_create_reply(msg, DBUS_TYPE_UNIX_FD, &fds[dir],
DBUS_TYPE_UINT16, &mtu,
DBUS_TYPE_INVALID);
- close(pipefd[dir]);
+ close(fds[dir]);
if (dir) {
chrc->write_io->io = io;
@@ -1185,7 +1195,7 @@ static DBusMessage *characteristic_create_pipe(struct characteristic *chrc,
fail:
io_destroy(io);
- close(pipefd[dir]);
+ close(fds[dir]);
return btd_error_failed(msg, strerror(EIO));
}
@@ -1197,7 +1207,7 @@ static void characteristic_ready(bool success, uint8_t ecode, void *user_data)
chrc->ready_id = 0;
if (chrc->write_io && chrc->write_io->msg) {
- reply = characteristic_create_pipe(chrc, chrc->write_io->msg);
+ reply = create_sock(chrc, chrc->write_io->msg);
g_dbus_send_message(btd_get_dbus_connection(), reply);
@@ -1206,7 +1216,7 @@ static void characteristic_ready(bool success, uint8_t ecode, void *user_data)
}
if (chrc->notify_io && chrc->notify_io->msg) {
- reply = characteristic_create_pipe(chrc, chrc->notify_io->msg);
+ reply = create_sock(chrc, chrc->notify_io->msg);
g_dbus_send_message(btd_get_dbus_connection(), reply);
@@ -1230,7 +1240,7 @@ static DBusMessage *characteristic_acquire_write(DBusConnection *conn,
if (!(chrc->props & BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP))
return btd_error_not_supported(msg);
- chrc->write_io = new0(struct pipe_io, 1);
+ chrc->write_io = new0(struct sock_io, 1);
if (!bt_gatt_client_is_ready(gatt)) {
/* GATT not ready, wait until it becomes ready */
@@ -1242,7 +1252,7 @@ static DBusMessage *characteristic_acquire_write(DBusConnection *conn,
return NULL;
}
- return characteristic_create_pipe(chrc, msg);
+ return create_sock(chrc, msg);
}
struct notify_client {
@@ -1414,21 +1424,26 @@ static void register_notify_cb(uint16_t att_ecode, void *user_data)
static void notify_io_cb(uint16_t value_handle, const uint8_t *value,
uint16_t length, void *user_data)
{
+ struct msghdr msg;
struct iovec iov;
struct notify_client *client = user_data;
struct characteristic *chrc = client->chrc;
int err;
- /* Drop notification if the pipe is not ready */
- if (!chrc->notify_io->io)
+ /* Drop notification if the sock is not ready */
+ if (!chrc->notify_io || !chrc->notify_io->io)
return;
iov.iov_base = (void *) value;
iov.iov_len = length;
- err = io_send(chrc->notify_io->io, &iov, 1);
+ memset(&msg, 0, sizeof(msg));
+ msg.msg_iov = &iov;
+ msg.msg_iovlen = 1;
+
+ err = sendmsg(io_get_fd(chrc->notify_io->io), &msg, MSG_NOSIGNAL);
if (err < 0)
- error("io_send: %s", strerror(-err));
+ error("sendmsg: %s", strerror(errno));
}
static void register_notify_io_cb(uint16_t att_ecode, void *user_data)
@@ -1499,7 +1514,7 @@ static DBusMessage *characteristic_acquire_notify(DBusConnection *conn,
queue_push_tail(chrc->notify_clients, client);
- chrc->notify_io = new0(struct pipe_io, 1);
+ chrc->notify_io = new0(struct sock_io, 1);
chrc->notify_io->data = client;
chrc->notify_io->msg = dbus_message_ref(msg);
chrc->notify_io->destroy = notify_io_destroy;
@@ -1586,16 +1601,16 @@ static DBusMessage *characteristic_stop_notify(DBusConnection *conn,
const char *sender = dbus_message_get_sender(msg);
struct notify_client *client;
+ if (chrc->notify_io) {
+ destroy_sock(chrc, chrc->notify_io->io);
+ return dbus_message_new_method_return(msg);
+ }
+
client = queue_remove_if(chrc->notify_clients, match_notify_sender,
(void *) sender);
if (!client)
return btd_error_failed(msg, "No notify session started");
- if (chrc->notify_io) {
- characteristic_destroy_pipe(chrc, chrc->notify_io->io);
- return dbus_message_new_method_return(msg);
- }
-
queue_remove(chrc->service->client->all_notify_clients, client);
bt_gatt_client_unregister_notify(gatt, client->notify_id);
update_notifying(chrc);
@@ -1664,12 +1679,12 @@ static void characteristic_free(void *data)
if (chrc->write_io) {
queue_remove(chrc->service->client->ios, chrc->write_io->io);
- pipe_io_destroy(chrc->write_io);
+ sock_io_destroy(chrc->write_io);
}
if (chrc->notify_io) {
queue_remove(chrc->service->client->ios, chrc->notify_io->io);
- pipe_io_destroy(chrc->notify_io);
+ sock_io_destroy(chrc->notify_io);
}
queue_destroy(chrc->notify_clients, remove_client);
diff --git a/src/gatt-database.c b/src/gatt-database.c
index ec584fc3c..c31afa0e9 100644
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -2139,7 +2139,7 @@ static struct pending_op *send_write(struct btd_device *device,
return NULL;
}
-static bool pipe_hup(struct io *io, void *user_data)
+static bool sock_hup(struct io *io, void *user_data)
{
struct external_chrc *chrc = user_data;
@@ -2155,7 +2155,7 @@ static bool pipe_hup(struct io *io, void *user_data)
return false;
}
-static bool pipe_io_read(struct io *io, void *user_data)
+static bool sock_io_read(struct io *io, void *user_data)
{
struct external_chrc *chrc = user_data;
uint8_t buf[512];
@@ -2176,7 +2176,7 @@ static bool pipe_io_read(struct io *io, void *user_data)
return true;
}
-static struct io *pipe_io_new(int fd, void *user_data)
+static struct io *sock_io_new(int fd, void *user_data)
{
struct io *io;
@@ -2184,21 +2184,25 @@ static struct io *pipe_io_new(int fd, void *user_data)
io_set_close_on_destroy(io, true);
- io_set_read_handler(io, pipe_io_read, user_data, NULL);
+ io_set_read_handler(io, sock_io_read, user_data, NULL);
- io_set_disconnect_handler(io, pipe_hup, user_data, NULL);
+ io_set_disconnect_handler(io, sock_hup, user_data, NULL);
return io;
}
-static int pipe_io_send(struct io *io, const void *data, size_t len)
+static int sock_io_send(struct io *io, const void *data, size_t len)
{
+ struct msghdr msg;
struct iovec iov;
iov.iov_base = (void *) data;
iov.iov_len = len;
- return io_send(io, &iov, 1);
+ memset(&msg, 0, sizeof(msg));
+ msg.msg_iov = &iov;
+
+ return sendmsg(io_get_fd(io), &msg, MSG_NOSIGNAL);
}
static void acquire_write_reply(DBusMessage *message, void *user_data)
@@ -2227,9 +2231,9 @@ static void acquire_write_reply(DBusMessage *message, void *user_data)
DBG("AcquireWrite success: fd %d MTU %u\n", fd, mtu);
- chrc->write_io = pipe_io_new(fd, chrc);
+ chrc->write_io = sock_io_new(fd, chrc);
- if (pipe_io_send(chrc->write_io, op->data.iov_base,
+ if (sock_io_send(chrc->write_io, op->data.iov_base,
op->data.iov_len) < 0)
goto retry;
@@ -2308,7 +2312,7 @@ static void acquire_notify_reply(DBusMessage *message, void *user_data)
DBG("AcquireNotify success: fd %d MTU %u\n", fd, mtu);
- chrc->notify_io = pipe_io_new(fd, chrc);
+ chrc->notify_io = sock_io_new(fd, chrc);
__sync_fetch_and_add(&chrc->ntfy_cnt, 1);
@@ -2685,7 +2689,7 @@ static void chrc_write_cb(struct gatt_db_attribute *attrib,
chrc->prep_authorized = false;
if (chrc->write_io) {
- if (pipe_io_send(chrc->write_io, value, len) < 0) {
+ if (sock_io_send(chrc->write_io, value, len) < 0) {
error("Unable to write: %s", strerror(errno));
goto fail;
}
--
2.17.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 4/5] client: Switch from write to sendmsg for Acquire*
2018-11-20 10:39 [PATCH v2 1/5] gatt: Fix invalid read when disconnecting Luiz Augusto von Dentz
2018-11-20 10:39 ` [PATCH v2 2/5] doc/gatt-api: Restrict supported file descriptors Luiz Augusto von Dentz
2018-11-20 10:39 ` [PATCH v2 3/5] gatt: Switch from pipe2 to sockepair for Acquire* Luiz Augusto von Dentz
@ 2018-11-20 10:39 ` Luiz Augusto von Dentz
2018-11-20 10:39 ` [PATCH v2 5/5] meshctl: " Luiz Augusto von Dentz
3 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2018-11-20 10:39 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Use sendmsg with MSG_NOSIGNAL to prevent crashes involving SIGPIPE.
---
client/gatt.c | 85 ++++++++++++++++++++++++++++++++++++---------------
1 file changed, 60 insertions(+), 25 deletions(-)
diff --git a/client/gatt.c b/client/gatt.c
index c7dfe42d7..9877c4b47 100644
--- a/client/gatt.c
+++ b/client/gatt.c
@@ -33,6 +33,8 @@
#include <sys/uio.h>
#include <fcntl.h>
#include <string.h>
+#include <sys/types.h>
+#include <sys/socket.h>
#include <glib.h>
@@ -99,14 +101,14 @@ static GList *managers;
static GList *uuids;
static DBusMessage *pending_message = NULL;
-struct pipe_io {
+struct sock_io {
GDBusProxy *proxy;
struct io *io;
uint16_t mtu;
};
-static struct pipe_io write_io;
-static struct pipe_io notify_io;
+static struct sock_io write_io;
+static struct sock_io notify_io;
static void print_service(struct service *service, const char *description)
{
@@ -635,6 +637,24 @@ static void write_setup(DBusMessageIter *iter, void *user_data)
dbus_message_iter_close_container(iter, &dict);
}
+static int sock_send(struct io *io, struct iovec *iov, size_t iovlen)
+{
+ struct msghdr msg;
+ int ret;
+
+ memset(&msg, 0, sizeof(msg));
+ msg.msg_iov = iov;
+ msg.msg_iovlen = iovlen;
+
+ ret = sendmsg(io_get_fd(io), &msg, MSG_NOSIGNAL);
+ if (ret < 0) {
+ ret = -errno;
+ bt_shell_printf("sendmsg: %s", strerror(-ret));
+ }
+
+ return ret;
+}
+
static void write_attribute(GDBusProxy *proxy, char *val_str, uint16_t offset)
{
struct iovec iov;
@@ -671,7 +691,7 @@ static void write_attribute(GDBusProxy *proxy, char *val_str, uint16_t offset)
if (proxy == write_io.proxy && (write_io.io && write_io.mtu >= i)) {
bt_shell_printf("Attempting to write fd %d\n",
io_get_fd(write_io.io));
- if (io_send(write_io.io, &iov, 1) < 0) {
+ if (sock_send(write_io.io, &iov, 1) < 0) {
bt_shell_printf("Failed to write: %s", strerror(errno));
return bt_shell_noninteractive_quit(EXIT_FAILURE);
}
@@ -713,9 +733,11 @@ void gatt_write_attribute(GDBusProxy *proxy, int argc, char *argv[])
return bt_shell_noninteractive_quit(EXIT_FAILURE);
}
-static bool pipe_read(struct io *io, void *user_data)
+static bool sock_read(struct io *io, void *user_data)
{
struct chrc *chrc = user_data;
+ struct msghdr msg;
+ struct iovec iov;
uint8_t buf[MAX_ATTR_VAL_LEN];
int fd = io_get_fd(io);
ssize_t bytes_read;
@@ -723,8 +745,20 @@ static bool pipe_read(struct io *io, void *user_data)
if (io != notify_io.io && !chrc)
return true;
- bytes_read = read(fd, buf, sizeof(buf));
- if (bytes_read < 0)
+ iov.iov_base = buf;
+ iov.iov_len = sizeof(buf);
+
+ memset(&msg, 0, sizeof(msg));
+ msg.msg_iov = &iov;
+ msg.msg_iovlen = 1;
+
+ bytes_read = recvmsg(fd, &msg, MSG_DONTWAIT);
+ if (bytes_read < 0) {
+ bt_shell_printf("recvmsg: %s", strerror(errno));
+ return false;
+ }
+
+ if (!bytes_read)
return false;
if (chrc)
@@ -739,12 +773,12 @@ static bool pipe_read(struct io *io, void *user_data)
return true;
}
-static bool pipe_hup(struct io *io, void *user_data)
+static bool sock_hup(struct io *io, void *user_data)
{
struct chrc *chrc = user_data;
if (chrc) {
- bt_shell_printf("Attribute %s %s pipe closed\n", chrc->path,
+ bt_shell_printf("Attribute %s %s sock closed\n", chrc->path,
io == chrc->write_io ? "Write" : "Notify");
if (io == chrc->write_io) {
@@ -768,7 +802,7 @@ static bool pipe_hup(struct io *io, void *user_data)
return false;
}
-static struct io *pipe_io_new(int fd, void *user_data)
+static struct io *sock_io_new(int fd, void *user_data)
{
struct io *io;
@@ -776,9 +810,9 @@ static struct io *pipe_io_new(int fd, void *user_data)
io_set_close_on_destroy(io, true);
- io_set_read_handler(io, pipe_read, user_data, NULL);
+ io_set_read_handler(io, sock_read, user_data, NULL);
- io_set_disconnect_handler(io, pipe_hup, user_data, NULL);
+ io_set_disconnect_handler(io, sock_hup, user_data, NULL);
return io;
}
@@ -810,7 +844,7 @@ static void acquire_write_reply(DBusMessage *message, void *user_data)
bt_shell_printf("AcquireWrite success: fd %d MTU %u\n", fd,
write_io.mtu);
- write_io.io = pipe_io_new(fd, NULL);
+ write_io.io = sock_io_new(fd, NULL);
return bt_shell_noninteractive_quit(EXIT_SUCCESS);
}
@@ -892,7 +926,7 @@ static void acquire_notify_reply(DBusMessage *message, void *user_data)
bt_shell_printf("AcquireNotify success: fd %d MTU %u\n", fd,
notify_io.mtu);
- notify_io.io = pipe_io_new(fd, NULL);
+ notify_io.io = sock_io_new(fd, NULL);
return bt_shell_noninteractive_quit(EXIT_SUCCESS);
}
@@ -1932,39 +1966,40 @@ static DBusMessage *chrc_write_value(DBusConnection *conn, DBusMessage *msg,
return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
}
-static DBusMessage *chrc_create_pipe(struct chrc *chrc, DBusMessage *msg)
+static DBusMessage *create_sock(struct chrc *chrc, DBusMessage *msg)
{
- int pipefd[2];
+ int fds[2];
struct io *io;
bool dir;
DBusMessage *reply;
- if (pipe2(pipefd, O_DIRECT | O_NONBLOCK | O_CLOEXEC) < 0)
+ if (socketpair(AF_LOCAL, SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC,
+ 0, fds) < 0)
return g_dbus_create_error(msg, "org.bluez.Error.Failed", "%s",
strerror(errno));
dir = dbus_message_has_member(msg, "AcquireWrite");
- io = pipe_io_new(pipefd[!dir], chrc);
+ io = sock_io_new(fds[!dir], chrc);
if (!io) {
- close(pipefd[0]);
- close(pipefd[1]);
+ close(fds[0]);
+ close(fds[1]);
return g_dbus_create_error(msg, "org.bluez.Error.Failed", "%s",
strerror(errno));
}
- reply = g_dbus_create_reply(msg, DBUS_TYPE_UNIX_FD, &pipefd[dir],
+ reply = g_dbus_create_reply(msg, DBUS_TYPE_UNIX_FD, &fds[dir],
DBUS_TYPE_UINT16, &chrc->mtu,
DBUS_TYPE_INVALID);
- close(pipefd[dir]);
+ close(fds[dir]);
if (dir)
chrc->write_io = io;
else
chrc->notify_io = io;
- bt_shell_printf("[" COLORED_CHG "] Attribute %s %s pipe acquired\n",
+ bt_shell_printf("[" COLORED_CHG "] Attribute %s %s sock acquired\n",
chrc->path, dir ? "Write" : "Notify");
return reply;
@@ -1993,7 +2028,7 @@ static DBusMessage *chrc_acquire_write(DBusConnection *conn, DBusMessage *msg,
bt_shell_printf("AcquireWrite: %s link %s\n", path_to_address(device),
link);
- reply = chrc_create_pipe(chrc, msg);
+ reply = create_sock(chrc, msg);
if (chrc->write_io)
g_dbus_emit_property_changed(conn, chrc->path, CHRC_INTERFACE,
@@ -2025,7 +2060,7 @@ static DBusMessage *chrc_acquire_notify(DBusConnection *conn, DBusMessage *msg,
bt_shell_printf("AcquireNotify: %s link %s\n", path_to_address(device),
link);
- reply = chrc_create_pipe(chrc, msg);
+ reply = create_sock(chrc, msg);
if (chrc->notify_io)
g_dbus_emit_property_changed(conn, chrc->path, CHRC_INTERFACE,
--
2.17.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 5/5] meshctl: Switch from write to sendmsg for Acquire*
2018-11-20 10:39 [PATCH v2 1/5] gatt: Fix invalid read when disconnecting Luiz Augusto von Dentz
` (2 preceding siblings ...)
2018-11-20 10:39 ` [PATCH v2 4/5] client: Switch from write to sendmsg " Luiz Augusto von Dentz
@ 2018-11-20 10:39 ` Luiz Augusto von Dentz
3 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2018-11-20 10:39 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Use sendmsg with MSG_NOSIGNAL to prevent crashes involving SIGPIPE.
---
tools/mesh/gatt.c | 64 +++++++++++++++++++++++++++++++++--------------
1 file changed, 45 insertions(+), 19 deletions(-)
diff --git a/tools/mesh/gatt.c b/tools/mesh/gatt.c
index 2c29c09b5..2269a20cf 100644
--- a/tools/mesh/gatt.c
+++ b/tools/mesh/gatt.c
@@ -166,7 +166,25 @@ uint16_t mesh_gatt_sar(uint8_t **pkt, uint16_t size)
}
}
-static bool pipe_write(struct io *io, void *user_data)
+static int sock_send(struct io *io, struct iovec *iov, size_t iovlen)
+{
+ struct msghdr msg;
+ int ret;
+
+ memset(&msg, 0, sizeof(msg));
+ msg.msg_iov = iov;
+ msg.msg_iovlen = iovlen;
+
+ ret = sendmsg(io_get_fd(io), &msg, MSG_NOSIGNAL);
+ if (ret < 0) {
+ ret = -errno;
+ bt_shell_printf("sendmsg: %s", strerror(-ret));
+ }
+
+ return ret;
+}
+
+static bool sock_write(struct io *io, void *user_data)
{
struct write_data *data = user_data;
struct iovec iov[2];
@@ -200,9 +218,8 @@ static bool pipe_write(struct io *io, void *user_data)
iov[1] = data->iov;
- err = io_send(io, iov, 2);
+ err = sock_send(io, iov, 2);
if (err < 0) {
- bt_shell_printf("Failed to write: %s\n", strerror(-err));
write_data_free(data);
return false;
}
@@ -247,7 +264,7 @@ static void notify_io_destroy(void)
notify_mtu = 0;
}
-static bool pipe_hup(struct io *io, void *user_data)
+static bool sock_hup(struct io *io, void *user_data)
{
bt_shell_printf("%s closed\n", io == notify_io ? "Notify" : "Write");
@@ -259,7 +276,7 @@ static bool pipe_hup(struct io *io, void *user_data)
return false;
}
-static struct io *pipe_io_new(int fd)
+static struct io *sock_io_new(int fd)
{
struct io *io;
@@ -267,7 +284,7 @@ static struct io *pipe_io_new(int fd)
io_set_close_on_destroy(io, true);
- io_set_disconnect_handler(io, pipe_hup, NULL, NULL);
+ io_set_disconnect_handler(io, sock_hup, NULL, NULL);
return io;
}
@@ -296,9 +313,9 @@ static void acquire_write_reply(DBusMessage *message, void *user_data)
bt_shell_printf("AcquireWrite success: fd %d MTU %u\n", fd, write_mtu);
- write_io = pipe_io_new(fd);
+ write_io = sock_io_new(fd);
- pipe_write(write_io, data);
+ sock_write(write_io, data);
}
static void acquire_setup(DBusMessageIter *iter, void *user_data)
@@ -342,7 +359,7 @@ bool mesh_gatt_write(GDBusProxy *proxy, uint8_t *buf, uint16_t len,
data->cb = cb;
if (write_io)
- return pipe_write(write_io, data);
+ return sock_write(write_io, data);
if (g_dbus_proxy_method_call(proxy, "AcquireWrite",
acquire_setup, acquire_write_reply,
@@ -377,9 +394,11 @@ done:
g_free(data);
}
-static bool pipe_read(struct io *io, bool prov, void *user_data)
+static bool sock_read(struct io *io, bool prov, void *user_data)
{
struct mesh_node *node = user_data;
+ struct msghdr msg;
+ struct iovec iov;
uint8_t buf[512];
uint8_t *res;
int fd = io_get_fd(io);
@@ -388,9 +407,16 @@ static bool pipe_read(struct io *io, bool prov, void *user_data)
if (io != notify_io)
return true;
- while ((len = read(fd, buf, sizeof(buf)))) {
+ iov.iov_base = buf;
+ iov.iov_len = sizeof(buf);
+
+ memset(&msg, 0, sizeof(msg));
+ msg.msg_iov = &iov;
+ msg.msg_iovlen = 1;
+
+ while ((len = recvmsg(fd, &msg, MSG_DONTWAIT))) {
if (len <= 0)
- break;
+ return false;
res = buf;
len_sar = mesh_gatt_sar(&res, len);
@@ -404,14 +430,14 @@ static bool pipe_read(struct io *io, bool prov, void *user_data)
return true;
}
-static bool pipe_read_prov(struct io *io, void *user_data)
+static bool sock_read_prov(struct io *io, void *user_data)
{
- return pipe_read(io, true, user_data);
+ return sock_read(io, true, user_data);
}
-static bool pipe_read_proxy(struct io *io, void *user_data)
+static bool sock_read_proxy(struct io *io, void *user_data)
{
- return pipe_read(io, false, user_data);
+ return sock_read(io, false, user_data);
}
static void acquire_notify_reply(DBusMessage *message, void *user_data)
@@ -457,15 +483,15 @@ static void acquire_notify_reply(DBusMessage *message, void *user_data)
if (g_dbus_proxy_get_property(data->proxy, "UUID", &iter) == FALSE)
goto done;
- notify_io = pipe_io_new(fd);
+ notify_io = sock_io_new(fd);
dbus_message_iter_get_basic(&iter, &uuid);
if (!bt_uuid_strcmp(uuid, MESH_PROV_DATA_OUT_UUID_STR))
- io_set_read_handler(notify_io, pipe_read_prov, data->user_data,
+ io_set_read_handler(notify_io, sock_read_prov, data->user_data,
NULL);
else if (!bt_uuid_strcmp(uuid, MESH_PROXY_DATA_OUT_UUID_STR))
- io_set_read_handler(notify_io, pipe_read_proxy, data->user_data,
+ io_set_read_handler(notify_io, sock_read_proxy, data->user_data,
NULL);
done:
--
2.17.2
^ permalink raw reply related [flat|nested] 5+ messages in thread