Thread (7 messages) 7 messages, 4 authors, 2016-08-30

Re: [f2fs-dev] [PATCH] fscrypto: fix to null-terminate encrypted filename in fname_encrypt

From: Chao Yu <hidden>
Date: 2016-08-30 16:11:17
Also in: linux-f2fs-devel, lkml

Hi Ted,

On 2016/8/30 3:08, Theodore Ts'o wrote:
On Mon, Aug 29, 2016 at 10:55:47PM +0800, Chao Yu wrote:
quoted
Hi Ted, Jaegeuk,

Since encryption functionality in ext4/f2fs was exported to vfs as fscrypot
module, more filesystems can use it, I'm not sure, maybe other fs will traverse
encrypted filename directly.

So, could we set this null character in fname_encrypt in advance in order to
avoid hitting random characters behind target filename when traversing it?
The encrypted filename is only used by the file system; it's not
anything which is visible outside of the file system --- if it does,
such as passing it to the security subsystem, it's a bug.

Secondly, remember that the encrypted filename is a binary blob, and
may contain hex 00 as part of the encrypted filename.  So ***any***
code that tries to use NULL termination for the encrypted filename by
definition is a bug.  In other words, you must use memcpy, and not
strcpy.  If you use strcpy, even if you did add a NUL character to the
end of the encrypted filename (which is a bit of a misnomer because it
is a binary blob, not an ASCII string, so NUL is really not
technically correct), there will be encrypted filenames where strcpy
will stop early, because there is a 0x00 byte in the encrypted
filename.

Hence, other file systems MUST NOT traverse the encrypted filename
directly, because treating it as a NUL-terminated string when it is
really a binary blob of bits that can include a 0x00 byte is by
definition a BUG.
Thanks for your detailed explain. :)

I just be misguided by comments of following code:

int fscrypt_fname_alloc_buffer(struct inode *inode,
				u32 ilen, struct fscrypt_str *crypto_str)
{
	unsigned int olen = fscrypt_fname_encrypted_size(inode, ilen);

	crypto_str->len = olen;
	if (olen < FS_FNAME_CRYPTO_DIGEST_SIZE * 2)
		olen = FS_FNAME_CRYPTO_DIGEST_SIZE * 2;
	/*
	 * Allocated buffer can hold one more character to null-terminate the
	 * string
	 */
	crypto_str->name = kmalloc(olen + 1, GFP_NOFS);
	if (!(crypto_str->name))
		return -ENOMEM;
	return 0;
}
EXPORT_SYMBOL(fscrypt_fname_alloc_buffer);

Thanks,
Cheers,

						- Ted

.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help