Skip to contentSkip to conversation

eCryptfs: Remove buggy and unnecessary write in file name decode routine

PATCH In review Michael Halcrow 2 participants 2 messages Nov 26, 2014 Ecrypt file system

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 eCryptfs: Remove buggy and unnecessary write in file name decode routine

Dmitry Chernenkov used KASAN to discover that eCryptfs writes past the end of the allocated buffer during encrypted filename decoding. This fix corrects the issue by getting rid of the unnecessary 0 write when the current bit offset is 2.

---
 fs/ecryptfs/crypto.c | 1 -
 1 file changed, 1 deletion(-)
fs/ecryptfs/crypto.c +0 -1
index 2f6735d..31b148f 100644
@@ -1917,7 +1917,6 @@ ecryptfs_decode_from_filename(unsigned char *dst, size_t *dst_size,
 			break;
 		case 2:
 			dst[dst_byte_offset++] |= (src_byte);
-			dst[dst_byte_offset] = 0;
 			current_bit_offset = 0;
 			break;
 		}
Signature · 1 line
2.2.0.rc0.207.ga3a616c
#1 lore raw
Quoted 6 lines · Michael Halcrow

Dmitry Chernenkov used KASAN to discover that eCryptfs writes past the end of the allocated buffer during encrypted filename decoding. This fix corrects the issue by getting rid of the unnecessary 0 write when the current bit offset is 2.

I like this fix better than the other where you modified ecryptfs_max_decoded_size() to return an extra byte. ecryptfs_max_decoded_size() is actually returning the correct size instead of being conservative, as the comments in that function suggest so returning an extra byte doesn't make sense.

Also, the decode operation takes blocks of 4 encoded bytes down to 3 decoded bytes so there's no reason to zero the 4th decoded byte in the sequence when there may not even be a 4th encoded byte.

So, this one gets my ack. I'll take it from here. I'll add the correct tag to get it backported to the appropriate stable kernels and also give Dmitry credit with a Reported-by tag.

Thanks!

Tyler

Quoted 19 lines
---
 fs/ecryptfs/crypto.c | 1 -
 1 file changed, 1 deletion(-)
fs/ecryptfs/crypto.c +0 -1
index 2f6735d..31b148f 100644
@@ -1917,7 +1917,6 @@ ecryptfs_decode_from_filename(unsigned char *dst, size_t *dst_size,
 			break;
 		case 2:
 			dst[dst_byte_offset++] |= (src_byte);
-			dst[dst_byte_offset] = 0;
 			current_bit_offset = 0;
 			break;
 		}
Signature · 1 line
2.2.0.rc0.207.ga3a616c