Eric Lee / linux-smarc-t335x-v3.2

Download as
Browse Code ยป

Commit 778aeb42a708d2a57e491d2cbb5a1e74f61270b9

Authored by Tyler Hicks
1 parent 7a86617e55
Exists in master and in 4 other branches v3.2_AM335xPSP_04.06.00.11, v3.2_NEWT335xPSP_04.06.00.11, v3.2_SBC_SMARTMEN, v3.2_SMARCT335xPSP_04.06.00.11

eCryptfs: Cleanup and optimize ecryptfs_lookup_interpose() 

ecryptfs_lookup_interpose() has turned into spaghetti code over the
years. This is an effort to clean it up.

 - Shorten overly descriptive variable names such as ecryptfs_dentry
 - Simplify gotos and error paths
 - Create helper function for reading plaintext i_size from metadata

It also includes an optimization when reading i_size from the metadata.
A complete page-sized kmem_cache_alloc() was being done to read in 16
bytes of metadata. The buffer for that is now statically declared.

Signed-off-by: Tyler Hicks <tyhicks@linux.vnet.ibm.com>

Showing 3 changed files with 88 additions and 105 deletions Side-by-side Diff

fs/ecryptfs/crypto.c
Diff comments   View file @ 778aeb4
... ... @@ -1201,24 +1201,19 @@
1201 1201 return rc;
1202 1202 }
1203 1203  
1204   -int ecryptfs_read_and_validate_header_region(char *data,
1205   - struct inode *ecryptfs_inode)
  1204 +int ecryptfs_read_and_validate_header_region(struct inode *inode)
1206 1205 {
1207   - struct ecryptfs_crypt_stat *crypt_stat =
1208   - &(ecryptfs_inode_to_private(ecryptfs_inode)->crypt_stat);
  1206 + u8 file_size[ECRYPTFS_SIZE_AND_MARKER_BYTES];
  1207 + u8 *marker = file_size + ECRYPTFS_FILE_SIZE_BYTES;
1209 1208 int rc;
1210 1209  
1211   - if (crypt_stat->extent_size == 0)
1212   - crypt_stat->extent_size = ECRYPTFS_DEFAULT_EXTENT_SIZE;
1213   - rc = ecryptfs_read_lower(data, 0, crypt_stat->extent_size,
1214   - ecryptfs_inode);
1215   - if (rc < 0) {
1216   - printk(KERN_ERR "%s: Error reading header region; rc = [%d]\n",
1217   - __func__, rc);
1218   - goto out;
1219   - }
1220   - rc = ecryptfs_validate_marker(data + ECRYPTFS_FILE_SIZE_BYTES);
1221   -out:
  1210 + rc = ecryptfs_read_lower(file_size, 0, ECRYPTFS_SIZE_AND_MARKER_BYTES,
  1211 + inode);
  1212 + if (rc < ECRYPTFS_SIZE_AND_MARKER_BYTES)
  1213 + return rc >= 0 ? -EINVAL : rc;
  1214 + rc = ecryptfs_validate_marker(marker);
  1215 + if (!rc)
  1216 + ecryptfs_i_size_init(file_size, inode);
1222 1217 return rc;
1223 1218 }
1224 1219  
1225 1220  
1226 1221  
... ... @@ -1562,19 +1557,21 @@
1562 1557 return rc;
1563 1558 }
1564 1559  
1565   -int ecryptfs_read_and_validate_xattr_region(char *page_virt,
  1560 +int ecryptfs_read_and_validate_xattr_region(struct dentry *dentry,
1566 1561 struct inode *inode)
1567 1562 {
  1563 + u8 file_size[ECRYPTFS_SIZE_AND_MARKER_BYTES];
  1564 + u8 *marker = file_size + ECRYPTFS_FILE_SIZE_BYTES;
1568 1565 int rc;
1569 1566  
1570   - rc = ecryptfs_read_xattr_region(page_virt, inode);
1571   - if (rc)
1572   - goto out;
1573   - rc = ecryptfs_validate_marker(page_virt + ECRYPTFS_FILE_SIZE_BYTES);
1574   - if (rc)
1575   - printk(KERN_WARNING "Valid data found in [%s] xattr, but "
1576   - "the marker is invalid\n", ECRYPTFS_XATTR_NAME);
1577   -out:
  1567 + rc = ecryptfs_getxattr_lower(ecryptfs_dentry_to_lower(dentry),
  1568 + ECRYPTFS_XATTR_NAME, file_size,
  1569 + ECRYPTFS_SIZE_AND_MARKER_BYTES);
  1570 + if (rc < ECRYPTFS_SIZE_AND_MARKER_BYTES)
  1571 + return rc >= 0 ? -EINVAL : rc;
  1572 + rc = ecryptfs_validate_marker(marker);
  1573 + if (!rc)
  1574 + ecryptfs_i_size_init(file_size, inode);
1578 1575 return rc;
1579 1576 }
1580 1577  
fs/ecryptfs/ecryptfs_kernel.h
Diff comments   View file @ 778aeb4
... ... @@ -200,6 +200,8 @@
200 200 #define MAGIC_ECRYPTFS_MARKER 0x3c81b7f5
201 201 #define MAGIC_ECRYPTFS_MARKER_SIZE_BYTES 8 /* 4*2 */
202 202 #define ECRYPTFS_FILE_SIZE_BYTES (sizeof(u64))
  203 +#define ECRYPTFS_SIZE_AND_MARKER_BYTES (ECRYPTFS_FILE_SIZE_BYTES \
  204 + + MAGIC_ECRYPTFS_MARKER_SIZE_BYTES)
203 205 #define ECRYPTFS_DEFAULT_CIPHER "aes"
204 206 #define ECRYPTFS_DEFAULT_KEY_BYTES 16
205 207 #define ECRYPTFS_DEFAULT_HASH "md5"
... ... @@ -659,9 +661,8 @@
659 661 void ecryptfs_write_crypt_stat_flags(char *page_virt,
660 662 struct ecryptfs_crypt_stat *crypt_stat,
661 663 size_t *written);
662   -int ecryptfs_read_and_validate_header_region(char *data,
663   - struct inode *ecryptfs_inode);
664   -int ecryptfs_read_and_validate_xattr_region(char *page_virt,
  664 +int ecryptfs_read_and_validate_header_region(struct inode *inode);
  665 +int ecryptfs_read_and_validate_xattr_region(struct dentry *dentry,
665 666 struct inode *inode);
666 667 u8 ecryptfs_code_for_cipher_string(char *cipher_name, size_t key_bytes);
667 668 int ecryptfs_cipher_code_to_string(char *str, u8 cipher_code);
... ... @@ -307,105 +307,90 @@
307 307 return rc;
308 308 }
309 309  
  310 +static int ecryptfs_i_size_read(struct dentry *dentry, struct inode *inode)
  311 +{
  312 + struct ecryptfs_crypt_stat *crypt_stat;
  313 + int rc;
  314 +
  315 + rc = ecryptfs_get_lower_file(dentry, inode);
  316 + if (rc) {
  317 + printk(KERN_ERR "%s: Error attempting to initialize "
  318 + "the lower file for the dentry with name "
  319 + "[%s]; rc = [%d]\n", __func__,
  320 + dentry->d_name.name, rc);
  321 + return rc;
  322 + }
  323 +
  324 + crypt_stat = &ecryptfs_inode_to_private(inode)->crypt_stat;
  325 + /* TODO: lock for crypt_stat comparison */
  326 + if (!(crypt_stat->flags & ECRYPTFS_POLICY_APPLIED))
  327 + ecryptfs_set_default_sizes(crypt_stat);
  328 +
  329 + rc = ecryptfs_read_and_validate_header_region(inode);
  330 + ecryptfs_put_lower_file(inode);
  331 + if (rc) {
  332 + rc = ecryptfs_read_and_validate_xattr_region(dentry, inode);
  333 + if (!rc)
  334 + crypt_stat->flags |= ECRYPTFS_METADATA_IN_XATTR;
  335 + }
  336 +
  337 + /* Must return 0 to allow non-eCryptfs files to be looked up, too */
  338 + return 0;
  339 +}
  340 +
310 341 /**
311 342 * ecryptfs_lookup_interpose - Dentry interposition for a lookup
312 343 */
313   -static int ecryptfs_lookup_interpose(struct dentry *ecryptfs_dentry,
  344 +static int ecryptfs_lookup_interpose(struct dentry *dentry,
314 345 struct dentry *lower_dentry,
315   - struct inode *ecryptfs_dir_inode)
  346 + struct inode *dir_inode)
316 347 {
317   - struct dentry *lower_dir_dentry;
  348 + struct inode *inode, *lower_inode = lower_dentry->d_inode;
  349 + struct ecryptfs_dentry_info *dentry_info;
318 350 struct vfsmount *lower_mnt;
319   - struct inode *inode, *lower_inode;
320   - struct ecryptfs_crypt_stat *crypt_stat;
321   - char *page_virt = NULL;
322   - int put_lower = 0, rc = 0;
  351 + int rc = 0;
323 352  
324   - lower_dir_dentry = lower_dentry->d_parent;
325   - lower_mnt = mntget(ecryptfs_dentry_to_lower_mnt(
326   - ecryptfs_dentry->d_parent));
327   - lower_inode = lower_dentry->d_inode;
328   - fsstack_copy_attr_atime(ecryptfs_dir_inode, lower_dir_dentry->d_inode);
  353 + lower_mnt = mntget(ecryptfs_dentry_to_lower_mnt(dentry->d_parent));
  354 + fsstack_copy_attr_atime(dir_inode, lower_dentry->d_parent->d_inode);
329 355 BUG_ON(!lower_dentry->d_count);
330   - ecryptfs_set_dentry_private(ecryptfs_dentry,
331   - kmem_cache_alloc(ecryptfs_dentry_info_cache,
332   - GFP_KERNEL));
333   - if (!ecryptfs_dentry_to_private(ecryptfs_dentry)) {
334   - rc = -ENOMEM;
  356 +
  357 + dentry_info = kmem_cache_alloc(ecryptfs_dentry_info_cache, GFP_KERNEL);
  358 + ecryptfs_set_dentry_private(dentry, dentry_info);
  359 + if (!dentry_info) {
335 360 printk(KERN_ERR "%s: Out of memory whilst attempting "
336 361 "to allocate ecryptfs_dentry_info struct\n",
337 362 __func__);
338   - goto out_put;
  363 + dput(lower_dentry);
  364 + mntput(lower_mnt);
  365 + d_drop(dentry);
  366 + return -ENOMEM;
339 367 }
340   - ecryptfs_set_dentry_lower(ecryptfs_dentry, lower_dentry);
341   - ecryptfs_set_dentry_lower_mnt(ecryptfs_dentry, lower_mnt);
  368 + ecryptfs_set_dentry_lower(dentry, lower_dentry);
  369 + ecryptfs_set_dentry_lower_mnt(dentry, lower_mnt);
  370 +
342 371 if (!lower_dentry->d_inode) {
343 372 /* We want to add because we couldn't find in lower */
344   - d_add(ecryptfs_dentry, NULL);
345   - goto out;
  373 + d_add(dentry, NULL);
  374 + return 0;
346 375 }
347   - inode = __ecryptfs_get_inode(lower_inode, ecryptfs_dir_inode->i_sb);
  376 + inode = __ecryptfs_get_inode(lower_inode, dir_inode->i_sb);
348 377 if (IS_ERR(inode)) {
349   - rc = PTR_ERR(inode);
350   - printk(KERN_ERR "%s: Error interposing; rc = [%d]\n",
351   - __func__, rc);
352   - goto out;
  378 + printk(KERN_ERR "%s: Error interposing; rc = [%ld]\n",
  379 + __func__, PTR_ERR(inode));
  380 + return PTR_ERR(inode);
353 381 }
354   - if (!S_ISREG(inode->i_mode)) {
355   - if (inode->i_state & I_NEW)
356   - unlock_new_inode(inode);
357   - d_add(ecryptfs_dentry, inode);
358   - goto out;
359   - }
360   - /* Released in this function */
361   - page_virt = kmem_cache_zalloc(ecryptfs_header_cache_2, GFP_USER);
362   - if (!page_virt) {
363   - printk(KERN_ERR "%s: Cannot kmem_cache_zalloc() a page\n",
364   - __func__);
365   - rc = -ENOMEM;
366   - make_bad_inode(inode);
367   - goto out;
368   - }
369   - rc = ecryptfs_get_lower_file(ecryptfs_dentry, inode);
370   - if (rc) {
371   - printk(KERN_ERR "%s: Error attempting to initialize "
372   - "the lower file for the dentry with name "
373   - "[%s]; rc = [%d]\n", __func__,
374   - ecryptfs_dentry->d_name.name, rc);
375   - make_bad_inode(inode);
376   - goto out_free_kmem;
377   - }
378   - put_lower = 1;
379   - crypt_stat = &ecryptfs_inode_to_private(inode)->crypt_stat;
380   - /* TODO: lock for crypt_stat comparison */
381   - if (!(crypt_stat->flags & ECRYPTFS_POLICY_APPLIED))
382   - ecryptfs_set_default_sizes(crypt_stat);
383   - rc = ecryptfs_read_and_validate_header_region(page_virt, inode);
384   - if (rc) {
385   - memset(page_virt, 0, PAGE_CACHE_SIZE);
386   - rc = ecryptfs_read_and_validate_xattr_region(page_virt,
387   - inode);
  382 + if (S_ISREG(inode->i_mode)) {
  383 + rc = ecryptfs_i_size_read(dentry, inode);
388 384 if (rc) {
389   - rc = 0;
390   - goto unlock_inode;
  385 + make_bad_inode(inode);
  386 + return rc;
391 387 }
392   - crypt_stat->flags |= ECRYPTFS_METADATA_IN_XATTR;
393 388 }
394   - ecryptfs_i_size_init(page_virt, inode);
395   -unlock_inode:
  389 +
396 390 if (inode->i_state & I_NEW)
397 391 unlock_new_inode(inode);
398   - d_add(ecryptfs_dentry, inode);
399   -out_free_kmem:
400   - kmem_cache_free(ecryptfs_header_cache_2, page_virt);
401   - goto out;
402   -out_put:
403   - dput(lower_dentry);
404   - mntput(lower_mnt);
405   - d_drop(ecryptfs_dentry);
406   -out:
407   - if (put_lower)
408   - ecryptfs_put_lower_file(inode);
  392 + d_add(dentry, inode);
  393 +
409 394 return rc;
410 395 }
411 396