fscrypto-make-filename-crypto-functions-return-0-on-success

   1 fscrypto: make filename crypto functions return 0 on success
   2
   3 From: Eric Biggers <ebiggers@google.com>
   4
   5 Several filename crypto functions: fname_decrypt(),
   6 fscrypt_fname_disk_to_usr(), and fscrypt_fname_usr_to_disk(), returned
   7 the output length on success or -errno on failure.  However, the output
   8 length was redundant with the value written to 'oname->len'.  It is also
   9 potentially error-prone to make callers have to check for '< 0' instead
  10 of '!= 0'.
  11
  12 Therefore, make these functions return 0 instead of a length, and make
  13 the callers who cared about the return value being a length use
  14 'oname->len' instead.  For consistency also make other callers check for
  15 a nonzero result rather than a negative result.
  16
  17 This change also fixes the inconsistency of fname_encrypt() actually
  18 already returning 0 on success, not a length like the other filename
  19 crypto functions and as documented in its function comment.
  20
  21 Signed-off-by: Eric Biggers <ebiggers@google.com>
  22 Signed-off-by: Theodore Ts'o <tytso@mit.edu>
  23 Reviewed-by: Andreas Dilger <adilger@dilger.ca>
  24 Acked-by: Jaegeuk Kim <jaegeuk@kernel.org>
  25 ---
  26  fs/crypto/fname.c | 56 ++++++++++++++++++++++++++++++++------------------------
  27  fs/ext4/dir.c     |  5 +++--
  28  fs/ext4/namei.c   |  8 ++++----
  29  fs/ext4/symlink.c |  5 ++---
  30  fs/f2fs/dir.c     |  6 +++---
  31  fs/f2fs/namei.c   |  6 +++---
  32  6 files changed, 47 insertions(+), 39 deletions(-)
  33
  34 diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
  35 index 6bbc3b1..90697c7 100644
  36 --- a/fs/crypto/fname.c
  37 +++ b/fs/crypto/fname.c
  38 @@ -35,11 +35,11 @@ static void fname_crypt_complete(struct crypto_async_request *req, int res)
  39  }
  40
  41  /**
  42 - * fname_encrypt() -
  43 + * fname_encrypt() - encrypt a filename
  44   *
  45 - * This function encrypts the input filename, and returns the length of the
  46 - * ciphertext. Errors are returned as negative numbers.  We trust the caller to
  47 - * allocate sufficient memory to oname string.
  48 + * The caller must have allocated sufficient memory for the @oname string.
  49 + *
  50 + * Return: 0 on success, -errno on failure
  51   */
  52  static int fname_encrypt(struct inode *inode,
  53                         const struct qstr *iname, struct fscrypt_str *oname)
  54 @@ -105,20 +105,22 @@ static int fname_encrypt(struct inode *inode,
  55         }
  56         kfree(alloc_buf);
  57         skcipher_request_free(req);
  58 -       if (res < 0)
  59 +       if (res < 0) {
  60                 printk_ratelimited(KERN_ERR
  61                                 "%s: Error (error code %d)\n", __func__, res);
  62 +               return res;
  63 +       }
  64
  65         oname->len = ciphertext_len;
  66 -       return res;
  67 +       return 0;
  68  }
  69
  70 -/*
  71 - * fname_decrypt()
  72 - *     This function decrypts the input filename, and returns
  73 - *     the length of the plaintext.
  74 - *     Errors are returned as negative numbers.
  75 - *     We trust the caller to allocate sufficient memory to oname string.
  76 +/**
  77 + * fname_decrypt() - decrypt a filename
  78 + *
  79 + * The caller must have allocated sufficient memory for the @oname string.
  80 + *
  81 + * Return: 0 on success, -errno on failure
  82   */
  83  static int fname_decrypt(struct inode *inode,
  84                                 const struct fscrypt_str *iname,
  85 @@ -168,7 +170,7 @@ static int fname_decrypt(struct inode *inode,
  86         }
  87
  88         oname->len = strnlen(oname->name, iname->len);
  89 -       return oname->len;
  90 +       return 0;
  91  }
  92
  93  static const char *lookup_table =
  94 @@ -279,6 +281,10 @@ EXPORT_SYMBOL(fscrypt_fname_free_buffer);
  95  /**
  96   * fscrypt_fname_disk_to_usr() - converts a filename from disk space to user
  97   * space
  98 + *
  99 + * The caller must have allocated sufficient memory for the @oname string.
 100 + *
 101 + * Return: 0 on success, -errno on failure
 102   */
 103  int fscrypt_fname_disk_to_usr(struct inode *inode,
 104                         u32 hash, u32 minor_hash,
 105 @@ -287,13 +293,12 @@ int fscrypt_fname_disk_to_usr(struct inode *inode,
 106  {
 107         const struct qstr qname = FSTR_TO_QSTR(iname);
 108         char buf[24];
 109 -       int ret;
 110
 111         if (fscrypt_is_dot_dotdot(&qname)) {
 112                 oname->name[0] = '.';
 113                 oname->name[iname->len - 1] = '.';
 114                 oname->len = iname->len;
 115 -               return oname->len;
 116 +               return 0;
 117         }
 118
 119         if (iname->len < FS_CRYPTO_BLOCK_SIZE)
 120 @@ -303,9 +308,9 @@ int fscrypt_fname_disk_to_usr(struct inode *inode,
 121                 return fname_decrypt(inode, iname, oname);
 122
 123         if (iname->len <= FS_FNAME_CRYPTO_DIGEST_SIZE) {
 124 -               ret = digest_encode(iname->name, iname->len, oname->name);
 125 -               oname->len = ret;
 126 -               return ret;
 127 +               oname->len = digest_encode(iname->name, iname->len,
 128 +                                          oname->name);
 129 +               return 0;
 130         }
 131         if (hash) {
 132                 memcpy(buf, &hash, 4);
 133 @@ -315,15 +320,18 @@ int fscrypt_fname_disk_to_usr(struct inode *inode,
 134         }
 135         memcpy(buf + 8, iname->name + iname->len - 16, 16);
 136         oname->name[0] = '_';
 137 -       ret = digest_encode(buf, 24, oname->name + 1);
 138 -       oname->len = ret + 1;
 139 -       return ret + 1;
 140 +       oname->len = 1 + digest_encode(buf, 24, oname->name + 1);
 141 +       return 0;
 142  }
 143  EXPORT_SYMBOL(fscrypt_fname_disk_to_usr);
 144
 145  /**
 146   * fscrypt_fname_usr_to_disk() - converts a filename from user space to disk
 147   * space
 148 + *
 149 + * The caller must have allocated sufficient memory for the @oname string.
 150 + *
 151 + * Return: 0 on success, -errno on failure
 152   */
 153  int fscrypt_fname_usr_to_disk(struct inode *inode,
 154                         const struct qstr *iname,
 155 @@ -333,7 +341,7 @@ int fscrypt_fname_usr_to_disk(struct inode *inode,
 156                 oname->name[0] = '.';
 157                 oname->name[iname->len - 1] = '.';
 158                 oname->len = iname->len;
 159 -               return oname->len;
 160 +               return 0;
 161         }
 162         if (inode->i_crypt_info)
 163                 return fname_encrypt(inode, iname, oname);
 164 @@ -367,10 +375,10 @@ int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname,
 165         if (dir->i_crypt_info) {
 166                 ret = fscrypt_fname_alloc_buffer(dir, iname->len,
 167                                                         &fname->crypto_buf);
 168 -               if (ret < 0)
 169 +               if (ret)
 170                         return ret;
 171                 ret = fname_encrypt(dir, iname, &fname->crypto_buf);
 172 -               if (ret < 0)
 173 +               if (ret)
 174                         goto errout;
 175                 fname->disk_name.name = fname->crypto_buf.name;
 176                 fname->disk_name.len = fname->crypto_buf.len;
 177 diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
 178 index 67415e0..4d4b910 100644
 179 --- a/fs/ext4/dir.c
 180 +++ b/fs/ext4/dir.c
 181 @@ -260,11 +260,12 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)
 182                                         /* Directory is encrypted */
 183                                         err = fscrypt_fname_disk_to_usr(inode,
 184                                                 0, 0, &de_name, &fstr);
 185 +                                       de_name = fstr;
 186                                         fstr.len = save_len;
 187 -                                       if (err < 0)
 188 +                                       if (err)
 189                                                 goto errout;
 190                                         if (!dir_emit(ctx,
 191 -                                           fstr.name, err,
 192 +                                           de_name.name, de_name.len,
 193                                             le32_to_cpu(de->inode),
 194                                             get_dtype(sb, de->file_type)))
 195                                                 goto done;
 196 diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
 197 index 34c0142..2243ae2 100644
 198 --- a/fs/ext4/namei.c
 199 +++ b/fs/ext4/namei.c
 200 @@ -639,7 +639,7 @@ static struct stats dx_show_leaf(struct inode *dir,
 201                                         res = fscrypt_fname_alloc_buffer(
 202                                                 dir, len,
 203                                                 &fname_crypto_str);
 204 -                                       if (res < 0)
 205 +                                       if (res)
 206                                                 printk(KERN_WARNING "Error "
 207                                                         "allocating crypto "
 208                                                         "buffer--skipping "
 209 @@ -647,7 +647,7 @@ static struct stats dx_show_leaf(struct inode *dir,
 210                                         res = fscrypt_fname_disk_to_usr(dir,
 211                                                 0, 0, &de_name,
 212                                                 &fname_crypto_str);
 213 -                                       if (res < 0) {
 214 +                                       if (res) {
 215                                                 printk(KERN_WARNING "Error "
 216                                                         "converting filename "
 217                                                         "from disk to usr"
 218 @@ -1011,7 +1011,7 @@ static int htree_dirblock_to_tree(struct file *dir_file,
 219                         err = fscrypt_fname_disk_to_usr(dir, hinfo->hash,
 220                                         hinfo->minor_hash, &de_name,
 221                                         &fname_crypto_str);
 222 -                       if (err < 0) {
 223 +                       if (err) {
 224                                 count = err;
 225                                 goto errout;
 226                         }
 227 @@ -3144,7 +3144,7 @@ static int ext4_symlink(struct inode *dir,
 228                 istr.name = (const unsigned char *) symname;
 229                 istr.len = len;
 230                 err = fscrypt_fname_usr_to_disk(inode, &istr, &ostr);
 231 -               if (err < 0)
 232 +               if (err)
 233                         goto err_drop_inode;
 234                 sd->len = cpu_to_le16(ostr.len);
 235                 disk_link.name = (char *) sd;
 236 diff --git a/fs/ext4/symlink.c b/fs/ext4/symlink.c
 237 index 04a7850..0a26cbd 100644
 238 --- a/fs/ext4/symlink.c
 239 +++ b/fs/ext4/symlink.c
 240 @@ -68,12 +68,11 @@ static const char *ext4_encrypted_get_link(struct dentry *dentry,
 241         paddr = pstr.name;
 242
 243         res = fscrypt_fname_disk_to_usr(inode, 0, 0, &cstr, &pstr);
 244 -       if (res < 0)
 245 +       if (res)
 246                 goto errout;
 247
 248         /* Null-terminate the name */
 249 -       if (res <= pstr.len)
 250 -               paddr[res] = '\0';
 251 +       paddr[pstr.len] = '\0';
 252         if (cpage)
 253                 put_page(cpage);
 254         set_delayed_call(done, kfree_link, paddr);
 255 diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
 256 index 9054aea..8716943 100644
 257 --- a/fs/f2fs/dir.c
 258 +++ b/fs/f2fs/dir.c
 259 @@ -786,7 +786,7 @@ bool f2fs_fill_dentries(struct dir_context *ctx, struct f2fs_dentry_ptr *d,
 260
 261                 if (f2fs_encrypted_inode(d->inode)) {
 262                         int save_len = fstr->len;
 263 -                       int ret;
 264 +                       int err;
 265
 266                         de_name.name = f2fs_kmalloc(de_name.len, GFP_NOFS);
 267                         if (!de_name.name)
 268 @@ -794,11 +794,11 @@ bool f2fs_fill_dentries(struct dir_context *ctx, struct f2fs_dentry_ptr *d,
 269
 270                         memcpy(de_name.name, d->filename[bit_pos], de_name.len);
 271
 272 -                       ret = fscrypt_fname_disk_to_usr(d->inode,
 273 +                       err = fscrypt_fname_disk_to_usr(d->inode,
 274                                                 (u32)de->hash_code, 0,
 275                                                 &de_name, fstr);
 276                         kfree(de_name.name);
 277 -                       if (ret < 0)
 278 +                       if (err)
 279                                 return true;
 280
 281                         de_name = *fstr;
 282 diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
 283 index 73fa356..afd5633 100644
 284 --- a/fs/f2fs/namei.c
 285 +++ b/fs/f2fs/namei.c
 286 @@ -449,7 +449,7 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
 287                 ostr.name = sd->encrypted_path;
 288                 ostr.len = disk_link.len;
 289                 err = fscrypt_fname_usr_to_disk(inode, &istr, &ostr);
 290 -               if (err < 0)
 291 +               if (err)
 292                         goto err_out;
 293
 294                 sd->len = cpu_to_le16(ostr.len);
 295 @@ -1048,7 +1048,7 @@ static const char *f2fs_encrypted_get_link(struct dentry *dentry,
 296                 goto errout;
 297
 298         res = fscrypt_fname_disk_to_usr(inode, 0, 0, &cstr, &pstr);
 299 -       if (res < 0)
 300 +       if (res)
 301                 goto errout;
 302
 303         /* this is broken symlink case */
 304 @@ -1060,7 +1060,7 @@ static const char *f2fs_encrypted_get_link(struct dentry *dentry,
 305         paddr = pstr.name;
 306
 307         /* Null-terminate the name */
 308 -       paddr[res] = '\0';
 309 +       paddr[pstr.len] = '\0';
 310
 311         put_page(cpage);
 312         set_delayed_call(done, kfree_link, paddr);