[lttng-dev] [RFC PATCH babeltrace-1.5] Fix: report truncated files while reading (v2)

Jérémie Galarneau jeremie.galarneau at efficios.com
Tue Feb 20 15:25:16 EST 2018


Merged in master, stable-1.5, and stable-1.4.

Thanks!
Jérémie

On Tue, Feb 06, 2018 at 03:40:19PM -0500, Mathieu Desnoyers wrote:
> When index files are available, this ensures we report the issue
> properly before mapping a truncated packet rather than hitting a
> SIGBUS.
> 
> For traces without index files, remove the check that was done at
> index creation, and use the new check done when mapping the packet
> instead. This ensures babeltrace can print stream data prior to the
> truncated packet for this stream.
> 
> TODO: What is _not_ done in this patch is changing handling of truncated
> packets: this should be treated as a warning rather than an error, so
> processing of other streams continue.
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> CC: Jonathan Rajotte-Julien <jonathan.rajotte-julien at efficios.com>
> CC: Jérémie Galarneau <jeremie.galarneau at efficios.com>
> ---
>  formats/ctf/ctf.c              | 44 +++++++++++++++++++-----------------------
>  include/babeltrace/ctf/types.h |  1 +
>  2 files changed, 21 insertions(+), 24 deletions(-)
> 
> diff --git a/formats/ctf/ctf.c b/formats/ctf/ctf.c
> index ab98b288..b74cddd7 100644
> --- a/formats/ctf/ctf.c
> +++ b/formats/ctf/ctf.c
> @@ -1180,6 +1180,14 @@ void ctf_packet_seek(struct bt_stream_pos *stream_pos, size_t index, int whence)
>  				goto end;
>  			}
>  		}
> +
> +		if (packet_index->packet_size > ((uint64_t) pos->file_length - packet_index->offset) * CHAR_BIT) {
> +			fprintf(stderr, "[error] Packet size (%" PRIu64 " bits) is larger than remaining file size (%" PRIu64 " bits).\n",
> +				packet_index->packet_size, ((uint64_t) pos->file_length - packet_index->offset) * CHAR_BIT);
> +			ret = -1;
> +			goto end;
> +		}
> +
>  		pos->content_size = packet_index->content_size;
>  		pos->packet_size = packet_index->packet_size;
>  		pos->mmap_offset = packet_index->offset;
> @@ -1708,8 +1716,7 @@ int stream_assign_class(struct ctf_trace *td,
>  static
>  int create_stream_one_packet_index(struct ctf_stream_pos *pos,
>  			struct ctf_trace *td,
> -			struct ctf_file_stream *file_stream,
> -			size_t filesize)
> +			struct ctf_file_stream *file_stream)
>  {
>  	struct packet_index packet_index;
>  	uint64_t stream_id = 0;
> @@ -1724,8 +1731,8 @@ begin:
>  		first_packet = 1;
>  	}
>  
> -	if (filesize - pos->mmap_offset < (packet_map_len >> LOG2_CHAR_BIT)) {
> -		packet_map_len = (filesize - pos->mmap_offset) << LOG2_CHAR_BIT;
> +	if (pos->file_length - pos->mmap_offset < (packet_map_len >> LOG2_CHAR_BIT)) {
> +		packet_map_len = (pos->file_length - pos->mmap_offset) << LOG2_CHAR_BIT;
>  	}
>  
>  	if (pos->base_mma) {
> @@ -1843,7 +1850,7 @@ begin:
>  			packet_index.packet_size = bt_get_unsigned_int(field);
>  		} else {
>  			/* Use file size for packet size */
> -			packet_index.packet_size = filesize * CHAR_BIT;
> +			packet_index.packet_size = pos->file_length * CHAR_BIT;
>  		}
>  
>  		/* read content size from header */
> @@ -1855,7 +1862,7 @@ begin:
>  			packet_index.content_size = bt_get_unsigned_int(field);
>  		} else {
>  			/* Use packet size if non-zero, else file size */
> -			packet_index.content_size = packet_index.packet_size ? : filesize * CHAR_BIT;
> +			packet_index.content_size = packet_index.packet_size ? : pos->file_length * CHAR_BIT;
>  		}
>  
>  		/* read timestamp begin from header */
> @@ -1912,9 +1919,9 @@ begin:
>  		}
>  	} else {
>  		/* Use file size for packet size */
> -		packet_index.packet_size = filesize * CHAR_BIT;
> +		packet_index.packet_size = pos->file_length * CHAR_BIT;
>  		/* Use packet size if non-zero, else file size */
> -		packet_index.content_size = packet_index.packet_size ? : filesize * CHAR_BIT;
> +		packet_index.content_size = packet_index.packet_size ? : pos->file_length * CHAR_BIT;
>  	}
>  
>  	/* Validate content size and packet size values */
> @@ -1924,12 +1931,6 @@ begin:
>  		return -EINVAL;
>  	}
>  
> -	if (packet_index.packet_size > ((uint64_t) filesize - packet_index.offset) * CHAR_BIT) {
> -		fprintf(stderr, "[error] Packet size (%" PRIu64 " bits) is larger than remaining file size (%" PRIu64 " bits).\n",
> -			packet_index.packet_size, ((uint64_t) filesize - packet_index.offset) * CHAR_BIT);
> -		return -EINVAL;
> -	}
> -
>  	if (packet_index.content_size < pos->offset) {
>  		fprintf(stderr, "[error] Invalid CTF stream: content size is smaller than packet headers.\n");
>  		return -EINVAL;
> @@ -1952,7 +1953,7 @@ begin:
>  
>  	/* Retry with larger mapping */
>  retry:
> -	if (packet_map_len == ((filesize - pos->mmap_offset) << LOG2_CHAR_BIT)) {
> +	if (packet_map_len == ((pos->file_length - pos->mmap_offset) << LOG2_CHAR_BIT)) {
>  		/*
>  		 * Reached EOF, but still expecting header/context data.
>  		 */
> @@ -1975,17 +1976,12 @@ int create_stream_packet_index(struct ctf_trace *td,
>  			struct ctf_file_stream *file_stream)
>  {
>  	struct ctf_stream_pos *pos;
> -	struct stat filestats;
>  	int ret;
>  
>  	pos = &file_stream->pos;
>  
> -	ret = fstat(pos->fd, &filestats);
> -	if (ret < 0)
> -		return ret;
> -
>  	/* Deal with empty files */
> -	if (!filestats.st_size) {
> +	if (!pos->file_length) {
>  		if (file_stream->parent.trace_packet_header
>  				|| file_stream->parent.stream_packet_context) {
>  			/*
> @@ -2008,9 +2004,8 @@ int create_stream_packet_index(struct ctf_trace *td,
>  		}
>  	}
>  
> -	for (pos->mmap_offset = 0; pos->mmap_offset < filestats.st_size; ) {
> -		ret = create_stream_one_packet_index(pos, td, file_stream,
> -			filestats.st_size);
> +	for (pos->mmap_offset = 0; pos->mmap_offset < pos->file_length; ) {
> +		ret = create_stream_one_packet_index(pos, td, file_stream);
>  		if (ret)
>  			return ret;
>  	}
> @@ -2195,6 +2190,7 @@ int ctf_open_file_stream_read(struct ctf_trace *td, const char *path, int flags,
>  	file_stream->pos.last_offset = LAST_OFFSET_POISON;
>  	file_stream->pos.fd = -1;
>  	file_stream->pos.index_fp = NULL;
> +	file_stream->pos.file_length = statbuf.st_size;
>  
>  	strncpy(file_stream->parent.path, path, PATH_MAX);
>  	file_stream->parent.path[PATH_MAX - 1] = '\0';
> diff --git a/include/babeltrace/ctf/types.h b/include/babeltrace/ctf/types.h
> index 59ce9981..0bc003c8 100644
> --- a/include/babeltrace/ctf/types.h
> +++ b/include/babeltrace/ctf/types.h
> @@ -92,6 +92,7 @@ struct ctf_stream_pos {
>  
>  	int dummy;		/* dummy position, for length calculation */
>  	struct bt_stream_callbacks *cb;	/* Callbacks registered for iterator. */
> +	size_t file_length;	/* length of backing file, in bytes. */
>  	void *priv;
>  };
>  
> -- 
> 2.11.0
> 


More information about the lttng-dev mailing list