2018-11-16 20:57:00

by Gal Ben Haim

Subject: [PATCH BlueZ] io_send: Write to pipes with the MSG_NOSIGNAL flag

bluetoothd receives a SIGPIPE and terminates if writing to a pipe that
was acquired by AcquireNotify and there are no readers. it can be
reproduced by terminating the reader process without closing the reader
end of the pipe.

Requesting not to send SIGPIPE will cause the write operation to return an
error which will be logged as "io_send: Broken pipe".
---
src/shared/io-ell.c | 14 ++++++++++++--
src/shared/io-glib.c | 15 +++++++++++++--
src/shared/io-mainloop.c | 14 ++++++++++++--
src/shared/io.h | 2 +-
4 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/src/shared/io-ell.c b/src/shared/io-ell.c
index 6748982c7..2bd166966 100644
--- a/src/shared/io-ell.c
+++ b/src/shared/io-ell.c
@@ -117,7 +117,7 @@ bool io_set_disconnect_handler(struct io *io, io_callback_func_t callback,
user_data, destroy);
}

-ssize_t io_send(struct io *io, const struct iovec *iov, int iovcnt)
+ssize_t io_send(struct io *io, struct iovec *iov, int iovcnt)
{
ssize_t ret;
int fd;
@@ -129,8 +129,18 @@ ssize_t io_send(struct io *io, const struct iovec *iov, int iovcnt)
if (fd < 0)
return -ENOTCONN;

+ const struct msghdr msg = {
+ .msg_name = NULL,
+ .msg_namelen = 0,
+ .msg_iov = iov,
+ .msg_iovlen = iovcnt,
+ .msg_control = NULL,
+ .msg_controllen = 0,
+ .msg_flags = 0
+ };
+
do {
- ret = writev(fd, iov, iovcnt);
+ ret = sendmsg(fd, &msg, MSG_NOSIGNAL);
} while (ret < 0 && errno == EINTR);

if (ret < 0)
diff --git a/src/shared/io-glib.c b/src/shared/io-glib.c
index d62de4e10..7e4c9961b 100644
--- a/src/shared/io-glib.c
+++ b/src/shared/io-glib.c
@@ -26,6 +26,7 @@
#endif

#include <errno.h>
+#include <sys/socket.h>

#include <glib.h>

@@ -263,7 +264,7 @@ bool io_set_disconnect_handler(struct io *io, io_callback_func_t callback,
return io_set_handler(io, G_IO_HUP, callback, user_data, destroy);
}

-ssize_t io_send(struct io *io, const struct iovec *iov, int iovcnt)
+ssize_t io_send(struct io *io, struct iovec *iov, int iovcnt)
{
int fd;
ssize_t ret;
@@ -273,8 +274,18 @@ ssize_t io_send(struct io *io, const struct iovec *iov, int iovcnt)

fd = io_get_fd(io);

+ const struct msghdr msg = {
+ .msg_name = NULL,
+ .msg_namelen = 0,
+ .msg_iov = iov,
+ .msg_iovlen = iovcnt,
+ .msg_control = NULL,
+ .msg_controllen = 0,
+ .msg_flags = 0
+ };
+
do {
- ret = writev(fd, iov, iovcnt);
+ ret = sendmsg(fd, &msg, MSG_NOSIGNAL);
} while (ret < 0 && errno == EINTR);

if (ret < 0)
diff --git a/src/shared/io-mainloop.c b/src/shared/io-mainloop.c
index 2306c3479..4b6cc14c7 100644
--- a/src/shared/io-mainloop.c
+++ b/src/shared/io-mainloop.c
@@ -298,15 +298,25 @@ bool io_set_disconnect_handler(struct io *io, io_callback_func_t callback,
return true;
}

-ssize_t io_send(struct io *io, const struct iovec *iov, int iovcnt)
+ssize_t io_send(struct io *io, struct iovec *iov, int iovcnt)
{
ssize_t ret;

if (!io || io->fd < 0)
return -ENOTCONN;

+ const struct msghdr msg = {
+ .msg_name = NULL,
+ .msg_namelen = 0,
+ .msg_iov = iov,
+ .msg_iovlen = iovcnt,
+ .msg_control = NULL,
+ .msg_controllen = 0,
+ .msg_flags = 0
+ };
+
do {
- ret = writev(io->fd, iov, iovcnt);
+ ret = sendmsg(io->fd, &msg, MSG_NOSIGNAL);
} while (ret < 0 && errno == EINTR);

if (ret < 0)
diff --git a/src/shared/io.h b/src/shared/io.h
index 8bc1111d0..522ec26ff 100644
--- a/src/shared/io.h
+++ b/src/shared/io.h
@@ -34,7 +34,7 @@ void io_destroy(struct io *io);
int io_get_fd(struct io *io);
bool io_set_close_on_destroy(struct io *io, bool do_close);

-ssize_t io_send(struct io *io, const struct iovec *iov, int iovcnt);
+ssize_t io_send(struct io *io, struct iovec *iov, int iovcnt);
bool io_shutdown(struct io *io);

typedef bool (*io_callback_func_t)(struct io *io, void *user_data);
--
2.19.1

2018-11-16 21:36:22

by Gal Ben Haim

Subject: Re: [PATCH BlueZ] io_send: Write to pipes with the MSG_NOSIGNAL flag

I did this change but it doesn't seem to write as expected, what do you think?

On Fri, Nov 16, 2018 at 10:56 PM Gal Ben-Haim <[email protected]> wrote:
>
> bluetoothd receives a SIGPIPE and terminates if writing to a pipe that
> was acquired by AcquireNotify and there are no readers. it can be
> reproduced by terminating the reader process without closing the reader
> end of the pipe.
>
> Requesting not to send SIGPIPE will cause the write operation to return an
> error which will be logged as "io_send: Broken pipe".
> ---
> src/shared/io-ell.c | 14 ++++++++++++--
> src/shared/io-glib.c | 15 +++++++++++++--
> src/shared/io-mainloop.c | 14 ++++++++++++--
> src/shared/io.h | 2 +-
> 4 files changed, 38 insertions(+), 7 deletions(-)
>
> diff --git a/src/shared/io-ell.c b/src/shared/io-ell.c
> index 6748982c7..2bd166966 100644
> --- a/src/shared/io-ell.c
> +++ b/src/shared/io-ell.c
> @@ -117,7 +117,7 @@ bool io_set_disconnect_handler(struct io *io, io_callback_func_t callback,
> user_data, destroy);
> }
>
> -ssize_t io_send(struct io *io, const struct iovec *iov, int iovcnt)
> +ssize_t io_send(struct io *io, struct iovec *iov, int iovcnt)
> {
> ssize_t ret;
> int fd;
> @@ -129,8 +129,18 @@ ssize_t io_send(struct io *io, const struct iovec *iov, int iovcnt)
> if (fd < 0)
> return -ENOTCONN;
>
> + const struct msghdr msg = {
> + .msg_name = NULL,
> + .msg_namelen = 0,
> + .msg_iov = iov,
> + .msg_iovlen = iovcnt,
> + .msg_control = NULL,
> + .msg_controllen = 0,
> + .msg_flags = 0
> + };
> +
> do {
> - ret = writev(fd, iov, iovcnt);
> + ret = sendmsg(fd, &msg, MSG_NOSIGNAL);
> } while (ret < 0 && errno == EINTR);
>
> if (ret < 0)
> diff --git a/src/shared/io-glib.c b/src/shared/io-glib.c
> index d62de4e10..7e4c9961b 100644
> --- a/src/shared/io-glib.c
> +++ b/src/shared/io-glib.c
> @@ -26,6 +26,7 @@
> #endif
>
> #include <errno.h>
> +#include <sys/socket.h>
>
> #include <glib.h>
>
> @@ -263,7 +264,7 @@ bool io_set_disconnect_handler(struct io *io, io_callback_func_t callback,
> return io_set_handler(io, G_IO_HUP, callback, user_data, destroy);
> }
>
> -ssize_t io_send(struct io *io, const struct iovec *iov, int iovcnt)
> +ssize_t io_send(struct io *io, struct iovec *iov, int iovcnt)
> {
> int fd;
> ssize_t ret;
> @@ -273,8 +274,18 @@ ssize_t io_send(struct io *io, const struct iovec *iov, int iovcnt)
>
> fd = io_get_fd(io);
>
> + const struct msghdr msg = {
> + .msg_name = NULL,
> + .msg_namelen = 0,
> + .msg_iov = iov,
> + .msg_iovlen = iovcnt,
> + .msg_control = NULL,
> + .msg_controllen = 0,
> + .msg_flags = 0
> + };
> +
> do {
> - ret = writev(fd, iov, iovcnt);
> + ret = sendmsg(fd, &msg, MSG_NOSIGNAL);
> } while (ret < 0 && errno == EINTR);
>
> if (ret < 0)
> diff --git a/src/shared/io-mainloop.c b/src/shared/io-mainloop.c
> index 2306c3479..4b6cc14c7 100644
> --- a/src/shared/io-mainloop.c
> +++ b/src/shared/io-mainloop.c
> @@ -298,15 +298,25 @@ bool io_set_disconnect_handler(struct io *io, io_callback_func_t callback,
> return true;
> }
>
> -ssize_t io_send(struct io *io, const struct iovec *iov, int iovcnt)
> +ssize_t io_send(struct io *io, struct iovec *iov, int iovcnt)
> {
> ssize_t ret;
>
> if (!io || io->fd < 0)
> return -ENOTCONN;
>
> + const struct msghdr msg = {
> + .msg_name = NULL,
> + .msg_namelen = 0,
> + .msg_iov = iov,
> + .msg_iovlen = iovcnt,
> + .msg_control = NULL,
> + .msg_controllen = 0,
> + .msg_flags = 0
> + };
> +
> do {
> - ret = writev(io->fd, iov, iovcnt);
> + ret = sendmsg(io->fd, &msg, MSG_NOSIGNAL);
> } while (ret < 0 && errno == EINTR);
>
> if (ret < 0)
> diff --git a/src/shared/io.h b/src/shared/io.h
> index 8bc1111d0..522ec26ff 100644
> --- a/src/shared/io.h
> +++ b/src/shared/io.h
> @@ -34,7 +34,7 @@ void io_destroy(struct io *io);
> int io_get_fd(struct io *io);
> bool io_set_close_on_destroy(struct io *io, bool do_close);
>
> -ssize_t io_send(struct io *io, const struct iovec *iov, int iovcnt);
> +ssize_t io_send(struct io *io, struct iovec *iov, int iovcnt);
> bool io_shutdown(struct io *io);
>
> typedef bool (*io_callback_func_t)(struct io *io, void *user_data);
> --
> 2.19.1
>

2018-11-16 23:44:23

by Gal Ben Haim

Subject: Re: [PATCH BlueZ] io_send: Write to pipes with the MSG_NOSIGNAL flag

this is the error message: io_send: Socket operation on non-socket
On Fri, Nov 16, 2018 at 11:36 PM Gal Ben Haim <[email protected]> wrote:
>
> I did this change but it doesn't seem to write as expected, what do you think?
>
> On Fri, Nov 16, 2018 at 10:56 PM Gal Ben-Haim <[email protected]> wrote:
> >
> > bluetoothd receives a SIGPIPE and terminates if writing to a pipe that
> > was acquired by AcquireNotify and there are no readers. it can be
> > reproduced by terminating the reader process without closing the reader
> > end of the pipe.
> >
> > Requesting not to send SIGPIPE will cause the write operation to return an
> > error which will be logged as "io_send: Broken pipe".
> > ---
> > src/shared/io-ell.c | 14 ++++++++++++--
> > src/shared/io-glib.c | 15 +++++++++++++--
> > src/shared/io-mainloop.c | 14 ++++++++++++--
> > src/shared/io.h | 2 +-
> > 4 files changed, 38 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/shared/io-ell.c b/src/shared/io-ell.c
> > index 6748982c7..2bd166966 100644
> > --- a/src/shared/io-ell.c
> > +++ b/src/shared/io-ell.c
> > @@ -117,7 +117,7 @@ bool io_set_disconnect_handler(struct io *io, io_callback_func_t callback,
> > user_data, destroy);
> > }
> >
> > -ssize_t io_send(struct io *io, const struct iovec *iov, int iovcnt)
> > +ssize_t io_send(struct io *io, struct iovec *iov, int iovcnt)
> > {
> > ssize_t ret;
> > int fd;
> > @@ -129,8 +129,18 @@ ssize_t io_send(struct io *io, const struct iovec *iov, int iovcnt)
> > if (fd < 0)
> > return -ENOTCONN;
> >
> > + const struct msghdr msg = {
> > + .msg_name = NULL,
> > + .msg_namelen = 0,
> > + .msg_iov = iov,
> > + .msg_iovlen = iovcnt,
> > + .msg_control = NULL,
> > + .msg_controllen = 0,
> > + .msg_flags = 0
> > + };
> > +
> > do {
> > - ret = writev(fd, iov, iovcnt);
> > + ret = sendmsg(fd, &msg, MSG_NOSIGNAL);
> > } while (ret < 0 && errno == EINTR);
> >
> > if (ret < 0)
> > diff --git a/src/shared/io-glib.c b/src/shared/io-glib.c
> > index d62de4e10..7e4c9961b 100644
> > --- a/src/shared/io-glib.c
> > +++ b/src/shared/io-glib.c
> > @@ -26,6 +26,7 @@
> > #endif
> >
> > #include <errno.h>
> > +#include <sys/socket.h>
> >
> > #include <glib.h>
> >
> > @@ -263,7 +264,7 @@ bool io_set_disconnect_handler(struct io *io, io_callback_func_t callback,
> > return io_set_handler(io, G_IO_HUP, callback, user_data, destroy);
> > }
> >
> > -ssize_t io_send(struct io *io, const struct iovec *iov, int iovcnt)
> > +ssize_t io_send(struct io *io, struct iovec *iov, int iovcnt)
> > {
> > int fd;
> > ssize_t ret;
> > @@ -273,8 +274,18 @@ ssize_t io_send(struct io *io, const struct iovec *iov, int iovcnt)
> >
> > fd = io_get_fd(io);
> >
> > + const struct msghdr msg = {
> > + .msg_name = NULL,
> > + .msg_namelen = 0,
> > + .msg_iov = iov,
> > + .msg_iovlen = iovcnt,
> > + .msg_control = NULL,
> > + .msg_controllen = 0,
> > + .msg_flags = 0
> > + };
> > +
> > do {
> > - ret = writev(fd, iov, iovcnt);
> > + ret = sendmsg(fd, &msg, MSG_NOSIGNAL);
> > } while (ret < 0 && errno == EINTR);
> >
> > if (ret < 0)
> > diff --git a/src/shared/io-mainloop.c b/src/shared/io-mainloop.c
> > index 2306c3479..4b6cc14c7 100644
> > --- a/src/shared/io-mainloop.c
> > +++ b/src/shared/io-mainloop.c
> > @@ -298,15 +298,25 @@ bool io_set_disconnect_handler(struct io *io, io_callback_func_t callback,
> > return true;
> > }
> >
> > -ssize_t io_send(struct io *io, const struct iovec *iov, int iovcnt)
> > +ssize_t io_send(struct io *io, struct iovec *iov, int iovcnt)
> > {
> > ssize_t ret;
> >
> > if (!io || io->fd < 0)
> > return -ENOTCONN;
> >
> > + const struct msghdr msg = {
> > + .msg_name = NULL,
> > + .msg_namelen = 0,
> > + .msg_iov = iov,
> > + .msg_iovlen = iovcnt,
> > + .msg_control = NULL,
> > + .msg_controllen = 0,
> > + .msg_flags = 0
> > + };
> > +
> > do {
> > - ret = writev(io->fd, iov, iovcnt);
> > + ret = sendmsg(io->fd, &msg, MSG_NOSIGNAL);
> > } while (ret < 0 && errno == EINTR);
> >
> > if (ret < 0)
> > diff --git a/src/shared/io.h b/src/shared/io.h
> > index 8bc1111d0..522ec26ff 100644
> > --- a/src/shared/io.h
> > +++ b/src/shared/io.h
> > @@ -34,7 +34,7 @@ void io_destroy(struct io *io);
> > int io_get_fd(struct io *io);
> > bool io_set_close_on_destroy(struct io *io, bool do_close);
> >
> > -ssize_t io_send(struct io *io, const struct iovec *iov, int iovcnt);
> > +ssize_t io_send(struct io *io, struct iovec *iov, int iovcnt);
> > bool io_shutdown(struct io *io);
> >
> > typedef bool (*io_callback_func_t)(struct io *io, void *user_data);
> > --
> > 2.19.1
> >