[lttng-dev] Fix: application SIGBUS when starting in parallel with sessiond

[lttng-dev] Fix: application SIGBUS when starting in parallel with sessiond

[Mathieu Desnoyers]

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

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-sigbus-race.patch
Type: text/x-patch
Size: 3355 bytes
Desc: not available
URL: <http://lists.lttng.org/pipermail/lttng-dev/attachments/20131106/2c748b17/attachment.bin>

[lttng-dev] Fix: application SIGBUS when starting in parallel with sessiond

[David Goulet]

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 at 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 at 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);

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: Digital signature
URL: <http://lists.lttng.org/pipermail/lttng-dev/attachments/20131106/95dff005/attachment.pgp>