Fix: application SIGBUS when starting in parallel with sessiond

Fix: application SIGBUS when starting in parallel with sessiond

[Mathieu Desnoyers]

[-- Attachment #1: Type: text/plain, Size: 132 bytes --]

Should be applied to master, stable-2.3, stable-2.2.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fix-sigbus-race.patch --]
[-- Type: text/x-patch; name=fix-sigbus-race.patch, Size: 3356 bytes --]

commit be2bcc7e9a02e5bd78dc2615886af32ddd87032b
Author: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date:   Wed Nov 6 07:06:51 2013 -0500

    Fix: application SIGBUS when starting in parallel with sessiond
    
    There is a race between application startup and sessiond startup, where
    there is an intermediate state where applications can SIGBUS if they see
    a zero-sized shm, if the shm has been created, but not ftruncated yet.
    
    On the sessiond side, we need to ensure that the shared memory is
    writeable by applications as long as its size is 0, which allow
    applications to perform ftruncate and extend its size.
    
    On the UST side, another commit needs to ensure that UST can read the
    shared memory file descriptor with a read() system call before they
    try accessing it through a memory map (which triggers the SIGBUS if
    the access goes beyond the file size)
    
    Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/src/bin/lttng-sessiond/shm.c b/src/bin/lttng-sessiond/shm.c
index b94f4eb..7bb52d4 100644
--- a/src/bin/lttng-sessiond/shm.c
+++ b/src/bin/lttng-sessiond/shm.c
@@ -48,40 +48,17 @@ static int get_wait_shm(char *shm_path, size_t mmap_size, int global)
 	/* Default permissions */
 	mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP;
 
-	/* Change owner of the shm path */
+	/*
+	 * Change owner of the shm path.
+	 */
 	if (global) {
-		ret = chown(shm_path, 0, 0);
-		if (ret < 0) {
-			if (errno != ENOENT) {
-				PERROR("chown wait shm");
-				goto error;
-			}
-		}
-
 		/*
-		 * If global session daemon, any application can register so the shm
-		 * needs to be set in read-only mode for others.
+		 * If global session daemon, any application can
+		 * register. Make it initially writeable so applications
+		 * registering concurrently can do ftruncate() by
+		 * themselves.
 		 */
-		mode |= S_IROTH;
-	} else {
-		ret = chown(shm_path, getuid(), getgid());
-		if (ret < 0) {
-			if (errno != ENOENT) {
-				PERROR("chown wait shm");
-				goto error;
-			}
-		}
-	}
-
-	/*
-	 * Set permissions to the shm even if we did not create the shm.
-	 */
-	ret = chmod(shm_path, mode);
-	if (ret < 0) {
-		if (errno != ENOENT) {
-			PERROR("chmod wait shm");
-			goto error;
-		}
+		mode |= S_IROTH | S_IWOTH;
 	}
 
 	/*
@@ -107,13 +84,32 @@ static int get_wait_shm(char *shm_path, size_t mmap_size, int global)
 	}
 
 #ifndef __FreeBSD__
-	ret = fchmod(wait_shm_fd, mode);
-	if (ret < 0) {
-		PERROR("fchmod");
-		exit(EXIT_FAILURE);
+	if (global) {
+		ret = fchown(wait_shm_fd, 0, 0);
+		if (ret < 0) {
+			PERROR("fchown");
+			exit(EXIT_FAILURE);
+		}
+		/*
+		 * If global session daemon, any application can
+		 * register so the shm needs to be set in read-only mode
+		 * for others.
+		 */
+		mode &= ~S_IWOTH;
+		ret = fchmod(wait_shm_fd, mode);
+		if (ret < 0) {
+			PERROR("fchmod");
+			exit(EXIT_FAILURE);
+		}
+	} else {
+		ret = fchown(wait_shm_fd, getuid(), getgid());
+		if (ret < 0) {
+			PERROR("fchown");
+			exit(EXIT_FAILURE);
+		}
 	}
 #else
-#warning "FreeBSD does not support setting file mode on shm FD. Remember that for secure use, lttng-sessiond should be started before applications linked on lttng-ust."
+#warning "FreeBSD does not support setting file mode on shm FD."
 #endif
 
 	DBG("Got the wait shm fd %d", wait_shm_fd);

[-- Attachment #3: Type: text/plain, Size: 155 bytes --]

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: Fix: application SIGBUS when starting in parallel with sessiond

[David Goulet]

[-- Attachment #1.1: Type: text/plain, Size: 3919 bytes --]

On 06 Nov (12:18:47), Mathieu Desnoyers wrote:
> Should be applied to master, stable-2.3, stable-2.2.

Merged!

> 
> Thanks,
> 
> Mathieu
> 
> -- 
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com

> commit be2bcc7e9a02e5bd78dc2615886af32ddd87032b
> Author: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Date:   Wed Nov 6 07:06:51 2013 -0500
> 
>     Fix: application SIGBUS when starting in parallel with sessiond
>     
>     There is a race between application startup and sessiond startup, where
>     there is an intermediate state where applications can SIGBUS if they see
>     a zero-sized shm, if the shm has been created, but not ftruncated yet.
>     
>     On the sessiond side, we need to ensure that the shared memory is
>     writeable by applications as long as its size is 0, which allow
>     applications to perform ftruncate and extend its size.
>     
>     On the UST side, another commit needs to ensure that UST can read the
>     shared memory file descriptor with a read() system call before they
>     try accessing it through a memory map (which triggers the SIGBUS if
>     the access goes beyond the file size)
>     
>     Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> 
> diff --git a/src/bin/lttng-sessiond/shm.c b/src/bin/lttng-sessiond/shm.c
> index b94f4eb..7bb52d4 100644
> --- a/src/bin/lttng-sessiond/shm.c
> +++ b/src/bin/lttng-sessiond/shm.c
> @@ -48,40 +48,17 @@ static int get_wait_shm(char *shm_path, size_t mmap_size, int global)
>  	/* Default permissions */
>  	mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP;
>  
> -	/* Change owner of the shm path */
> +	/*
> +	 * Change owner of the shm path.
> +	 */
>  	if (global) {
> -		ret = chown(shm_path, 0, 0);
> -		if (ret < 0) {
> -			if (errno != ENOENT) {
> -				PERROR("chown wait shm");
> -				goto error;
> -			}
> -		}
> -
>  		/*
> -		 * If global session daemon, any application can register so the shm
> -		 * needs to be set in read-only mode for others.
> +		 * If global session daemon, any application can
> +		 * register. Make it initially writeable so applications
> +		 * registering concurrently can do ftruncate() by
> +		 * themselves.
>  		 */
> -		mode |= S_IROTH;
> -	} else {
> -		ret = chown(shm_path, getuid(), getgid());
> -		if (ret < 0) {
> -			if (errno != ENOENT) {
> -				PERROR("chown wait shm");
> -				goto error;
> -			}
> -		}
> -	}
> -
> -	/*
> -	 * Set permissions to the shm even if we did not create the shm.
> -	 */
> -	ret = chmod(shm_path, mode);
> -	if (ret < 0) {
> -		if (errno != ENOENT) {
> -			PERROR("chmod wait shm");
> -			goto error;
> -		}
> +		mode |= S_IROTH | S_IWOTH;
>  	}
>  
>  	/*
> @@ -107,13 +84,32 @@ static int get_wait_shm(char *shm_path, size_t mmap_size, int global)
>  	}
>  
>  #ifndef __FreeBSD__
> -	ret = fchmod(wait_shm_fd, mode);
> -	if (ret < 0) {
> -		PERROR("fchmod");
> -		exit(EXIT_FAILURE);
> +	if (global) {
> +		ret = fchown(wait_shm_fd, 0, 0);
> +		if (ret < 0) {
> +			PERROR("fchown");
> +			exit(EXIT_FAILURE);
> +		}
> +		/*
> +		 * If global session daemon, any application can
> +		 * register so the shm needs to be set in read-only mode
> +		 * for others.
> +		 */
> +		mode &= ~S_IWOTH;
> +		ret = fchmod(wait_shm_fd, mode);
> +		if (ret < 0) {
> +			PERROR("fchmod");
> +			exit(EXIT_FAILURE);
> +		}
> +	} else {
> +		ret = fchown(wait_shm_fd, getuid(), getgid());
> +		if (ret < 0) {
> +			PERROR("fchown");
> +			exit(EXIT_FAILURE);
> +		}
>  	}
>  #else
> -#warning "FreeBSD does not support setting file mode on shm FD. Remember that for secure use, lttng-sessiond should be started before applications linked on lttng-ust."
> +#warning "FreeBSD does not support setting file mode on shm FD."
>  #endif
>  
>  	DBG("Got the wait shm fd %d", wait_shm_fd);


[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

[-- Attachment #2: Type: text/plain, Size: 155 bytes --]

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev