Re: [PATCH] eCryptfs: Remove buggy and unnecessary write in file name decode routine

[Tyler Hicks <tyhicks@canonical.com>]

[-- Attachment #1: Type: text/plain, Size: 1590 bytes --]

On 2014-11-26 09:09:16, Michael Halcrow wrote:
> 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.
> 
> Signed-off-by: Michael Halcrow <mhalcrow@google.com>

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

> ---
>  fs/ecryptfs/crypto.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
> index 2f6735d..31b148f 100644
> --- a/fs/ecryptfs/crypto.c
> +++ b/fs/ecryptfs/crypto.c
> @@ -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;
>  		}
> -- 
> 2.2.0.rc0.207.ga3a616c
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]