Advertisement
Guest User

[PATCH 1/2] ocfs2: Fix llseek() semantics and do some cleanu

a guest
Sep 2nd, 2013
65
0
Never
Not a member of Pastebin yet? Sign Up, it unlocks many cool features!
text 4.00 KB | None | 0 0
  1. There are multiple issues with the custom llseek implemented in ocfs2 for
  2. implementing SEEK_HOLE/SEEK_DATA.
  3.  
  4. 1. It takes the inode->i_mutex lock before calling generic_file_llseek(), which
  5. is unnecessary.
  6.  
  7. 2. It fails to take the filp->f_lock spinlock before modifying filp->f_pos and
  8. filp->f_version, which differs from generic_file_llseek().
  9.  
  10. 3. It does a offset > inode->i_sb->s_maxbytes check that permits seeking up to
  11. the maximum file size possible on the ocfs2 filesystem, even when it is past
  12. the end of the file. Seeking beyond that (if possible), would return EINVAL
  13. instead of ENXIO.
  14.  
  15. 4. The switch statement tries to cover all whence values when in reality it
  16. should only care about SEEK_HOLE/SEEK_DATA. Any other cases should be passsed
  17. to generic_file_llseek().
  18.  
  19. btrfs_file_llseek() and ocfs2_file_llseek() are extremely similar and
  20. consequently, contain many of the same flaws. Li Dongyang filed a pull
  21. request with ZFSOnLinux for SEEK_HOLE/SEEK_DATA support that included a
  22. custom llseek function that appears to have been modelled after the one
  23. in ocfs2. The similarity was strong enough that it suffered from many of
  24. the same flaws, which I caught during review. I addressed the issues
  25. with his patch with one that I wrote. Since a small percentage of Gentoo
  26. Linux users are affected by these flaws, I decided to adapt that code
  27. that to btrfs (separate patch) and ocfs2.
  28.  
  29. Note that commit 48802c8ae2a9d618ec734a61283d645ad527e06c by Jeff Liu at
  30. Oracle mostly addressed #3 in btrfs. The only lingering issue was that
  31. the offset > inode->i_sb->s_maxbytes check became dead code. The ocfs2
  32. code was not fortunate enough to have had a similar correction until
  33. now.
  34.  
  35. Signed-off-by: Richard Yao <ryao@gentoo.org>
  36. ---
  37. fs/ocfs2/file.c | 65 ++++++++++++++++++++++-----------------------------------
  38. 1 file changed, 25 insertions(+), 40 deletions(-)
  39.  
  40. diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
  41. index ff54014..84f8c9c 100644
  42. --- a/fs/ocfs2/file.c
  43. +++ b/fs/ocfs2/file.c
  44. @@ -2615,54 +2615,39 @@ bail:
  45. }
  46.  
  47. /* Refer generic_file_llseek_unlocked() */
  48. -static loff_t ocfs2_file_llseek(struct file *file, loff_t offset, int whence)
  49. +static loff_t ocfs2_file_llseek(struct file *filp, loff_t offset, int whence)
  50. {
  51. - struct inode *inode = file->f_mapping->host;
  52. - int ret = 0;
  53. + if (whence == SEEK_DATA || whence == SEEK_HOLE) {
  54. + struct inode *inode = filp->f_mapping->host;
  55. + int ret;
  56.  
  57. - mutex_lock(&inode->i_mutex);
  58. + if (offset < 0 && !(filp->f_mode & FMODE_UNSIGNED_OFFSET))
  59. + return -EINVAL;
  60.  
  61. - switch (whence) {
  62. - case SEEK_SET:
  63. - break;
  64. - case SEEK_END:
  65. - offset += inode->i_size;
  66. - break;
  67. - case SEEK_CUR:
  68. - if (offset == 0) {
  69. - offset = file->f_pos;
  70. - goto out;
  71. + if (offset >= i_size_read(inode)) {
  72. + return -ENXIO;
  73. }
  74. - offset += file->f_pos;
  75. - break;
  76. - case SEEK_DATA:
  77. - case SEEK_HOLE:
  78. - ret = ocfs2_seek_data_hole_offset(file, &offset, whence);
  79. - if (ret)
  80. - goto out;
  81. - break;
  82. - default:
  83. - ret = -EINVAL;
  84. - goto out;
  85. - }
  86.  
  87. - if (offset < 0 && !(file->f_mode & FMODE_UNSIGNED_OFFSET))
  88. - ret = -EINVAL;
  89. - if (!ret && offset > inode->i_sb->s_maxbytes)
  90. - ret = -EINVAL;
  91. - if (ret)
  92. - goto out;
  93. + mutex_lock(&inode->i_mutex);
  94. + ret = ocfs2_seek_data_hole_offset(filp, &offset, whence);
  95. + mutex_unlock(&inode->i_mutex);
  96. +
  97. + if (ret) {
  98. + return ret;
  99. + }
  100.  
  101. - if (offset != file->f_pos) {
  102. - file->f_pos = offset;
  103. - file->f_version = 0;
  104. + if (offset != filp->f_pos) {
  105. + spin_lock(&filp->f_lock);
  106. + filp->f_pos = offset;
  107. + filp->f_version = 0;
  108. + spin_unlock(&filp->f_lock);
  109. + }
  110. +
  111. + return offset;
  112. }
  113.  
  114. -out:
  115. - mutex_unlock(&inode->i_mutex);
  116. - if (ret)
  117. - return ret;
  118. - return offset;
  119. + return generic_file_llseek(filp, offset, whence);
  120. +
  121. }
  122.  
  123. const struct inode_operations ocfs2_file_iops = {
  124. --
  125. 1.8.1.5
  126.  
  127. --
  128. To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
  129. the body of a message to majordomo@vger.kernel.org
  130. More majordomo info at http://vger.kernel.org/majordomo-info.html
Advertisement
Add Comment
Please, Sign In to add comment
Advertisement