On Fri, Jul 04, 2025 at 12:13:22PM +0800, Yuwen Chen wrote:
> With the CONFIG_UNICODE configuration enabled, the fname_decrypt
> and fscrypt_fname_encrypt functions may be called very frequently.
> Since filenames are generally short, the frequent invocation of
> memory allocation and release operations by these two functions
> will lead to very poor performance.
>
> Use the following program to conduct a file-creation test in
> the private program directory(/data/media/0/Android/data/*)
> of Android.
>
> int main(int argc, char **argv)
> {
> size_t fcnt = 0;
> char path[PATH_MAX];
> char buf[4096] = {0};
> int i, fd;
>
> if (argc < 2)
> return - EINVAL;
>
> fcnt = atoi(argv[1]);
> for (i = 0; i < fcnt; i++) {
> snprintf(path, sizeof(path), "./%d", i);
> fd = open(path, O_RDWR | O_CREAT, 0600);
> if (fd < 0)
> return - 1;
> write(fd, buf, sizeof(buf));
> close(fd);
> }
> return 0;
> }
>
> The test platform is Snapdragon 8s Gen4, with a kernel version
> of v6.6 and a userdebug version.
>
> Before this submission was merged, when creating 2000 files,
> the performance test results are as follows:
> $ time /data/file_creater 2000
> 0m14.83s real 0m00.00s user 0m14.30s system
> 0m15.61s real 0m00.00s user 0m15.04s system
> 0m14.72s real 0m00.01s user 0m14.18s system
>
> After this submission was merged, the performance is as follows:
> $ time /data/file_creater 2000
> 0m07.64s real 0m00.00s user 0m07.37s system
> 0m07.66s real 0m00.00s user 0m07.40s system
> 0m08.67s real 0m00.01s user 0m08.35s system
>
> Signed-off-by: Yuwen Chen <ywen.c...@foxmail.com>
> ---
> fs/crypto/fname.c | 22 ++++++++++++++--------
> include/crypto/skcipher.h | 9 +++++++++
> 2 files changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
> index 010f9c0a4c2f1..168b2a88fa23b 100644
> --- a/fs/crypto/fname.c
> +++ b/fs/crypto/fname.c
> @@ -92,14 +92,20 @@ static inline bool fscrypt_is_dot_dotdot(const struct
> qstr *str)
> int fscrypt_fname_encrypt(const struct inode *inode, const struct qstr
> *iname,
> u8 *out, unsigned int olen)
> {
> - struct skcipher_request *req = NULL;
> DECLARE_CRYPTO_WAIT(wait);
> const struct fscrypt_inode_info *ci = inode->i_crypt_info;
> struct crypto_skcipher *tfm = ci->ci_enc_key.tfm;
> + SKCIPHER_REQUEST_ON_STACK(req, tfm, MAX_SKCIPHER_REQSIZE);
> union fscrypt_iv iv;
> struct scatterlist sg;
> int res;
>
> + /*
> + * When the size of the statically - allocated skcipher_request
> + * structure is insufficient, remind us to make modifications.
> + */
> + BUG_ON(MAX_SKCIPHER_REQSIZE < crypto_skcipher_reqsize(tfm));
> +
> /*
> * Copy the filename to the output buffer for encrypting in-place and
> * pad it with the needed number of NUL bytes.
> @@ -124,7 +130,6 @@ int fscrypt_fname_encrypt(const struct inode *inode,
> const struct qstr *iname,
>
> /* Do the encryption */
> res = crypto_wait_req(crypto_skcipher_encrypt(req), &wait);
> - skcipher_request_free(req);
> if (res < 0) {
> fscrypt_err(inode, "Filename encryption failed: %d", res);
> return res;
I'm guessing you have some debugging options enabled in your kconfig. Usually
the allocations aren't quite *that* expensive. That being said, it's always
been really annoying that they have to be there.
Unfortunately, as far as I know, you actually can't just allocate the
skcipher_request on the stack like that, since the legacy crypto API assumes
that the request memory is DMA-able. On-stack requests also might not be
properly aligned (see
https://lore.kernel.org/all/ca+55afxjozmim_d-o2e2yip8jwo0ndys_72snwfksktjy8q...@mail.gmail.com/
-- may be outdated, but I haven't heard otherwise).
The problem is really that the legacy crypto API (crypto_skcipher in this case)
was never really designed for efficient CPU-based crypto in the first place.
The correct solution is to add simple library APIs for the algorithms to
lib/crypto/, then update fscrypt to use that instead of crypto_skcipher.
I plan to do that, but I'm first focusing on other related things, such as doing
the same for fsverity.
- Eric
