Re: [PATCHv2 01/10] monitor: Fix segmentation fault with malformed packet

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Andrei,

On Mon, Aug 11, 2014, Andrei Emeltchenko wrote:
> Do not allow to read more then buffer size.
> This fixes segmentation fault reading capture from target (apparently
> end of the trace was broken).
> ---
>  monitor/btsnoop.c    | 7 +++++++
>  src/shared/btsnoop.c | 5 +++++
>  src/shared/btsnoop.h | 2 ++
>  3 files changed, 14 insertions(+)
> 
> diff --git a/monitor/btsnoop.c b/monitor/btsnoop.c
> index fafeff8..ec19812 100644
> --- a/monitor/btsnoop.c
> +++ b/monitor/btsnoop.c
> @@ -304,6 +304,13 @@ int btsnoop_read_hci(struct timeval *tv, uint16_t *index, uint16_t *opcode,
>  	}
>  
>  	toread = be32toh(pkt.size);
> +	if (toread > MAX_PACKET_SIZE) {
> +		perror("Packet len suspicially big: %u", toread);
> +		close(btsnoop_fd);
> +		btsnoop_fd = -1;
> +		return -1;
> +	}
> +
>  	flags = be32toh(pkt.flags);
>  
>  	ts = be64toh(pkt.ts) - 0x00E03AB44A676000ll;
> diff --git a/src/shared/btsnoop.c b/src/shared/btsnoop.c
> index 17a872c..521be10 100644
> --- a/src/shared/btsnoop.c
> +++ b/src/shared/btsnoop.c
> @@ -415,6 +415,11 @@ bool btsnoop_read_hci(struct btsnoop *btsnoop, struct timeval *tv,
>  	}
>  
>  	toread = be32toh(pkt.size);
> +	if (toread > MAX_PACKET_SIZE) {
> +		btsnoop->aborted = true;
> +		return false;
> +	}
> +
>  	flags = be32toh(pkt.flags);
>  
>  	ts = be64toh(pkt.ts) - 0x00E03AB44A676000ll;

The above two chunks should probably be in separate patches. One for
shared/btsnoop and the other for btmon.

> diff --git a/src/shared/btsnoop.h b/src/shared/btsnoop.h
> index 2c55d02..9f73913 100644
> --- a/src/shared/btsnoop.h
> +++ b/src/shared/btsnoop.h
> @@ -44,6 +44,8 @@
>  #define BTSNOOP_OPCODE_SCO_TX_PKT	6
>  #define BTSNOOP_OPCODE_SCO_RX_PKT	7
>  
> +#define MAX_PACKET_SIZE (1486 + 4)

Where does this number come from? At least provide an explanation in the
form of a code comment so that the reader can determine that it is
correct. Also, you're violating the name space used by this header file.
Everything else in it is prefixed by btsnoop_* or BTSNOOP_*.

Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux