media: vicodec: ensure comp frame pointer kept in range
PATCH 1/1 media: vicodec: ensure comp frame pointer kept in range
Make sure that the pointer to the compressed frame does not get out of the buffer.
--- drivers/media/platform/vicodec/codec-fwht.c | 73 +++++++++++++------ drivers/media/platform/vicodec/codec-fwht.h | 8 +- .../media/platform/vicodec/codec-v4l2-fwht.c | 8 +- drivers/media/platform/vicodec/vicodec-core.c | 4 + 4 files changed, 62 insertions(+), 31 deletions(-)
drivers/media/platform/vicodec/codec-fwht.c +49 -24
@@ -104,16 +104,20 @@ static int rlc(const s16 *in, __be16 *output, int blocktype)* This function will worst-case increase rlc_in by 65*2 bytes: * one s16 value for the header and 8 * 8 coefficients of type s16. */ -static s16 derlc (const __be16 **rlc_in,s16 *dwht_out) +static int derlc (const __be16 **rlc_in,s16 *dwht_out,s16 *stat, +const __be16 *after_rlco) {/* header */ const __be16 *input = *rlc_in; -s16 ret =ntohs (*input++);int dec_count =0 ;s16 block[8 *8 +16 ];s16 *wp = block;int i; +if (input >= after_rlco) +return -1 ; + *stat =ntohs (*input++); +/* * Now de-compress, it expands one byte to up to 15 bytes * (or fills the remainder of the 64 bytes with zeroes if it
@@ -123,9 +127,15 @@ static s16 derlc(const __be16 **rlc_in, s16 *dwht_out)* allow for overflow if the incoming data was malformed. */ while (dec_count <8 *8 ) { -s16 in =ntohs (*input++); -int length = in &0xf ; -int coeff = in >>4 ; +s16 in; +int length; +int coeff; + +if (input >= after_rlco) +return -1 ; + in =ntohs (*input++); + length = in &0xf ; + coeff = in >>4 ;/* fill remainder with zeros */ if (length ==15 ) {
@@ -150,7 +160,7 @@ static s16 derlc(const __be16 **rlc_in, s16 *dwht_out) dwht_out[x + y *8 ] = *wp++; } *rlc_in = input; -return ret; +return 0 ; }static const int quant_table[] = {
@@ -808,9 +818,9 @@ u32 fwht_encode_frame(struct fwht_raw_frame *frm,return encoding; } -static void decode_plane (struct fwht_cframe *cf,const __be16 **rlco,u8 *ref, -u32 height,u32 width,u32 coded_width, -bool uncompressed) +static int decode_plane (struct fwht_cframe *cf,const __be16 **rlco,u8 *ref, +u32 height,u32 width,u32 coded_width, +bool uncompressed,const __be16 *after_rlco) {unsigned int copies =0 ;s16 copy[8 *8 ];
@@ -821,9 +831,11 @@ static void decode_plane(struct fwht_cframe *cf, const __be16 **rlco, u8 *ref, height =round_up (height,8 );if (uncompressed) { +if (after_rlco < *rlco + width * height /2 ) +return -1 ;memcpy (ref, *rlco, width * height); *rlco += width * height /2 ; -return ; +return 0 ; }/*
@@ -835,6 +847,7 @@ static void decode_plane(struct fwht_cframe *cf, const __be16 **rlco, u8 *ref,for (j =0 ; j < height /8 ; j++) {for (i =0 ; i < width /8 ; i++) {u8 *refp = ref + j *8 * coded_width + i *8 ; +int ret;if (copies) {memcpy (cf->de_fwht, copy,sizeof (copy));
@@ -847,8 +860,9 @@ static void decode_plane(struct fwht_cframe *cf, const __be16 **rlco, u8 *ref,continue ; } - stat =derlc (rlco, cf->coeffs); - + ret =derlc (rlco, cf->coeffs, &stat, after_rlco); +if (ret <0 ) +return -1 ;if (stat &PFRAME_BIT )dequantize_inter (cf->coeffs);else
@@ -865,17 +879,21 @@ static void decode_plane(struct fwht_cframe *cf, const __be16 **rlco, u8 *ref,fill_decoder_block (refp, cf->de_fwht, coded_width); } } +return 0 ; } -void fwht_decode_frame (struct fwht_cframe *cf,struct fwht_raw_frame *ref, -u32 hdr_flags,unsigned int components_num, -unsigned int width,unsigned int height, -unsigned int coded_width) +int fwht_decode_frame (struct fwht_cframe *cf,struct fwht_raw_frame *ref, +u32 hdr_flags,unsigned int components_num, +unsigned int width,unsigned int height, +unsigned int coded_width) {const __be16 *rlco = cf->rlc_data; +const __be16 *after_rlco = cf->rlc_data + (cf->size /sizeof (*rlco)); -decode_plane (cf, &rlco, ref->luma, height, width, coded_width, - hdr_flags &FWHT_FL_LUMA_IS_UNCOMPRESSED ); +if (decode_plane (cf, &rlco, ref->luma, height, width, coded_width, + hdr_flags &FWHT_FL_LUMA_IS_UNCOMPRESSED , + after_rlco) <0 ) +return -1 ;if (components_num >=3 ) {u32 h = height;
@@ -888,13 +906,20 @@ void fwht_decode_frame(struct fwht_cframe *cf, struct fwht_raw_frame *ref, w /=2 ; c /=2 ; } -decode_plane (cf, &rlco, ref->cb, h, w, c, - hdr_flags &FWHT_FL_CB_IS_UNCOMPRESSED ); -decode_plane (cf, &rlco, ref->cr, h, w, c, - hdr_flags &FWHT_FL_CR_IS_UNCOMPRESSED ); +if (decode_plane (cf, &rlco, ref->cb, h, w, c, + hdr_flags &FWHT_FL_CB_IS_UNCOMPRESSED , + after_rlco) <0 ) +return -1 ; +if (decode_plane (cf, &rlco, ref->cr, h, w, c, + hdr_flags &FWHT_FL_CR_IS_UNCOMPRESSED , + after_rlco) <0 ) +return -1 ; }if (components_num ==4 ) -decode_plane (cf, &rlco, ref->alpha, height, width, coded_width, - hdr_flags &FWHT_FL_ALPHA_IS_UNCOMPRESSED ); +if (decode_plane (cf, &rlco, ref->alpha, height, width, coded_width, + hdr_flags &FWHT_FL_ALPHA_IS_UNCOMPRESSED , + after_rlco) <0 ) +return -1 ; +return 0 ; }
drivers/media/platform/vicodec/codec-fwht.h +4 -4
@@ -139,9 +139,9 @@ u32 fwht_encode_frame(struct fwht_raw_frame *frm, bool is_intra, bool next_is_intra, unsigned int width, unsigned int height, unsigned int stride, unsigned int chroma_stride); -void fwht_decode_frame (struct fwht_cframe *cf,struct fwht_raw_frame *ref, -u32 hdr_flags,unsigned int components_num, -unsigned int width,unsigned int height, -unsigned int coded_width); +int fwht_decode_frame (struct fwht_cframe *cf,struct fwht_raw_frame *ref, +u32 hdr_flags,unsigned int components_num, +unsigned int width,unsigned int height, +unsigned int coded_width);#endif
drivers/media/platform/vicodec/codec-v4l2-fwht.c +5 -3
@@ -280,6 +280,7 @@ int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out) state->ycbcr_enc =ntohl (state->header.ycbcr_enc); state->quantization =ntohl (state->header.quantization); cf.rlc_data = (__be16 *)p_in; + cf.size =ntohl (state->header.size); hdr_width_div = (flags &FWHT_FL_CHROMA_FULL_WIDTH ) ?1 :2 ; hdr_height_div = (flags &FWHT_FL_CHROMA_FULL_HEIGHT ) ?1 :2 ;
@@ -287,9 +288,10 @@ int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out) hdr_height_div != info->height_div)return -EINVAL ; -fwht_decode_frame (&cf, &state->ref_frame, flags, components_num, - state->visible_width, state->visible_height, - state->coded_width); +if (fwht_decode_frame (&cf, &state->ref_frame, flags, components_num, + state->visible_width, state->visible_height, + state->coded_width) <0 ) +return -EINVAL ;/* * TODO - handle the case where the compressed stream encodes a
drivers/media/platform/vicodec/vicodec-core.c +4 -0
@@ -186,6 +186,10 @@ static int device_process(struct vicodec_ctx *ctx,return ret;vb2_set_plane_payload (&dst_vb->vb2_buf,0 , ret); }else { +unsigned int comp_frame_size =ntohl (ctx->state.header.size); + +if (comp_frame_size > ctx->comp_max_size) +return -EINVAL ; state->info = q_dst->info; ret =v4l2_fwht_decode (state, p_src, p_dst);if (ret <0 )
Signature · 1 line
2.17.1
Hi Dafna,
Quoted 16 lines · Dafna Hirschfeld
Make sure that the pointer to the compressed frame does not get out of the buffer.
Signed-off-by Dafna Hirschfeld <[email protected]>--- drivers/media/platform/vicodec/codec-fwht.c | 73 +++++++++++++------ drivers/media/platform/vicodec/codec-fwht.h | 8 +- .../media/platform/vicodec/codec-v4l2-fwht.c | 8 +- drivers/media/platform/vicodec/vicodec-core.c | 4 + 4 files changed, 62 insertions(+), 31 deletions(-)drivers/media/platform/vicodec/codec-fwht.c +0
-0
drivers/media/platform/vicodec/codec-fwht.c static int rlc(const s16 *in, __be16 *output, int blocktype) · line 108
Quoted 6 lines
drivers/media/platform/vicodec/codec-fwht.c +2
-1@@ -104,16 +104,20 @@ static int rlc(const s16 *in, __be16 *output, int blocktype)* This function will worst-case increase rlc_in by 65*2 bytes: * one s16 value for the header and 8 * 8 coefficients of type s16. */ -static s16 derlc (const __be16 **rlc_in,s16 *dwht_out) +static int derlc (const __be16 **rlc_in,s16 *dwht_out,s16 *stat, +const __be16 *after_rlco)
Hmm. I think this is confusing.
I would add a
#define OVERFLOW_BIT BIT(14)
and return that when a buffer overrun is detected.
I'm not sure why derlc returned a s16 instead of a u16, so let's fix that as well.
The name 'after_rlco' is, I think, very confusing. How about 'end_of_buffer' which the last valid address of the buffer (so one less than after_rlco).
Quoted 45 lines
drivers/media/platform/vicodec/codec-fwht.c +14
-5{/* header */ const __be16 *input = *rlc_in; -s16 ret =ntohs (*input++);int dec_count =0 ;s16 block[8 *8 +16 ];s16 *wp = block;int i; +if (input >= after_rlco) +return -1 ; + *stat =ntohs (*input++); +/* * Now de-compress, it expands one byte to up to 15 bytes * (or fills the remainder of the 64 bytes with zeroes if it @@ -123,9 +127,15 @@ static s16 derlc(const __be16 **rlc_in, s16 *dwht_out)* allow for overflow if the incoming data was malformed. */ while (dec_count <8 *8 ) { -s16 in =ntohs (*input++); -int length = in &0xf ; -int coeff = in >>4 ; +s16 in; +int length; +int coeff; + +if (input >= after_rlco) +return -1 ; + in =ntohs (*input++); + length = in &0xf ; + coeff = in >>4 ;/* fill remainder with zeros */ if (length ==15 ) {@@ -150,7 +160,7 @@ static s16 derlc(const __be16 **rlc_in, s16 *dwht_out) dwht_out[x + y *8 ] = *wp++; } *rlc_in = input; -return ret; +return 0 ; }static const int quant_table[] = {
drivers/media/platform/vicodec/codec-fwht.c u32 fwht_encode_frame(struct fwht_raw_frame *frm, · line 823
Quoted 9 lines
drivers/media/platform/vicodec/codec-fwht.c +3
-3@@ -808,9 +818,9 @@ u32 fwht_encode_frame(struct fwht_raw_frame *frm,return encoding; } -static void decode_plane (struct fwht_cframe *cf,const __be16 **rlco,u8 *ref, -u32 height,u32 width,u32 coded_width, -bool uncompressed) +static int decode_plane (struct fwht_cframe *cf,const __be16 **rlco,u8 *ref, +u32 height,u32 width,u32 coded_width, +bool uncompressed,const __be16 *after_rlco)
I think returning a bool here makes more sense: true == success, false == failure.
Quoted 38 lines
drivers/media/platform/vicodec/codec-fwht.c +7
-3{unsigned int copies =0 ;s16 copy[8 *8 ];@@ -821,9 +831,11 @@ static void decode_plane(struct fwht_cframe *cf, const __be16 **rlco, u8 *ref, height =round_up (height,8 );if (uncompressed) { +if (after_rlco < *rlco + width * height /2 ) +return -1 ;memcpy (ref, *rlco, width * height); *rlco += width * height /2 ; -return ; +return 0 ; }/* @@ -835,6 +847,7 @@ static void decode_plane(struct fwht_cframe *cf, const __be16 **rlco, u8 *ref,for (j =0 ; j < height /8 ; j++) {for (i =0 ; i < width /8 ; i++) {u8 *refp = ref + j *8 * coded_width + i *8 ; +int ret;if (copies) {memcpy (cf->de_fwht, copy,sizeof (copy));@@ -847,8 +860,9 @@ static void decode_plane(struct fwht_cframe *cf, const __be16 **rlco, u8 *ref,continue ; } - stat =derlc (rlco, cf->coeffs); - + ret =derlc (rlco, cf->coeffs, &stat, after_rlco); +if (ret <0 ) +return -1 ;if (stat &PFRAME_BIT )dequantize_inter (cf->coeffs);else @@ -865,17 +879,21 @@ static void decode_plane(struct fwht_cframe *cf, const __be16 **rlco, u8 *ref,fill_decoder_block (refp, cf->de_fwht, coded_width);
drivers/media/platform/vicodec/codec-fwht.c static void decode_plane(struct fwht_cframe *cf, const __be16 **rlco, u8 *ref, · line 885
Quoted 10 lines
drivers/media/platform/vicodec/codec-fwht.c +2
-4} } +return 0 ; } -void fwht_decode_frame (struct fwht_cframe *cf,struct fwht_raw_frame *ref, -u32 hdr_flags,unsigned int components_num, -unsigned int width,unsigned int height, -unsigned int coded_width) +int fwht_decode_frame(struct fwht_cframe *cf, struct fwht_raw_frame *ref,
Also bool.
Quoted 93 lines
drivers/media/platform/vicodec/codec-fwht.c +21
-8+ u32 hdr_flags, unsigned int components_num, + unsigned int width, unsigned int height, + unsigned int coded_width) { const __be16 *rlco = cf->rlc_data; +const __be16 *after_rlco = cf->rlc_data + (cf->size /sizeof (*rlco)); -decode_plane (cf, &rlco, ref->luma, height, width, coded_width, - hdr_flags &FWHT_FL_LUMA_IS_UNCOMPRESSED ); +if (decode_plane (cf, &rlco, ref->luma, height, width, coded_width, + hdr_flags &FWHT_FL_LUMA_IS_UNCOMPRESSED , + after_rlco) <0 ) +return -1 ;if (components_num >=3 ) {u32 h = height;@@ -888,13 +906,20 @@ void fwht_decode_frame(struct fwht_cframe *cf, struct fwht_raw_frame *ref, w /=2 ; c /=2 ; } -decode_plane (cf, &rlco, ref->cb, h, w, c, - hdr_flags &FWHT_FL_CB_IS_UNCOMPRESSED ); -decode_plane (cf, &rlco, ref->cr, h, w, c, - hdr_flags &FWHT_FL_CR_IS_UNCOMPRESSED ); +if (decode_plane (cf, &rlco, ref->cb, h, w, c, + hdr_flags &FWHT_FL_CB_IS_UNCOMPRESSED , + after_rlco) <0 ) +return -1 ; +if (decode_plane (cf, &rlco, ref->cr, h, w, c, + hdr_flags &FWHT_FL_CR_IS_UNCOMPRESSED , + after_rlco) <0 ) +return -1 ; }if (components_num ==4 ) -decode_plane (cf, &rlco, ref->alpha, height, width, coded_width, - hdr_flags &FWHT_FL_ALPHA_IS_UNCOMPRESSED ); +if (decode_plane (cf, &rlco, ref->alpha, height, width, coded_width, + hdr_flags &FWHT_FL_ALPHA_IS_UNCOMPRESSED , + after_rlco) <0 ) +return -1 ; +return 0 ; }drivers/media/platform/vicodec/codec-fwht.h +4
-4@@ -139,9 +139,9 @@ u32 fwht_encode_frame(struct fwht_raw_frame *frm, bool is_intra, bool next_is_intra, unsigned int width, unsigned int height, unsigned int stride, unsigned int chroma_stride); -void fwht_decode_frame (struct fwht_cframe *cf,struct fwht_raw_frame *ref, -u32 hdr_flags,unsigned int components_num, -unsigned int width,unsigned int height, -unsigned int coded_width); +int fwht_decode_frame (struct fwht_cframe *cf,struct fwht_raw_frame *ref, +u32 hdr_flags,unsigned int components_num, +unsigned int width,unsigned int height, +unsigned int coded_width);#endif drivers/media/platform/vicodec/codec-v4l2-fwht.c +5
-3@@ -280,6 +280,7 @@ int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out) state->ycbcr_enc =ntohl (state->header.ycbcr_enc); state->quantization =ntohl (state->header.quantization); cf.rlc_data = (__be16 *)p_in; + cf.size =ntohl (state->header.size); hdr_width_div = (flags &FWHT_FL_CHROMA_FULL_WIDTH ) ?1 :2 ; hdr_height_div = (flags &FWHT_FL_CHROMA_FULL_HEIGHT ) ?1 :2 ;@@ -287,9 +288,10 @@ int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out) hdr_height_div != info->height_div)return -EINVAL ; -fwht_decode_frame (&cf, &state->ref_frame, flags, components_num, - state->visible_width, state->visible_height, - state->coded_width); +if (fwht_decode_frame (&cf, &state->ref_frame, flags, components_num, + state->visible_width, state->visible_height, + state->coded_width) <0 ) +return -EINVAL ;/* * TODO - handle the case where the compressed stream encodes a drivers/media/platform/vicodec/vicodec-core.c +0
-0
drivers/media/platform/vicodec/vicodec-core.c static int device_process(struct vicodec_ctx *ctx, · line 195
Quoted 10 lines
drivers/media/platform/vicodec/vicodec-core.c +4
-0@@ -186,6 +186,10 @@ static int device_process(struct vicodec_ctx *ctx,return ret;vb2_set_plane_payload (&dst_vb->vb2_buf,0 , ret); }else { +unsigned int comp_frame_size =ntohl (ctx->state.header.size); + +if (comp_frame_size > ctx->comp_max_size) +return -EINVAL ; state->info = q_dst->info; ret =v4l2_fwht_decode (state, p_src, p_dst);if (ret <0 )
Regards,
Hans