summaryrefslogtreecommitdiffstats
path: root/fs/crypto
Commit message (Collapse)AuthorAgeFilesLines
* fscrypt: improve a few commentsEric Biggers2021-10-252-3/+13
| | | | | | | | | Improve a few comments. These were extracted from the patch "fscrypt: add support for hardware-wrapped keys" (https://lore.kernel.org/r/20211021181608.54127-4-ebiggers@kernel.org). Link: https://lore.kernel.org/r/20211026021042.6581-1-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: allow 256-bit master keys with AES-256-XTSEric Biggers2021-09-223-17/+56
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | fscrypt currently requires a 512-bit master key when AES-256-XTS is used, since AES-256-XTS keys are 512-bit and fscrypt requires that the master key be at least as long any key that will be derived from it. However, this is overly strict because AES-256-XTS doesn't actually have a 512-bit security strength, but rather 256-bit. The fact that XTS takes twice the expected key size is a quirk of the XTS mode. It is sufficient to use 256 bits of entropy for AES-256-XTS, provided that it is first properly expanded into a 512-bit key, which HKDF-SHA512 does. Therefore, relax the check of the master key size to use the security strength of the derived key rather than the size of the derived key (except for v1 encryption policies, which don't use HKDF). Besides making things more flexible for userspace, this is needed in order for the use of a KDF which only takes a 256-bit key to be introduced into the fscrypt key hierarchy. This will happen with hardware-wrapped keys support, as all known hardware which supports that feature uses an SP800-108 KDF using AES-256-CMAC, so the wrapped keys are wrapped 256-bit AES keys. Moreover, there is interest in fscrypt supporting the same type of AES-256-CMAC based KDF in software as an alternative to HKDF-SHA512. There is no security problem with such features, so fix the key length check to work properly with them. Reviewed-by: Paul Crowley <paulcrowley@google.com> Link: https://lore.kernel.org/r/20210921030303.5598-1-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: clean up comments in bio.cEric Biggers2021-09-201-15/+17
| | | | | | | | | The file comment in bio.c is almost completely irrelevant to the actual contents of the file; it was originally copied from crypto.c. Fix it up, and also add a kerneldoc comment for fscrypt_decrypt_bio(). Link: https://lore.kernel.org/r/20210909190737.140841-1-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: remove fscrypt_operations::max_namelenEric Biggers2021-09-201-2/+1
| | | | | | | | | The max_namelen field is unnecessary, as it is set to 255 (NAME_MAX) on all filesystems that support fscrypt (or plan to support fscrypt). For simplicity, just use NAME_MAX directly instead. Link: https://lore.kernel.org/r/20210909184513.139281-1-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: align Base64 encoding with RFC 4648 base64urlEric Biggers2021-07-251-41/+65
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | fscrypt uses a Base64 encoding to encode no-key filenames (the filenames that are presented to userspace when a directory is listed without its encryption key). There are many variants of Base64, but the most common ones are specified by RFC 4648. fscrypt can't use the regular RFC 4648 "base64" variant because "base64" uses the '/' character, which isn't allowed in filenames. However, RFC 4648 also specifies a "base64url" variant for use in URLs and filenames. "base64url" is less common than "base64", but it's still implemented in many programming libraries. Unfortunately, what fscrypt actually uses is a custom Base64 variant that differs from "base64url" in several ways: - The binary data is divided into 6-bit chunks differently. - Values 62 and 63 are encoded with '+' and ',' instead of '-' and '_'. - '='-padding isn't used. This isn't a problem per se, as the padding isn't technically necessary, and RFC 4648 doesn't strictly require it. But it needs to be properly documented. There have been two attempts to copy the fscrypt Base64 code into lib/ (https://lkml.kernel.org/r/20200821182813.52570-6-jlayton@kernel.org and https://lkml.kernel.org/r/20210716110428.9727-5-hare@suse.de), and both have been caught up by the fscrypt Base64 variant being nonstandard and not properly documented. Also, the planned use of the fscrypt Base64 code in the CephFS storage back-end will prevent it from being changed later (whereas currently it can still be changed), so we need to choose an encoding that we're happy with before it's too late. Therefore, switch the fscrypt Base64 variant to base64url, in order to align more closely with RFC 4648 and other implementations and uses of Base64. However, I opted not to implement '='-padding, as '='-padding adds complexity, is unnecessary, and isn't required by the RFC. Link: https://lore.kernel.org/r/20210718000125.59701-1-ebiggers@kernel.org Reviewed-by: Hannes Reinecke <hare@suse.de> Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: add fscrypt_symlink_getattr() for computing st_sizeEric Biggers2021-07-251-0/+44
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a helper function fscrypt_symlink_getattr() which will be called from the various filesystems' ->getattr() methods to read and decrypt the target of encrypted symlinks in order to report the correct st_size. Detailed explanation: As required by POSIX and as documented in various man pages, st_size for a symlink is supposed to be the length of the symlink target. Unfortunately, st_size has always been wrong for encrypted symlinks because st_size is populated from i_size from disk, which intentionally contains the length of the encrypted symlink target. That's slightly greater than the length of the decrypted symlink target (which is the symlink target that userspace usually sees), and usually won't match the length of the no-key encoded symlink target either. This hadn't been fixed yet because reporting the correct st_size would require reading the symlink target from disk and decrypting or encoding it, which historically has been considered too heavyweight to do in ->getattr(). Also historically, the wrong st_size had only broken a test (LTP lstat03) and there were no known complaints from real users. (This is probably because the st_size of symlinks isn't used too often, and when it is, typically it's for a hint for what buffer size to pass to readlink() -- which a slightly-too-large size still works for.) However, a couple things have changed now. First, there have recently been complaints about the current behavior from real users: - Breakage in rpmbuild: https://github.com/rpm-software-management/rpm/issues/1682 https://github.com/google/fscrypt/issues/305 - Breakage in toybox cpio: https://www.mail-archive.com/toybox@lists.landley.net/msg07193.html - Breakage in libgit2: https://issuetracker.google.com/issues/189629152 (on Android public issue tracker, requires login) Second, we now cache decrypted symlink targets in ->i_link. Therefore, taking the performance hit of reading and decrypting the symlink target in ->getattr() wouldn't be as big a deal as it used to be, since usually it will just save having to do the same thing later. Also note that eCryptfs ended up having to read and decrypt symlink targets in ->getattr() as well, to fix this same issue; see commit 3a60a1686f0d ("eCryptfs: Decrypt symlink target for stat size"). So, let's just bite the bullet, and read and decrypt the symlink target in ->getattr() in order to report the correct st_size. Add a function fscrypt_symlink_getattr() which the filesystems will call to do this. (Alternatively, we could store the decrypted size of symlinks on-disk. But there isn't a great place to do so, and encryption is meant to hide the original size to some extent; that property would be lost.) Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20210702065350.209646-2-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: fix derivation of SipHash keys on big endian CPUsEric Biggers2021-06-051-8/+32
| | | | | | | | | | | | | | | | | | | | | | | | | Typically, the cryptographic APIs that fscrypt uses take keys as byte arrays, which avoids endianness issues. However, siphash_key_t is an exception. It is defined as 'u64 key[2];', i.e. the 128-bit key is expected to be given directly as two 64-bit words in CPU endianness. fscrypt_derive_dirhash_key() and fscrypt_setup_iv_ino_lblk_32_key() forgot to take this into account. Therefore, the SipHash keys used to index encrypted+casefolded directories differ on big endian vs. little endian platforms, as do the SipHash keys used to hash inode numbers for IV_INO_LBLK_32-encrypted directories. This makes such directories non-portable between these platforms. Fix this by always using the little endian order. This is a breaking change for big endian platforms, but this should be fine in practice since these features (encrypt+casefold support, and the IV_INO_LBLK_32 flag) aren't known to actually be used on any big endian platforms yet. Fixes: aa408f835d02 ("fscrypt: derive dirhash key for casefolded directories") Fixes: e3b1078bedd3 ("fscrypt: add support for IV_INO_LBLK_32 policies") Cc: <stable@vger.kernel.org> # v5.6+ Link: https://lore.kernel.org/r/20210605075033.54424-1-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: don't ignore minor_hash when hash is 0Eric Biggers2021-06-051-7/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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. Fixes: 0b81d0779072 ("fs crypto: move per-file encryption from f2fs tree to fs/crypto") Cc: <stable@vger.kernel.org> # v4.6+ Link: https://lore.kernel.org/r/20210527235236.2376556-1-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* Merge branch 'linus' of ↵Linus Torvalds2021-04-261-8/+22
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6 Pull crypto updates from Herbert Xu: "API: - crypto_destroy_tfm now ignores errors as well as NULL pointers Algorithms: - Add explicit curve IDs in ECDH algorithm names - Add NIST P384 curve parameters - Add ECDSA Drivers: - Add support for Green Sardine in ccp - Add ecdh/curve25519 to hisilicon/hpre - Add support for AM64 in sa2ul" * 'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6: (184 commits) fsverity: relax build time dependency on CRYPTO_SHA256 fscrypt: relax Kconfig dependencies for crypto API algorithms crypto: camellia - drop duplicate "depends on CRYPTO" crypto: s5p-sss - consistently use local 'dev' variable in probe() crypto: s5p-sss - remove unneeded local variable initialization crypto: s5p-sss - simplify getting of_device_id match data ccp: ccp - add support for Green Sardine crypto: ccp - Make ccp_dev_suspend and ccp_dev_resume void functions crypto: octeontx2 - add support for OcteonTX2 98xx CPT block. crypto: chelsio/chcr - Remove useless MODULE_VERSION crypto: ux500/cryp - Remove duplicate argument crypto: chelsio - remove unused function crypto: sa2ul - Add support for AM64 crypto: sa2ul - Support for per channel coherency dt-bindings: crypto: ti,sa2ul: Add new compatible for AM64 crypto: hisilicon - enable new error types for QM crypto: hisilicon - add new error type for SEC crypto: hisilicon - support new error types for ZIP crypto: hisilicon - dynamic configuration 'err_info' crypto: doc - fix kernel-doc notation in chacha.c and af_alg.c ...
| * fscrypt: relax Kconfig dependencies for crypto API algorithmsArd Biesheuvel2021-04-221-8/+22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Even if FS encryption has strict functional dependencies on various crypto algorithms and chaining modes. those dependencies could potentially be satisified by other implementations than the generic ones, and no link time dependency exists on the 'depends on' claused defined by CONFIG_FS_ENCRYPTION_ALGS. So let's relax these clauses to 'imply', so that the default behavior is still to pull in those generic algorithms, but in a way that permits them to be disabled again in Kconfig. Signed-off-by: Ard Biesheuvel <ardb@kernel.org> Acked-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* | block: rename BIO_MAX_PAGES to BIO_MAX_VECSChristoph Hellwig2021-03-111-3/+3
|/ | | | | | | | | | | | Ever since the addition of multipage bio_vecs BIO_MAX_PAGES has been horribly confusingly misnamed. Rename it to BIO_MAX_VECS to stop confusing users of the bio API. Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com> Link: https://lore.kernel.org/r/20210311110137.1132391-2-hch@lst.de Signed-off-by: Jens Axboe <axboe@kernel.dk>
* inode: make init and permission helpers idmapped mount awareChristian Brauner2021-01-241-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | The inode_owner_or_capable() helper determines whether the caller is the owner of the inode or is capable with respect to that inode. Allow it to handle idmapped mounts. If the inode is accessed through an idmapped mount it according to the mount's user namespace. Afterwards the checks are identical to non-idmapped mounts. If the initial user namespace is passed nothing changes so non-idmapped mounts will see identical behavior as before. Similarly, allow the inode_init_owner() helper to handle idmapped mounts. It initializes a new inode on idmapped mounts by mapping the fsuid and fsgid of the caller from the mount's user namespace. If the initial user namespace is passed nothing changes so non-idmapped mounts will see identical behavior as before. Link: https://lore.kernel.org/r/20210121131959.646623-7-christian.brauner@ubuntu.com Cc: Christoph Hellwig <hch@lst.de> Cc: David Howells <dhowells@redhat.com> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: linux-fsdevel@vger.kernel.org Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: James Morris <jamorris@linux.microsoft.com> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
* Merge tag 'f2fs-for-5.11-rc1' of ↵Linus Torvalds2020-12-173-6/+0
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs Pull f2fs updates from Jaegeuk Kim: "In this round, we've made more work into per-file compression support. For example, F2FS_IOC_GET | SET_COMPRESS_OPTION provides a way to change the algorithm or cluster size per file. F2FS_IOC_COMPRESS | DECOMPRESS_FILE provides a way to compress and decompress the existing normal files manually. There is also a new mount option, compress_mode=fs|user, which can control who compresses the data. Chao also added a checksum feature with a mount option so that we are able to detect any corrupted cluster. In addition, Daniel contributed casefolding with encryption patch, which will be used for Android devices. Summary: Enhancements: - add ioctls and mount option to manage per-file compression feature - support casefolding with encryption - support checksum for compressed cluster - avoid IO starvation by replacing mutex with rwsem - add sysfs, max_io_bytes, to control max bio size Bug fixes: - fix use-after-free issue when compression and fsverity are enabled - fix consistency corruption during fault injection test - fix data offset for lseek - get rid of buffer_head which has 32bits limit in fiemap - fix some bugs in multi-partitions support - fix nat entry count calculation in shrinker - fix some stat information And, we've refactored some logics and fix minor bugs as well" * tag 'f2fs-for-5.11-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs: (36 commits) f2fs: compress: fix compression chksum f2fs: fix shift-out-of-bounds in sanity_check_raw_super() f2fs: fix race of pending_pages in decompression f2fs: fix to account inline xattr correctly during recovery f2fs: inline: fix wrong inline inode stat f2fs: inline: correct comment in f2fs_recover_inline_data f2fs: don't check PAGE_SIZE again in sanity_check_raw_super() f2fs: convert to F2FS_*_INO macro f2fs: introduce max_io_bytes, a sysfs entry, to limit bio size f2fs: don't allow any writes on readonly mount f2fs: avoid race condition for shrinker count f2fs: add F2FS_IOC_DECOMPRESS_FILE and F2FS_IOC_COMPRESS_FILE f2fs: add compress_mode mount option f2fs: Remove unnecessary unlikely() f2fs: init dirty_secmap incorrectly f2fs: remove buffer_head which has 32bits limit f2fs: fix wrong block count instead of bytes f2fs: use new conversion functions between blks and bytes f2fs: rename logical_to_blk and blk_to_logical f2fs: fix kbytes written stat for multi-device case ...
| * fscrypt: Have filesystems handle their d_opsDaniel Rosenberg2020-12-023-6/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This shifts the responsibility of setting up dentry operations from fscrypt to the individual filesystems, allowing them to have their own operations while still setting fscrypt's d_revalidate as appropriate. Most filesystems can just use generic_set_encrypted_ci_d_ops, unless they have their own specific dentry operations as well. That operation will set the minimal d_ops required under the circumstances. Since the fscrypt d_ops are set later on, we must set all d_ops there, since we cannot adjust those later on. This should not result in any change in behavior. Signed-off-by: Daniel Rosenberg <drosen@google.com> Acked-by: Theodore Ts'o <tytso@mit.edu> Acked-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
* | Merge branch 'linus' of ↵Linus Torvalds2020-12-142-2/+2
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6 Pull crypto updates from Herbert Xu: "API: - Add speed testing on 1420-byte blocks for networking Algorithms: - Improve performance of chacha on ARM for network packets - Improve performance of aegis128 on ARM for network packets Drivers: - Add support for Keem Bay OCS AES/SM4 - Add support for QAT 4xxx devices - Enable crypto-engine retry mechanism in caam - Enable support for crypto engine on sdm845 in qce - Add HiSilicon PRNG driver support" * 'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6: (161 commits) crypto: qat - add capability detection logic in qat_4xxx crypto: qat - add AES-XTS support for QAT GEN4 devices crypto: qat - add AES-CTR support for QAT GEN4 devices crypto: atmel-i2c - select CONFIG_BITREVERSE crypto: hisilicon/trng - replace atomic_add_return() crypto: keembay - Add support for Keem Bay OCS AES/SM4 dt-bindings: Add Keem Bay OCS AES bindings crypto: aegis128 - avoid spurious references crypto_aegis128_update_simd crypto: seed - remove trailing semicolon in macro definition crypto: x86/poly1305 - Use TEST %reg,%reg instead of CMP $0,%reg crypto: x86/sha512 - Use TEST %reg,%reg instead of CMP $0,%reg crypto: aesni - Use TEST %reg,%reg instead of CMP $0,%reg crypto: cpt - Fix sparse warnings in cptpf hwrng: ks-sa - Add dependency on IOMEM and OF crypto: lib/blake2s - Move selftest prototype into header file crypto: arm/aes-ce - work around Cortex-A57/A72 silion errata crypto: ecdh - avoid unaligned accesses in ecdh_set_secret() crypto: ccree - rework cache parameters handling crypto: cavium - Use dma_set_mask_and_coherent to simplify code crypto: marvell/octeontx - Use dma_set_mask_and_coherent to simplify code ...
| * | crypto: sha - split sha.h into sha1.h and sha2.hEric Biggers2020-11-202-2/+2
| |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently <crypto/sha.h> contains declarations for both SHA-1 and SHA-2, and <crypto/sha3.h> contains declarations for SHA-3. This organization is inconsistent, but more importantly SHA-1 is no longer considered to be cryptographically secure. So to the extent possible, SHA-1 shouldn't be grouped together with any of the other SHA versions, and usage of it should be phased out. Therefore, split <crypto/sha.h> into two headers <crypto/sha1.h> and <crypto/sha2.h>, and make everyone explicitly specify whether they want the declarations for SHA-1, SHA-2, or both. This avoids making the SHA-1 declarations visible to files that don't want anything to do with SHA-1. It also prepares for potentially moving sha1.h into a new insecure/ or dangerous/ directory. Signed-off-by: Eric Biggers <ebiggers@google.com> Acked-by: Ard Biesheuvel <ardb@kernel.org> Acked-by: Jason A. Donenfeld <Jason@zx2c4.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* | fscrypt: allow deleting files with unsupported encryption policyEric Biggers2020-12-025-16/+41
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently it's impossible to delete files that use an unsupported encryption policy, as the kernel will just return an error when performing any operation on the top-level encrypted directory, even just a path lookup into the directory or opening the directory for readdir. More specifically, this occurs in any of the following cases: - The encryption context has an unrecognized version number. Current kernels know about v1 and v2, but there could be more versions in the future. - The encryption context has unrecognized encryption modes (FSCRYPT_MODE_*) or flags (FSCRYPT_POLICY_FLAG_*), an unrecognized combination of modes, or reserved bits set. - The encryption key has been added and the encryption modes are recognized but aren't available in the crypto API -- for example, a directory is encrypted with FSCRYPT_MODE_ADIANTUM but the kernel doesn't have CONFIG_CRYPTO_ADIANTUM enabled. It's desirable to return errors for most operations on files that use an unsupported encryption policy, but the current behavior is too strict. We need to allow enough to delete files, so that people can't be stuck with undeletable files when downgrading kernel versions. That includes allowing directories to be listed and allowing dentries to be looked up. Fix this by modifying the key setup logic to treat an unsupported encryption policy in the same way as "key unavailable" in the cases that are required for a recursive delete to work: preparing for a readdir or a dentry lookup, revalidating a dentry, or checking whether an inode has the same encryption policy as its parent directory. Reviewed-by: Andreas Dilger <adilger@dilger.ca> Link: https://lore.kernel.org/r/20201203022041.230976-10-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* | fscrypt: unexport fscrypt_get_encryption_info()Eric Biggers2020-12-022-1/+2
| | | | | | | | | | | | | | | | | | | | | | Now that fscrypt_get_encryption_info() is only called from files in fs/crypto/ (due to all key setup now being handled by higher-level helper functions instead of directly by filesystems), unexport it and move its declaration to fscrypt_private.h. Reviewed-by: Andreas Dilger <adilger@dilger.ca> Link: https://lore.kernel.org/r/20201203022041.230976-9-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* | fscrypt: move fscrypt_require_key() to fscrypt_private.hEric Biggers2020-12-021-0/+26
| | | | | | | | | | | | | | | | | | | | fscrypt_require_key() is now only used by files in fs/crypto/. So reduce its visibility to fscrypt_private.h. This is also a prerequsite for unexporting fscrypt_get_encryption_info(). Reviewed-by: Andreas Dilger <adilger@dilger.ca> Link: https://lore.kernel.org/r/20201203022041.230976-8-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* | fscrypt: move body of fscrypt_prepare_setattr() out-of-lineEric Biggers2020-12-021-0/+8
| | | | | | | | | | | | | | | | | | | | In preparation for reducing the visibility of fscrypt_require_key() by moving it to fscrypt_private.h, move the call to it from fscrypt_prepare_setattr() to an out-of-line function. Reviewed-by: Andreas Dilger <adilger@dilger.ca> Link: https://lore.kernel.org/r/20201203022041.230976-7-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* | fscrypt: introduce fscrypt_prepare_readdir()Eric Biggers2020-12-021-0/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The last remaining use of fscrypt_get_encryption_info() from filesystems is for readdir (->iterate_shared()). Every other call is now in fs/crypto/ as part of some other higher-level operation. We need to add a new argument to fscrypt_get_encryption_info() to indicate whether the encryption policy is allowed to be unrecognized or not. Doing this is easier if we can work with high-level operations rather than direct filesystem use of fscrypt_get_encryption_info(). So add a function fscrypt_prepare_readdir() which wraps the call to fscrypt_get_encryption_info() for the readdir use case. Reviewed-by: Andreas Dilger <adilger@dilger.ca> Link: https://lore.kernel.org/r/20201203022041.230976-6-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* | fscrypt: simplify master key lockingEric Biggers2020-11-244-34/+21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The stated reasons for separating fscrypt_master_key::mk_secret_sem from the standard semaphore contained in every 'struct key' no longer apply. First, due to commit a992b20cd4ee ("fscrypt: add fscrypt_prepare_new_inode() and fscrypt_set_context()"), fscrypt_get_encryption_info() is no longer called from within a filesystem transaction. Second, due to commit d3ec10aa9581 ("KEYS: Don't write out to userspace while holding key semaphore"), the semaphore for the "keyring" key type no longer ranks above page faults. That leaves performance as the only possible reason to keep the separate mk_secret_sem. Specifically, having mk_secret_sem reduces the contention between setup_file_encryption_key() and FS_IOC_{ADD,REMOVE}_ENCRYPTION_KEY. However, these ioctls aren't executed often, so this doesn't seem to be worth the extra complexity. Therefore, simplify the locking design by just using key->sem instead of mk_secret_sem. Link: https://lore.kernel.org/r/20201117032626.320275-1-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* | fscrypt: remove unnecessary calls to fscrypt_require_key()Eric Biggers2020-11-241-18/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In an encrypted directory, a regular dentry (one that doesn't have the no-key name flag) can only be created if the directory's encryption key is available. Therefore the calls to fscrypt_require_key() in __fscrypt_prepare_link() and __fscrypt_prepare_rename() are unnecessary, as these functions already check that the dentries they're given aren't no-key names. Remove these unnecessary calls to fscrypt_require_key(). Link: https://lore.kernel.org/r/20201118075609.120337-6-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* | fscrypt: add fscrypt_is_nokey_name()Eric Biggers2020-11-241-2/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It's possible to create a duplicate filename in an encrypted directory by creating a file concurrently with adding the encryption key. Specifically, sys_open(O_CREAT) (or sys_mkdir(), sys_mknod(), or sys_symlink()) can lookup the target filename while the directory's encryption key hasn't been added yet, resulting in a negative no-key dentry. The VFS then calls ->create() (or ->mkdir(), ->mknod(), or ->symlink()) because the dentry is negative. Normally, ->create() would return -ENOKEY due to the directory's key being unavailable. However, if the key was added between the dentry lookup and ->create(), then the filesystem will go ahead and try to create the file. If the target filename happens to already exist as a normal name (not a no-key name), a duplicate filename may be added to the directory. In order to fix this, we need to fix the filesystems to prevent ->create(), ->mkdir(), ->mknod(), and ->symlink() on no-key names. (->rename() and ->link() need it too, but those are already handled correctly by fscrypt_prepare_rename() and fscrypt_prepare_link().) In preparation for this, add a helper function fscrypt_is_nokey_name() that filesystems can use to do this check. Use this helper function for the existing checks that fs/crypto/ does for rename and link. Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20201118075609.120337-2-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* | fscrypt: remove kernel-internal constants from UAPI headerEric Biggers2020-11-164-6/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | There isn't really any valid reason to use __FSCRYPT_MODE_MAX or FSCRYPT_POLICY_FLAGS_VALID in a userspace program. These constants are only meant to be used by the kernel internally, and they are defined in the UAPI header next to the mode numbers and flags only so that kernel developers don't forget to update them when adding new modes or flags. In https://lkml.kernel.org/r/20201005074133.1958633-2-satyat@google.com there was an example of someone wanting to use __FSCRYPT_MODE_MAX in a user program, and it was wrong because the program would have broken if __FSCRYPT_MODE_MAX were ever increased. So having this definition available is harmful. FSCRYPT_POLICY_FLAGS_VALID has the same problem. So, remove these definitions from the UAPI header. Replace FSCRYPT_POLICY_FLAGS_VALID with just listing the valid flags explicitly in the one kernel function that needs it. Move __FSCRYPT_MODE_MAX to fscrypt_private.h, remove the double underscores (which were only present to discourage use by userspace), and add a BUILD_BUG_ON() and comments to (hopefully) ensure it is kept in sync. Keep the old name FS_POLICY_FLAGS_VALID, since it's been around for longer and there's a greater chance that removing it would break source compatibility with some program. Indeed, mtd-utils is using it in an #ifdef, and removing it would introduce compiler warnings (about FS_POLICY_FLAGS_PAD_* being redefined) into the mtd-utils build. However, reduce its value to 0x07 so that it only includes the flags with old names (the ones present before Linux 5.4), and try to make it clear that it's now "frozen" and no new flags should be added to it. Fixes: 2336d0deb2d4 ("fscrypt: use FSCRYPT_ prefix for uapi constants") Cc: <stable@vger.kernel.org> # v5.4+ Link: https://lore.kernel.org/r/20201024005132.495952-1-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* | fscrypt: fix inline encryption not used on new filesEric Biggers2020-11-111-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The new helper function fscrypt_prepare_new_inode() runs before S_ENCRYPTED has been set on the new inode. This accidentally made fscrypt_select_encryption_impl() never enable inline encryption on newly created files, due to its use of fscrypt_needs_contents_encryption() which only returns true when S_ENCRYPTED is set. Fix this by using S_ISREG() directly instead of fscrypt_needs_contents_encryption(), analogous to what select_encryption_mode() does. I didn't notice this earlier because by design, the user-visible behavior is the same (other than performance, potentially) regardless of whether inline encryption is used or not. Fixes: a992b20cd4ee ("fscrypt: add fscrypt_prepare_new_inode() and fscrypt_set_context()") Reviewed-by: Satya Tangirala <satyat@google.com> Link: https://lore.kernel.org/r/20201111015224.303073-1-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* | fscrypt: remove reachable WARN in fscrypt_setup_iv_ino_lblk_32_key()Eric Biggers2020-11-061-3/+1
|/ | | | | | | | | | | | | | | | | I_CREATING isn't actually set until the inode has been assigned an inode number and inserted into the inode hash table. So the WARN_ON() in fscrypt_setup_iv_ino_lblk_32_key() is wrong, and it can trigger when creating an encrypted file on ext4. Remove it. This was sometimes causing xfstest generic/602 to fail on ext4. I didn't notice it before because due to a separate oversight, new inodes that haven't been assigned an inode number yet don't necessarily have i_ino == 0 as I had thought, so by chance I never saw the test fail. Fixes: a992b20cd4ee ("fscrypt: add fscrypt_prepare_new_inode() and fscrypt_set_context()") Reported-by: Theodore Y. Ts'o <tytso@mit.edu> Link: https://lore.kernel.org/r/20201031004556.87862-1-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: export fscrypt_d_revalidate()Eric Biggers2020-09-281-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | Dentries that represent no-key names must have a dentry_operations that includes fscrypt_d_revalidate(). Currently, this is handled by fscrypt_prepare_lookup() installing fscrypt_d_ops. However, ceph support for encryption (https://lore.kernel.org/r/20200914191707.380444-1-jlayton@kernel.org) can't use fscrypt_d_ops, since ceph already has its own dentry_operations. Similarly, ext4 and f2fs support for directories that are both encrypted and casefolded (https://lore.kernel.org/r/20200923010151.69506-1-drosen@google.com) can't use fscrypt_d_ops either, since casefolding requires some dentry operations too. To satisfy both users, we need to move the responsibility of installing the dentry_operations to filesystems. In preparation for this, export fscrypt_d_revalidate() and give it a !CONFIG_FS_ENCRYPTION stub. Reviewed-by: Jeff Layton <jlayton@kernel.org> Link: https://lore.kernel.org/r/20200924054721.187797-1-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: rename DCACHE_ENCRYPTED_NAME to DCACHE_NOKEY_NAMEEric Biggers2020-09-232-5/+4
| | | | | | | | | | | | | | | | | Originally we used the term "encrypted name" or "ciphertext name" to mean the encoded filename that is shown when an encrypted directory is listed without its key. But these terms are ambiguous since they also mean the filename stored on-disk. "Encrypted name" is especially ambiguous since it could also be understood to mean "this filename is encrypted on-disk", similar to "encrypted file". So we've started calling these encoded names "no-key names" instead. Therefore, rename DCACHE_ENCRYPTED_NAME to DCACHE_NOKEY_NAME to avoid confusion about what this flag means. Link: https://lore.kernel.org/r/20200924042624.98439-3-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: don't call no-key names "ciphertext names"Eric Biggers2020-09-232-11/+11
| | | | | | | | | | | | | | | Currently we're using the term "ciphertext name" ambiguously because it can mean either the actual ciphertext filename, or the encoded filename that is shown when an encrypted directory is listed without its key. The latter we're now usually calling the "no-key name"; and while it's derived from the ciphertext name, it's not the same thing. To avoid this ambiguity, rename fscrypt_name::is_ciphertext_name to fscrypt_name::is_nokey_name, and update comments that say "ciphertext name" (or "encrypted name") to say "no-key name" instead when warranted. Link: https://lore.kernel.org/r/20200924042624.98439-2-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: use sha256() instead of open codingEric Biggers2020-09-221-16/+7
| | | | | | | | | Now that there's a library function that calculates the SHA-256 digest of a buffer in one step, use it instead of sha256_init() + sha256_update() + sha256_final(). Link: https://lore.kernel.org/r/20200917045341.324996-1-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: make fscrypt_set_test_dummy_encryption() take a 'const char *'Eric Biggers2020-09-221-14/+6
| | | | | | | | | | | | | | | | | fscrypt_set_test_dummy_encryption() requires that the optional argument to the test_dummy_encryption mount option be specified as a substring_t. That doesn't work well with filesystems that use the new mount API, since the new way of parsing mount options doesn't use substring_t. Make it take the argument as a 'const char *' instead. Instead of moving the match_strdup() into the callers in ext4 and f2fs, make them just use arg->from directly. Since the pattern is "test_dummy_encryption=%s", the argument will be null-terminated. Acked-by: Jeff Layton <jlayton@kernel.org> Link: https://lore.kernel.org/r/20200917041136.178600-14-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: handle test_dummy_encryption in more logical wayEric Biggers2020-09-225-93/+100
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The behavior of the test_dummy_encryption mount option is that when a new file (or directory or symlink) is created in an unencrypted directory, it's automatically encrypted using a dummy encryption policy. That's it; in particular, the encryption (or lack thereof) of existing files (or directories or symlinks) doesn't change. Unfortunately the implementation of test_dummy_encryption is a bit weird and confusing. When test_dummy_encryption is enabled and a file is being created in an unencrypted directory, we set up an encryption key (->i_crypt_info) for the directory. This isn't actually used to do any encryption, however, since the directory is still unencrypted! Instead, ->i_crypt_info is only used for inheriting the encryption policy. One consequence of this is that the filesystem ends up providing a "dummy context" (policy + nonce) instead of a "dummy policy". In commit ed318a6cc0b6 ("fscrypt: support test_dummy_encryption=v2"), I mistakenly thought this was required. However, actually the nonce only ends up being used to derive a key that is never used. Another consequence of this implementation is that it allows for 'inode->i_crypt_info != NULL && !IS_ENCRYPTED(inode)', which is an edge case that can be forgotten about. For example, currently FS_IOC_GET_ENCRYPTION_POLICY on an unencrypted directory may return the dummy encryption policy when the filesystem is mounted with test_dummy_encryption. That seems like the wrong thing to do, since again, the directory itself is not actually encrypted. Therefore, switch to a more logical and maintainable implementation where the dummy encryption policy inheritance is done without setting up keys for unencrypted directories. This involves: - Adding a function fscrypt_policy_to_inherit() which returns the encryption policy to inherit from a directory. This can be a real policy, a dummy policy, or no policy. - Replacing struct fscrypt_dummy_context, ->get_dummy_context(), etc. with struct fscrypt_dummy_policy, ->get_dummy_policy(), etc. - Making fscrypt_fname_encrypted_size() take an fscrypt_policy instead of an inode. Acked-by: Jaegeuk Kim <jaegeuk@kernel.org> Acked-by: Jeff Layton <jlayton@kernel.org> Link: https://lore.kernel.org/r/20200917041136.178600-13-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: move fscrypt_prepare_symlink() out-of-lineEric Biggers2020-09-221-4/+35
| | | | | | | | | | | | | | In preparation for moving the logic for "get the encryption policy inherited by new files in this directory" to a single place, make fscrypt_prepare_symlink() a regular function rather than an inline function that wraps __fscrypt_prepare_symlink(). This way, the new function fscrypt_policy_to_inherit() won't need to be exported to filesystems. Acked-by: Jeff Layton <jlayton@kernel.org> Link: https://lore.kernel.org/r/20200917041136.178600-12-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: make "#define fscrypt_policy" user-onlyEric Biggers2020-09-221-1/+0
| | | | | | | | | | | | | | | | | | | | | | | The fscrypt UAPI header defines fscrypt_policy to fscrypt_policy_v1, for source compatibility with old userspace programs. Internally, the kernel doesn't want that compatibility definition. Instead, fscrypt_private.h #undefs it and re-defines it to a union. That works for now. However, in order to add fscrypt_operations::get_dummy_policy(), we'll need to forward declare 'union fscrypt_policy' in include/linux/fscrypt.h. That would cause build errors because "fscrypt_policy" is used in ioctl numbers. To avoid this, modify the UAPI header to make the fscrypt_policy compatibility definition conditional on !__KERNEL__, and make the ioctls use fscrypt_policy_v1 instead of fscrypt_policy. Note that this doesn't change the actual ioctl numbers. Acked-by: Jeff Layton <jlayton@kernel.org> Link: https://lore.kernel.org/r/20200917041136.178600-11-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: stop pretending that key setup is nofs-safeEric Biggers2020-09-223-10/+7
| | | | | | | | | | | | | | | | | | | | | | fscrypt_get_encryption_info() has never actually been safe to call in a context that needs GFP_NOFS, since it calls crypto_alloc_skcipher(). crypto_alloc_skcipher() isn't GFP_NOFS-safe, even if called under memalloc_nofs_save(). This is because it may load kernel modules, and also because it internally takes crypto_alg_sem. Other tasks can do GFP_KERNEL allocations while holding crypto_alg_sem for write. The use of fscrypt_init_mutex isn't GFP_NOFS-safe either. So, stop pretending that fscrypt_get_encryption_info() is nofs-safe. I.e., when it allocates memory, just use GFP_KERNEL instead of GFP_NOFS. Note, another reason to do this is that GFP_NOFS is deprecated in favor of using memalloc_nofs_save() in the proper places. Acked-by: Jeff Layton <jlayton@kernel.org> Link: https://lore.kernel.org/r/20200917041136.178600-10-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: require that fscrypt_encrypt_symlink() already has keyEric Biggers2020-09-221-3/+7
| | | | | | | | | | | | Now that all filesystems have been converted to use fscrypt_prepare_new_inode(), the encryption key for new symlink inodes is now already set up whenever we try to encrypt the symlink target. Enforce this rather than try to set up the key again when it may be too late to do so safely. Acked-by: Jeff Layton <jlayton@kernel.org> Link: https://lore.kernel.org/r/20200917041136.178600-9-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: remove fscrypt_inherit_context()Eric Biggers2020-09-221-37/+0
| | | | | | | | | | Now that all filesystems have been converted to use fscrypt_prepare_new_inode() and fscrypt_set_context(), fscrypt_inherit_context() is no longer used. Remove it. Acked-by: Jeff Layton <jlayton@kernel.org> Link: https://lore.kernel.org/r/20200917041136.178600-8-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: adjust logging for in-creation inodesEric Biggers2020-09-222-3/+10
| | | | | | | | | | Now that a fscrypt_info may be set up for inodes that are currently being created and haven't yet had an inode number assigned, avoid logging confusing messages about "inode 0". Acked-by: Jeff Layton <jlayton@kernel.org> Link: https://lore.kernel.org/r/20200917041136.178600-7-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: add fscrypt_prepare_new_inode() and fscrypt_set_context()Eric Biggers2020-09-223-54/+206
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | fscrypt_get_encryption_info() is intended to be GFP_NOFS-safe. But actually it isn't, since it uses functions like crypto_alloc_skcipher() which aren't GFP_NOFS-safe, even when called under memalloc_nofs_save(). Therefore it can deadlock when called from a context that needs GFP_NOFS, e.g. during an ext4 transaction or between f2fs_lock_op() and f2fs_unlock_op(). This happens when creating a new encrypted file. We can't fix this by just not setting up the key for new inodes right away, since new symlinks need their key to encrypt the symlink target. So we need to set up the new inode's key before starting the transaction. But just calling fscrypt_get_encryption_info() earlier doesn't work, since it assumes the encryption context is already set, and the encryption context can't be set until the transaction. The recently proposed fscrypt support for the ceph filesystem (https://lkml.kernel.org/linux-fscrypt/20200821182813.52570-1-jlayton@kernel.org/T/#u) will have this same ordering problem too, since ceph will need to encrypt new symlinks before setting their encryption context. Finally, f2fs can deadlock when the filesystem is mounted with '-o test_dummy_encryption' and a new file is created in an existing unencrypted directory. Similarly, this is caused by holding too many locks when calling fscrypt_get_encryption_info(). To solve all these problems, add new helper functions: - fscrypt_prepare_new_inode() sets up a new inode's encryption key (fscrypt_info), using the parent directory's encryption policy and a new random nonce. It neither reads nor writes the encryption context. - fscrypt_set_context() persists the encryption context of a new inode, using the information from the fscrypt_info already in memory. This replaces fscrypt_inherit_context(). Temporarily keep fscrypt_inherit_context() around until all filesystems have been converted to use fscrypt_set_context(). Acked-by: Jeff Layton <jlayton@kernel.org> Link: https://lore.kernel.org/r/20200917041136.178600-2-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: restrict IV_INO_LBLK_32 to ino_bits <= 32Eric Biggers2020-09-071-2/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | When an encryption policy has the IV_INO_LBLK_32 flag set, the IV generation method involves hashing the inode number. This is different from fscrypt's other IV generation methods, where the inode number is either not used at all or is included directly in the IVs. Therefore, in principle IV_INO_LBLK_32 can work with any length inode number. However, currently fscrypt gets the inode number from inode::i_ino, which is 'unsigned long'. So currently the implementation limit is actually 32 bits (like IV_INO_LBLK_64), since longer inode numbers will have been truncated by the VFS on 32-bit platforms. Fix fscrypt_supported_v2_policy() to enforce the correct limit. This doesn't actually matter currently, since only ext4 and f2fs support IV_INO_LBLK_32, and they both only support 32-bit inode numbers. But we might as well fix it in case it matters in the future. Ideally inode::i_ino would instead be made 64-bit, but for now it's not needed. (Note, this limit does *not* prevent filesystems with 64-bit inode numbers from adding fscrypt support, since IV_INO_LBLK_* support is optional and is useful only on certain hardware.) Fixes: e3b1078bedd3 ("fscrypt: add support for IV_INO_LBLK_32 policies") Reported-by: Jeff Layton <jlayton@kernel.org> Link: https://lore.kernel.org/r/20200824203841.1707847-1-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: drop unused inode argument from fscrypt_fname_alloc_bufferJeff Layton2020-09-072-5/+2
| | | | | | Signed-off-by: Jeff Layton <jlayton@kernel.org> Link: https://lore.kernel.org/r/20200810142139.487631-1-jlayton@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* mm, treewide: rename kzfree() to kfree_sensitive()Waiman Long2020-08-073-7/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As said by Linus: A symmetric naming is only helpful if it implies symmetries in use. Otherwise it's actively misleading. In "kzalloc()", the z is meaningful and an important part of what the caller wants. In "kzfree()", the z is actively detrimental, because maybe in the future we really _might_ want to use that "memfill(0xdeadbeef)" or something. The "zero" part of the interface isn't even _relevant_. The main reason that kzfree() exists is to clear sensitive information that should not be leaked to other future users of the same memory objects. Rename kzfree() to kfree_sensitive() to follow the example of the recently added kvfree_sensitive() and make the intention of the API more explicit. In addition, memzero_explicit() is used to clear the memory to make sure that it won't get optimized away by the compiler. The renaming is done by using the command sequence: git grep -w --name-only kzfree |\ xargs sed -i 's/kzfree/kfree_sensitive/' followed by some editing of the kfree_sensitive() kerneldoc and adding a kzfree backward compatibility macro in slab.h. [akpm@linux-foundation.org: fs/crypto/inline_crypt.c needs linux/slab.h] [akpm@linux-foundation.org: fix fs/crypto/inline_crypt.c some more] Suggested-by: Joe Perches <joe@perches.com> Signed-off-by: Waiman Long <longman@redhat.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Acked-by: David Howells <dhowells@redhat.com> Acked-by: Michal Hocko <mhocko@suse.com> Acked-by: Johannes Weiner <hannes@cmpxchg.org> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> Cc: James Morris <jmorris@namei.org> Cc: "Serge E. Hallyn" <serge@hallyn.com> Cc: Joe Perches <joe@perches.com> Cc: Matthew Wilcox <willy@infradead.org> Cc: David Rientjes <rientjes@google.com> Cc: Dan Carpenter <dan.carpenter@oracle.com> Cc: "Jason A . Donenfeld" <Jason@zx2c4.com> Link: http://lkml.kernel.org/r/20200616154311.12314-3-longman@redhat.com Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
* fscrypt: don't load ->i_crypt_info before it's known to be validEric Biggers2020-07-301-1/+2
| | | | | | | | | | | | | In fscrypt_set_bio_crypt_ctx(), ->i_crypt_info isn't known to be non-NULL until we check fscrypt_inode_uses_inline_crypto(). So, load ->i_crypt_info after the check rather than before. This makes no difference currently, but it prevents people from introducing bugs where the pointer is dereferenced when it may be NULL. Suggested-by: Dave Chinner <david@fromorbit.com> Cc: Satya Tangirala <satyat@google.com> Link: https://lore.kernel.org/r/20200727174158.121456-1-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: use smp_load_acquire() for ->i_crypt_infoEric Biggers2020-07-212-3/+13
| | | | | | | | | | | | | | | | | | | | | | | Normally smp_store_release() or cmpxchg_release() is paired with smp_load_acquire(). Sometimes smp_load_acquire() can be replaced with the more lightweight READ_ONCE(). However, for this to be safe, all the published memory must only be accessed in a way that involves the pointer itself. This may not be the case if allocating the object also involves initializing a static or global variable, for example. fscrypt_info includes various sub-objects which are internal to and are allocated by other kernel subsystems such as keyrings and crypto. So by using READ_ONCE() for ->i_crypt_info, we're relying on internal implementation details of these other kernel subsystems. Remove this fragile assumption by using smp_load_acquire() instead. (Note: I haven't seen any real-world problems here. This change is just fixing the code to be guaranteed correct and less fragile.) Fixes: e37a784d8b6a ("fscrypt: use READ_ONCE() to access ->i_crypt_info") Link: https://lore.kernel.org/r/20200721225920.114347-5-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: use smp_load_acquire() for ->s_master_keysEric Biggers2020-07-211-3/+12
| | | | | | | | | | | | | | | | | | | | | | Normally smp_store_release() or cmpxchg_release() is paired with smp_load_acquire(). Sometimes smp_load_acquire() can be replaced with the more lightweight READ_ONCE(). However, for this to be safe, all the published memory must only be accessed in a way that involves the pointer itself. This may not be the case if allocating the object also involves initializing a static or global variable, for example. super_block::s_master_keys is a keyring, which is internal to and is allocated by the keyrings subsystem. By using READ_ONCE() for it, we're relying on internal implementation details of the keyrings subsystem. Remove this fragile assumption by using smp_load_acquire() instead. (Note: I haven't seen any real-world problems here. This change is just fixing the code to be guaranteed correct and less fragile.) Fixes: 22d94f493bfb ("fscrypt: add FS_IOC_ADD_ENCRYPTION_KEY ioctl") Link: https://lore.kernel.org/r/20200721225920.114347-4-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: use smp_load_acquire() for fscrypt_prepared_keyEric Biggers2020-07-213-10/+17
| | | | | | | | | | | | | | | | | | | | | | | | Normally smp_store_release() or cmpxchg_release() is paired with smp_load_acquire(). Sometimes smp_load_acquire() can be replaced with the more lightweight READ_ONCE(). However, for this to be safe, all the published memory must only be accessed in a way that involves the pointer itself. This may not be the case if allocating the object also involves initializing a static or global variable, for example. fscrypt_prepared_key includes a pointer to a crypto_skcipher object, which is internal to and is allocated by the crypto subsystem. By using READ_ONCE() for it, we're relying on internal implementation details of the crypto subsystem. Remove this fragile assumption by using smp_load_acquire() instead. (Note: I haven't seen any real-world problems here. This change is just fixing the code to be guaranteed correct and less fragile.) Fixes: 5fee36095cda ("fscrypt: add inline encryption support") Cc: Satya Tangirala <satyat@google.com> Link: https://lore.kernel.org/r/20200721225920.114347-3-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: switch fscrypt_do_sha256() to use the SHA-256 libraryEric Biggers2020-07-212-32/+11
| | | | | | | | | | | | | | | | | | | | | | | fscrypt_do_sha256() is only used for hashing encrypted filenames to create no-key tokens, which isn't performance-critical. Therefore a C implementation of SHA-256 is sufficient. Also, the logic to create no-key tokens is always potentially needed. This differs from fscrypt's other dependencies on crypto API algorithms, which are conditionally needed depending on what encryption policies userspace is using. Therefore, for fscrypt there isn't much benefit to allowing SHA-256 to be a loadable module. So, make fscrypt_do_sha256() use the SHA-256 library instead of the crypto_shash API. This is much simpler, since it avoids having to implement one-time-init (which is hard to do correctly, and in fact was implemented incorrectly) and handle failures to allocate the crypto_shash object. Fixes: edc440e3d27f ("fscrypt: improve format of no-key names") Cc: Daniel Rosenberg <drosen@google.com> Link: https://lore.kernel.org/r/20200721225920.114347-2-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: restrict IV_INO_LBLK_* to AES-256-XTSEric Biggers2020-07-211-0/+14
| | | | | | | | | | | | | | | | | | | | IV_INO_LBLK_* exist only because of hardware limitations, and currently the only known use case for them involves AES-256-XTS. Therefore, for now only allow them in combination with AES-256-XTS. This way we don't have to worry about them being combined with other encryption modes. (To be clear, combining IV_INO_LBLK_* with other encryption modes *should* work just fine. It's just not being tested, so we can't be 100% sure it works. So with no known use case, it's best to disallow it for now, just like we don't allow other weird combinations like AES-256-XTS contents encryption with Adiantum filenames encryption.) This can be relaxed later if a use case for other combinations arises. Fixes: b103fb7653ff ("fscrypt: add support for IV_INO_LBLK_64 policies") Fixes: e3b1078bedd3 ("fscrypt: add support for IV_INO_LBLK_32 policies") Link: https://lore.kernel.org/r/20200721181012.39308-1-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
* fscrypt: rename FS_KEY_DERIVATION_NONCE_SIZEEric Biggers2020-07-205-14/+13
| | | | | | | | | | | | The name "FS_KEY_DERIVATION_NONCE_SIZE" is a bit outdated since due to the addition of FSCRYPT_POLICY_FLAG_DIRECT_KEY, the file nonce may now be used as a tweak instead of for key derivation. Also, we're now prefixing the fscrypt constants with "FSCRYPT_" instead of "FS_". Therefore, rename this constant to FSCRYPT_FILE_NONCE_SIZE. Link: https://lore.kernel.org/r/20200708215722.147154-1-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-27 11:52:31 +0000