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(+)
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;
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;
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
Hi Richard,
On 2018/12/20 18:45, Richard Weinberger wrote:
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.
Sorry for the later reply.
Yes. KASAN complains but it doesn't incur any harm because the minimal allocated size returned by kmalloc() will be 8-bytes.
But we better fix it, because it's bad to depend on the implementation of kmalloc().
It seems that mtd-utils has already fixed it years ago. Maybe we can use it directly ?
And your fix also looks good to me, so
Reviewed-by: Hou Tao houtao1@huawei.com
commit e8457f16306ad6e2c8708275bf42b5dfff40fffd Author: Enrico Scholz enrico.scholz@sigma-chemnitz.de Date: Thu Jun 24 15:02:40 2010 +0200
mkfs.jffs2: fix integer underflow in jffs2_rtime_compress()
When '*dstlen' is 0 or 1, comparison will return wrong result. Reported by valgrind as
==5919== Invalid write of size 1 ==5919== at 0x40564E: jffs2_rtime_compress (compr_rtime.c:51) ==5919== by 0x40676B: jffs2_compress (compr.c:246) ==5919== by 0x403EE4: recursive_populate_directory (mkfs.jffs2.c:884) ==5919== Address 0x4e1bdb1 is 0 bytes after a block of size 1 alloc'd ==5919== at 0x4A0515D: malloc (vg_replace_malloc.c:195) ==5919== by 0x40671C: jffs2_compress (compr.c:229) ==5919== by 0x403EE4: recursive_populate_directory (mkfs.jffs2.c:884)
Signed-off-by: Enrico Scholz enrico.scholz@sigma-chemnitz.de Signed-off-by: Artem Bityutskiy Artem.Bityutskiy@nokia.com
diff --git a/compr_rtime.c b/compr_rtime.c index 131536c..5613963 100644 --- a/compr_rtime.c +++ b/compr_rtime.c @@ -32,7 +32,7 @@ static int jffs2_rtime_compress(unsigned char *data_in, unsigned char *cpage_out
memset(positions,0,sizeof(positions));
- while (pos < (*sourcelen) && outpos <= (*dstlen)-2) { + while (pos < (*sourcelen) && outpos+2 <= (*dstlen)) { int backpos, runlen=0; unsigned char value;
Thanks, //richard
.
linux-stable-mirror@lists.linaro.org
-
Hou Tao -
Richard Weinberger