diff mbox series

[lttng-tools] Fix: wrong parameter to fcntl in pipe_set_flag

Message ID 1510096210-27806-1-git-send-email-jdesfossez@efficios.com
State Accepted, archived
Delegated to: Jérémie Galarneau
Headers show
Series [lttng-tools] Fix: wrong parameter to fcntl in pipe_set_flag | expand

Commit Message

Julien Desfossez Nov. 7, 2017, 11:10 p.m. UTC
Depending on the flags passed, fcntl must be called with F_SETFD or
F_SETFL. This fix checks the flag passed and ensure it is valid and
calls fcntl with the right parameter.

Also, for CLOEXEC, we need to pass FD_CLOEXEC, not O_CLOEXEC.

Signed-off-by: Julien Desfossez <jdesfossez at efficios.com>
---
 src/bin/lttng-sessiond/notification-thread.c |  2 +-
 src/common/pipe.c                            | 25 ++++++++++++++++++++++---
 2 files changed, 23 insertions(+), 4 deletions(-)

Comments

Jérémie Galarneau Nov. 14, 2017, 12:26 a.m. UTC | #1
I split this patch in two parts since the fix in notification-thread.c
only applies to master and stable-2.10.

Thanks!
Jérémie

On 7 November 2017 at 18:10, Julien Desfossez <jdesfossez at efficios.com> wrote:
> Depending on the flags passed, fcntl must be called with F_SETFD or
> F_SETFL. This fix checks the flag passed and ensure it is valid and
> calls fcntl with the right parameter.
>
> Also, for CLOEXEC, we need to pass FD_CLOEXEC, not O_CLOEXEC.
>
> Signed-off-by: Julien Desfossez <jdesfossez at efficios.com>
> ---
>  src/bin/lttng-sessiond/notification-thread.c |  2 +-
>  src/common/pipe.c                            | 25 ++++++++++++++++++++++---
>  2 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/src/bin/lttng-sessiond/notification-thread.c b/src/bin/lttng-sessiond/notification-thread.c
> index c47b365..3ef7df1 100644
> --- a/src/bin/lttng-sessiond/notification-thread.c
> +++ b/src/bin/lttng-sessiond/notification-thread.c
> @@ -96,7 +96,7 @@ struct notification_thread_handle *notification_thread_handle_create(
>                 goto end;
>         }
>
> -       event_pipe = lttng_pipe_open(O_CLOEXEC);
> +       event_pipe = lttng_pipe_open(FD_CLOEXEC);
>         if (!event_pipe) {
>                 ERR("event_pipe creation");
>                 goto error;
> diff --git a/src/common/pipe.c b/src/common/pipe.c
> index 4220a40..4fe45ef 100644
> --- a/src/common/pipe.c
> +++ b/src/common/pipe.c
> @@ -154,9 +154,28 @@ static int _pipe_set_flags(struct lttng_pipe *pipe, int flags)
>         }
>
>         for (i = 0; i < 2; i++) {
> -               ret = fcntl(pipe->fd[i], F_SETFD, flags);
> -               if (ret < 0) {
> -                       PERROR("fcntl lttng pipe %d", flags);
> +               if (flags & O_NONBLOCK) {
> +                       ret = fcntl(pipe->fd[i], F_SETFL, O_NONBLOCK);
> +                       if (ret < 0) {
> +                               PERROR("fcntl lttng pipe %d", flags);
> +                               goto end;
> +                       }
> +               }
> +               if (flags & FD_CLOEXEC) {
> +                       ret = fcntl(pipe->fd[i], F_SETFD, FD_CLOEXEC);
> +                       if (ret < 0) {
> +                               PERROR("fcntl lttng pipe %d", flags);
> +                               goto end;
> +                       }
> +               }
> +               /*
> +                * We only check for O_NONBLOCK or FD_CLOEXEC, if another flag is
> +                * needed, we can add it, but for now just make sure we don't make
> +                * mistakes with the parameters we pass.
> +                */
> +               if (!(flags & O_NONBLOCK) && !(flags & FD_CLOEXEC)) {
> +                       fprintf(stderr, "Unsupported flag\n");
> +                       ret = -1;
>                         goto end;
>                 }
>         }
> --
> 2.7.4
>
diff mbox series

Patch

diff --git a/src/bin/lttng-sessiond/notification-thread.c b/src/bin/lttng-sessiond/notification-thread.c
index c47b365..3ef7df1 100644
--- a/src/bin/lttng-sessiond/notification-thread.c
+++ b/src/bin/lttng-sessiond/notification-thread.c
@@ -96,7 +96,7 @@  struct notification_thread_handle *notification_thread_handle_create(
 		goto end;
 	}
 
-	event_pipe = lttng_pipe_open(O_CLOEXEC);
+	event_pipe = lttng_pipe_open(FD_CLOEXEC);
 	if (!event_pipe) {
 		ERR("event_pipe creation");
 		goto error;
diff --git a/src/common/pipe.c b/src/common/pipe.c
index 4220a40..4fe45ef 100644
--- a/src/common/pipe.c
+++ b/src/common/pipe.c
@@ -154,9 +154,28 @@  static int _pipe_set_flags(struct lttng_pipe *pipe, int flags)
 	}
 
 	for (i = 0; i < 2; i++) {
-		ret = fcntl(pipe->fd[i], F_SETFD, flags);
-		if (ret < 0) {
-			PERROR("fcntl lttng pipe %d", flags);
+		if (flags & O_NONBLOCK) {
+			ret = fcntl(pipe->fd[i], F_SETFL, O_NONBLOCK);
+			if (ret < 0) {
+				PERROR("fcntl lttng pipe %d", flags);
+				goto end;
+			}
+		}
+		if (flags & FD_CLOEXEC) {
+			ret = fcntl(pipe->fd[i], F_SETFD, FD_CLOEXEC);
+			if (ret < 0) {
+				PERROR("fcntl lttng pipe %d", flags);
+				goto end;
+			}
+		}
+		/*
+		 * We only check for O_NONBLOCK or FD_CLOEXEC, if another flag is
+		 * needed, we can add it, but for now just make sure we don't make
+		 * mistakes with the parameters we pass.
+		 */
+		if (!(flags & O_NONBLOCK) && !(flags & FD_CLOEXEC)) {
+			fprintf(stderr, "Unsupported flag\n");
+			ret = -1;
 			goto end;
 		}
 	}