| Message ID | 16ccf3d588665a5a0dda91cbb04374d6aea99ca6.1729074076.git.mchehab+huawei@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Media: fix several issues on drivers | expand |
Hi Mauro, On 10/16/24 12:22, Mauro Carvalho Chehab wrote: > The current logic allows word to be less than 2. If this happens, > there will be buffer overflows. Add extra checks to prevent it. > > While here, remove an unused word = 0 assignment. > > Fixes: 6c96dbbc2aa9 ("[media] s5p-jpeg: add support for 5433") > Cc: stable@vger.kernel.org > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > --- > .../media/platform/samsung/s5p-jpeg/jpeg-core.c | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/drivers/media/platform/samsung/s5p-jpeg/jpeg-core.c b/drivers/media/platform/samsung/s5p-jpeg/jpeg-core.c > index d2c4a0178b3c..1db4609b3557 100644 > --- a/drivers/media/platform/samsung/s5p-jpeg/jpeg-core.c > +++ b/drivers/media/platform/samsung/s5p-jpeg/jpeg-core.c > @@ -775,11 +775,14 @@ static void exynos4_jpeg_parse_decode_h_tbl(struct s5p_jpeg_ctx *ctx) > (unsigned long)vb2_plane_vaddr(&vb->vb2_buf, 0) + ctx->out_q.sos + 2; > jpeg_buffer.curr = 0; > > - word = 0; > - > if (get_word_be(&jpeg_buffer, &word)) > return; > - jpeg_buffer.size = (long)word - 2; > + > + if (word < 2) > + jpeg_buffer.size = 0; > + else > + jpeg_buffer.size = (long)word - 2; > + > jpeg_buffer.data += 2; > jpeg_buffer.curr = 0; > > @@ -1058,6 +1061,7 @@ static int get_word_be(struct s5p_jpeg_buffer *buf, unsigned int *word) > if (byte == -1) > return -1; > *word = (unsigned int)byte | temp; > + > return 0; > } > > @@ -1145,7 +1149,7 @@ static bool s5p_jpeg_parse_hdr(struct s5p_jpeg_q_data *result, > if (get_word_be(&jpeg_buffer, &word)) > break; > length = (long)word - 2; > - if (!length) > + if (length <= 0) > return false; > sof = jpeg_buffer.curr; /* after 0xffc0 */ > sof_len = length; > @@ -1176,7 +1180,7 @@ static bool s5p_jpeg_parse_hdr(struct s5p_jpeg_q_data *result, > if (get_word_be(&jpeg_buffer, &word)) > break; > length = (long)word - 2; > - if (!length) > + if (length <= 0) > return false; > if (n_dqt >= S5P_JPEG_MAX_MARKER) > return false; > @@ -1189,7 +1193,7 @@ static bool s5p_jpeg_parse_hdr(struct s5p_jpeg_q_data *result, > if (get_word_be(&jpeg_buffer, &word)) > break; > length = (long)word - 2; > - if (!length) > + if (length <= 0) > return false; > if (n_dht >= S5P_JPEG_MAX_MARKER) > return false; > @@ -1214,6 +1218,7 @@ static bool s5p_jpeg_parse_hdr(struct s5p_jpeg_q_data *result, > if (get_word_be(&jpeg_buffer, &word)) > break; > length = (long)word - 2; > + /* No need to check underflows as skip() does it */ > skip(&jpeg_buffer, length); > break; > } Seems reasonable. Reviewed-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
diff --git a/drivers/media/platform/samsung/s5p-jpeg/jpeg-core.c b/drivers/media/platform/samsung/s5p-jpeg/jpeg-core.c index d2c4a0178b3c..1db4609b3557 100644 --- a/drivers/media/platform/samsung/s5p-jpeg/jpeg-core.c +++ b/drivers/media/platform/samsung/s5p-jpeg/jpeg-core.c @@ -775,11 +775,14 @@ static void exynos4_jpeg_parse_decode_h_tbl(struct s5p_jpeg_ctx *ctx) (unsigned long)vb2_plane_vaddr(&vb->vb2_buf, 0) + ctx->out_q.sos + 2; jpeg_buffer.curr = 0; - word = 0; - if (get_word_be(&jpeg_buffer, &word)) return; - jpeg_buffer.size = (long)word - 2; + + if (word < 2) + jpeg_buffer.size = 0; + else + jpeg_buffer.size = (long)word - 2; + jpeg_buffer.data += 2; jpeg_buffer.curr = 0; @@ -1058,6 +1061,7 @@ static int get_word_be(struct s5p_jpeg_buffer *buf, unsigned int *word) if (byte == -1) return -1; *word = (unsigned int)byte | temp; + return 0; } @@ -1145,7 +1149,7 @@ static bool s5p_jpeg_parse_hdr(struct s5p_jpeg_q_data *result, if (get_word_be(&jpeg_buffer, &word)) break; length = (long)word - 2; - if (!length) + if (length <= 0) return false; sof = jpeg_buffer.curr; /* after 0xffc0 */ sof_len = length; @@ -1176,7 +1180,7 @@ static bool s5p_jpeg_parse_hdr(struct s5p_jpeg_q_data *result, if (get_word_be(&jpeg_buffer, &word)) break; length = (long)word - 2; - if (!length) + if (length <= 0) return false; if (n_dqt >= S5P_JPEG_MAX_MARKER) return false; @@ -1189,7 +1193,7 @@ static bool s5p_jpeg_parse_hdr(struct s5p_jpeg_q_data *result, if (get_word_be(&jpeg_buffer, &word)) break; length = (long)word - 2; - if (!length) + if (length <= 0) return false; if (n_dht >= S5P_JPEG_MAX_MARKER) return false; @@ -1214,6 +1218,7 @@ static bool s5p_jpeg_parse_hdr(struct s5p_jpeg_q_data *result, if (get_word_be(&jpeg_buffer, &word)) break; length = (long)word - 2; + /* No need to check underflows as skip() does it */ skip(&jpeg_buffer, length); break; }
The current logic allows word to be less than 2. If this happens, there will be buffer overflows. Add extra checks to prevent it. While here, remove an unused word = 0 assignment. Fixes: 6c96dbbc2aa9 ("[media] s5p-jpeg: add support for 5433") Cc: stable@vger.kernel.org Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> --- .../media/platform/samsung/s5p-jpeg/jpeg-core.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)