jffs2: Fix integer underflow in jffs2_rtime_compress

Message ID 20181215162350.12489-1-richard@nod.at
State New
Delegated to: Richard Weinberger
Headers show
Series
  • jffs2: Fix integer underflow in jffs2_rtime_compress
Related show

Commit Message

Richard Weinberger Dec. 15, 2018, 4:23 p.m.
The rtime compressor assumes that at least two bytes are
compressed.
If we try to compress just one byte, the loop condition will
wrap around and an out-of-bounds write happens.

Cc: <stable@vger.kernel.org>
Signed-off-by: Richard Weinberger <richard@nod.at>
---
 fs/jffs2/compr_rtime.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Hou Tao Dec. 20, 2018, 10:43 a.m. | #1
On 2018/12/16 0:23, Richard Weinberger wrote:
> The rtime compressor assumes that at least two bytes are
> compressed.
> If we try to compress just one byte, the loop condition will
> wrap around and an out-of-bounds write happens.
> 
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>  fs/jffs2/compr_rtime.c | 3 +++
>  1 file changed, 3 insertions(+)
> It seems that it doesn't incur any harm because the minimal allocated
size will be 8-bytes and jffs2_rtime_compress() will write 2-bytes into
the allocated buffer.

> diff --git a/fs/jffs2/compr_rtime.c b/fs/jffs2/compr_rtime.c
> index 406d9cc84ba8..cbf700001fc9 100644
> --- a/fs/jffs2/compr_rtime.c
> +++ b/fs/jffs2/compr_rtime.c
> @@ -39,6 +39,9 @@ static int jffs2_rtime_compress(unsigned char *data_in,
>  
>  	memset(positions,0,sizeof(positions));
>  
> +	if (*dstlen < 2)
> +		return -1;
> +
>  	while (pos < (*sourcelen) && outpos <= (*dstlen)-2) {
>  		int backpos, runlen=0;
>  		unsigned char value;
>
Richard Weinberger Dec. 20, 2018, 10:45 a.m. | #2
Am Donnerstag, 20. Dezember 2018, 11:43:08 CET schrieb Hou Tao:
> 
> On 2018/12/16 0:23, Richard Weinberger wrote:
> > The rtime compressor assumes that at least two bytes are
> > compressed.
> > If we try to compress just one byte, the loop condition will
> > wrap around and an out-of-bounds write happens.
> > 
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Richard Weinberger <richard@nod.at>
> > ---
> >  fs/jffs2/compr_rtime.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > It seems that it doesn't incur any harm because the minimal allocated
> size will be 8-bytes and jffs2_rtime_compress() will write 2-bytes into
> the allocated buffer.

Are you sure about that? I saw odd kernel behavior and KASAN complained too.

Thanks,
//richard

Patch

diff --git a/fs/jffs2/compr_rtime.c b/fs/jffs2/compr_rtime.c
index 406d9cc84ba8..cbf700001fc9 100644
--- a/fs/jffs2/compr_rtime.c
+++ b/fs/jffs2/compr_rtime.c
@@ -39,6 +39,9 @@  static int jffs2_rtime_compress(unsigned char *data_in,
 
 	memset(positions,0,sizeof(positions));
 
+	if (*dstlen < 2)
+		return -1;
+
 	while (pos < (*sourcelen) && outpos <= (*dstlen)-2) {
 		int backpos, runlen=0;
 		unsigned char value;