fscrypt: don't ignore minor_hash when hash is 0
Subsystems (3)
Eric Biggers
maintainerPATCH 1/1 fscrypt: don't ignore minor_hash when hash is 0
From: Eric Biggers <[email protected]>
When initializing a no-key name, fscrypt_fname_disk_to_usr() sets the minor_hash to 0 if the (major) hash is 0.
This doesn't make sense because 0 is a valid hash code, so we shouldn't ignore the filesystem-provided minor_hash in that case. Fix this by removing the special case for 'hash == 0'.
This is an old bug that appears to have originated when the encryption code in ext4 and f2fs was moved into fs/crypto/. The original ext4 and f2fs code passed the hash by pointer instead of by value. So 'if (hash)' actually made sense then, as it was checking whether a pointer was NULL. But now the hashes are passed by value, and filesystems just pass 0 for any hashes they don't have. There is no need to handle this any differently from the hashes actually being 0.
It is difficult to reproduce this bug, as it only made a difference in the case where a filename's 32-bit major hash happened to be 0. However, it probably had the largest chance of causing problems on ubifs, since ubifs uses minor_hash to do lookups of no-key names, in addition to using it as a readdir cookie. ext4 only uses minor_hash as a readdir cookie, and f2fs doesn't use minor_hash at all.
--- fs/crypto/fname.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)
fs/crypto/fname.c +3 -7
@@ -344,13 +344,9 @@ int fscrypt_fname_disk_to_usr(const struct inode *inode, offsetof(struct fscrypt_nokey_name, sha256));BUILD_BUG_ON (BASE64_CHARS (FSCRYPT_NOKEY_NAME_MAX ) >NAME_MAX ); -if (hash) { - nokey_name.dirhash[0 ] = hash; - nokey_name.dirhash[1 ] = minor_hash; - }else { - nokey_name.dirhash[0 ] =0 ; - nokey_name.dirhash[1 ] =0 ; - } + nokey_name.dirhash[0 ] = hash; + nokey_name.dirhash[1 ] = minor_hash; +if (iname->len <=sizeof (nokey_name.bytes)) {memcpy (nokey_name.bytes, iname->name, iname->len); size = offsetof(struct fscrypt_nokey_name, bytes[iname->len]);
base-commit: c4681547bcce777daf576925a966ffa824edd09d
Signature · 1 line
2.32.0.rc0.204.g9fa02ecfa5-goog
Quoted 24 lines · Eric Biggers
From: Eric Biggers <[email protected]>
When initializing a no-key name,
fscrypt_fname_disk_to_usr()sets theminor_hashto 0 if the (major) hash is 0.This doesn't make sense because 0 is a valid hash code, so we shouldn't ignore the filesystem-provided
minor_hashin that case. Fix this by removing the special case for 'hash == 0'.This is an old bug that appears to have originated when the encryption code in ext4 and f2fs was moved into
fs/crypto/. The original ext4 and f2fs code passed the hash by pointer instead of by value. So 'if (hash)' actually made sense then, as it was checking whether a pointer was NULL. But now the hashes are passed by value, and filesystems just pass 0 for any hashes they don't have. There is no need to handle this any differently from the hashes actually being 0.It is difficult to reproduce this bug, as it only made a difference in the case where a filename's 32-bit major hash happened to be 0. However, it probably had the largest chance of causing problems on ubifs, since ubifs uses
minor_hashto do lookups of no-key names, in addition to using it as a readdir cookie. ext4 only usesminor_hashas a readdir cookie, and f2fs doesn't useminor_hashat all.
Quoted 7 lines
Fixes 0b81d0779072 ("fs crypto: move per-file encryption from f2fs tree to fs/crypto")Cc <[email protected]> # v4.6+Signed-off-by Eric Biggers <[email protected]>--- fs/crypto/fname.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)
Applied to fscrypt.git#master for 5.14.
- Eric