[PATCH 4/9] Add support for AMR frames to MNCC/RTP interface

[Holger Hans Peter Freyther]

On Fri, Apr 18, 2014 at 12:07:51PM +0200, Andreas Eversberg wrote:

hi,


> i had that patch done already. (see attachment)

what was the message id?  I didn't see it.

> happy easter

happy easter to you too.

> +	if (rtph->payload_type == RTP_PT_AMR) {
> +		new_msg = msgb_alloc(sizeof(struct gsm_data_frame) + 1
> +				     + payload_len, "GSM-DATA");
> +	} else {
> +		new_msg = msgb_alloc(sizeof(struct gsm_data_frame)
> +				     + payload_len, "GSM-DATA");
> +	}

I think the coding style asks us to ommit the {} here. Maybe use
different strings too in case we search for a memory leak?


> @@ -305,7 +324,13 @@ int rtp_send_frame(struct rtp_socket *rs, struct gsm_data_frame *frame)
>  	rtph->timestamp = htonl(rs->transmit.timestamp);
>  	rs->transmit.timestamp += duration;
>  	rtph->ssrc = htonl(rs->transmit.ssrc);
> -	memcpy(msg->data + sizeof(struct rtp_hdr), frame->data, payload_len);
> +	if (frame->msg_type == GSM_TCH_FRAME_AMR) {
> +		memcpy(msg->data + sizeof(struct rtp_hdr), frame->data + 1,
> +			payload_len);
> +	} else {
> +		memcpy(msg->data + sizeof(struct rtp_hdr), frame->data,
> +			payload_len);
> +	}


This lacks input validation. The code needs to check that the data
we read is within the bounds of the msgb and the data we write is within
the bounds too.