Skip to content

Commit fbc04bf

Browse files
committed
Merge tag 'xfs-5.17-fixes-1' of git://git.kernel.org/pub/scm/fs/xfs/xfs-linux
Pull xfs fixes from Darrick Wong: "I was auditing operations in XFS that clear file privileges, and realized that XFS' fallocate implementation drops suid/sgid but doesn't clear file capabilities the same way that file writes and reflink do. There are VFS helpers that do it correctly, so refactor XFS to use them. I also noticed that we weren't flushing the log at the correct point in the fallocate operation, so that's fixed too. Summary: - Fix fallocate so that it drops all file privileges when files are modified instead of open-coding that incompletely. - Fix fallocate to flush the log if the caller wanted synchronous file updates" * tag 'xfs-5.17-fixes-1' of git://git.kernel.org/pub/scm/fs/xfs/xfs-linux: xfs: ensure log flush at the end of a synchronous fallocate call xfs: move xfs_update_prealloc_flags() to xfs_pnfs.c xfs: set prealloc flag in xfs_alloc_file_space() xfs: fallocate() should call file_modified() xfs: remove XFS_PREALLOC_SYNC xfs: reject crazy array sizes being fed to XFS_IOC_GETBMAP*
2 parents ea7b3e6 + cea267c commit fbc04bf

File tree

5 files changed

+69
-79
lines changed

5 files changed

+69
-79
lines changed

fs/xfs/xfs_bmap_util.c

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -850,9 +850,6 @@ xfs_alloc_file_space(
850850
rblocks = 0;
851851
}
852852

853-
/*
854-
* Allocate and setup the transaction.
855-
*/
856853
error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_write,
857854
dblocks, rblocks, false, &tp);
858855
if (error)
@@ -869,9 +866,9 @@ xfs_alloc_file_space(
869866
if (error)
870867
goto error;
871868

872-
/*
873-
* Complete the transaction
874-
*/
869+
ip->i_diflags |= XFS_DIFLAG_PREALLOC;
870+
xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
871+
875872
error = xfs_trans_commit(tp);
876873
xfs_iunlock(ip, XFS_ILOCK_EXCL);
877874
if (error)

fs/xfs/xfs_file.c

Lines changed: 26 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -66,40 +66,6 @@ xfs_is_falloc_aligned(
6666
return !((pos | len) & mask);
6767
}
6868

69-
int
70-
xfs_update_prealloc_flags(
71-
struct xfs_inode *ip,
72-
enum xfs_prealloc_flags flags)
73-
{
74-
struct xfs_trans *tp;
75-
int error;
76-
77-
error = xfs_trans_alloc(ip->i_mount, &M_RES(ip->i_mount)->tr_writeid,
78-
0, 0, 0, &tp);
79-
if (error)
80-
return error;
81-
82-
xfs_ilock(ip, XFS_ILOCK_EXCL);
83-
xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
84-
85-
if (!(flags & XFS_PREALLOC_INVISIBLE)) {
86-
VFS_I(ip)->i_mode &= ~S_ISUID;
87-
if (VFS_I(ip)->i_mode & S_IXGRP)
88-
VFS_I(ip)->i_mode &= ~S_ISGID;
89-
xfs_trans_ichgtime(tp, ip, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
90-
}
91-
92-
if (flags & XFS_PREALLOC_SET)
93-
ip->i_diflags |= XFS_DIFLAG_PREALLOC;
94-
if (flags & XFS_PREALLOC_CLEAR)
95-
ip->i_diflags &= ~XFS_DIFLAG_PREALLOC;
96-
97-
xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
98-
if (flags & XFS_PREALLOC_SYNC)
99-
xfs_trans_set_sync(tp);
100-
return xfs_trans_commit(tp);
101-
}
102-
10369
/*
10470
* Fsync operations on directories are much simpler than on regular files,
10571
* as there is no file data to flush, and thus also no need for explicit
@@ -895,6 +861,21 @@ xfs_break_layouts(
895861
return error;
896862
}
897863

864+
/* Does this file, inode, or mount want synchronous writes? */
865+
static inline bool xfs_file_sync_writes(struct file *filp)
866+
{
867+
struct xfs_inode *ip = XFS_I(file_inode(filp));
868+
869+
if (xfs_has_wsync(ip->i_mount))
870+
return true;
871+
if (filp->f_flags & (__O_SYNC | O_DSYNC))
872+
return true;
873+
if (IS_SYNC(file_inode(filp)))
874+
return true;
875+
876+
return false;
877+
}
878+
898879
#define XFS_FALLOC_FL_SUPPORTED \
899880
(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE | \
900881
FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_ZERO_RANGE | \
@@ -910,7 +891,6 @@ xfs_file_fallocate(
910891
struct inode *inode = file_inode(file);
911892
struct xfs_inode *ip = XFS_I(inode);
912893
long error;
913-
enum xfs_prealloc_flags flags = 0;
914894
uint iolock = XFS_IOLOCK_EXCL | XFS_MMAPLOCK_EXCL;
915895
loff_t new_size = 0;
916896
bool do_file_insert = false;
@@ -955,6 +935,10 @@ xfs_file_fallocate(
955935
goto out_unlock;
956936
}
957937

938+
error = file_modified(file);
939+
if (error)
940+
goto out_unlock;
941+
958942
if (mode & FALLOC_FL_PUNCH_HOLE) {
959943
error = xfs_free_file_space(ip, offset, len);
960944
if (error)
@@ -1004,8 +988,6 @@ xfs_file_fallocate(
1004988
}
1005989
do_file_insert = true;
1006990
} else {
1007-
flags |= XFS_PREALLOC_SET;
1008-
1009991
if (!(mode & FALLOC_FL_KEEP_SIZE) &&
1010992
offset + len > i_size_read(inode)) {
1011993
new_size = offset + len;
@@ -1057,13 +1039,6 @@ xfs_file_fallocate(
10571039
}
10581040
}
10591041

1060-
if (file->f_flags & O_DSYNC)
1061-
flags |= XFS_PREALLOC_SYNC;
1062-
1063-
error = xfs_update_prealloc_flags(ip, flags);
1064-
if (error)
1065-
goto out_unlock;
1066-
10671042
/* Change file size if needed */
10681043
if (new_size) {
10691044
struct iattr iattr;
@@ -1082,8 +1057,14 @@ xfs_file_fallocate(
10821057
* leave shifted extents past EOF and hence losing access to
10831058
* the data that is contained within them.
10841059
*/
1085-
if (do_file_insert)
1060+
if (do_file_insert) {
10861061
error = xfs_insert_file_space(ip, offset, len);
1062+
if (error)
1063+
goto out_unlock;
1064+
}
1065+
1066+
if (xfs_file_sync_writes(file))
1067+
error = xfs_log_force_inode(ip);
10871068

10881069
out_unlock:
10891070
xfs_iunlock(ip, iolock);
@@ -1115,21 +1096,6 @@ xfs_file_fadvise(
11151096
return ret;
11161097
}
11171098

1118-
/* Does this file, inode, or mount want synchronous writes? */
1119-
static inline bool xfs_file_sync_writes(struct file *filp)
1120-
{
1121-
struct xfs_inode *ip = XFS_I(file_inode(filp));
1122-
1123-
if (xfs_has_wsync(ip->i_mount))
1124-
return true;
1125-
if (filp->f_flags & (__O_SYNC | O_DSYNC))
1126-
return true;
1127-
if (IS_SYNC(file_inode(filp)))
1128-
return true;
1129-
1130-
return false;
1131-
}
1132-
11331099
STATIC loff_t
11341100
xfs_file_remap_range(
11351101
struct file *file_in,

fs/xfs/xfs_inode.h

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -462,15 +462,6 @@ xfs_itruncate_extents(
462462
}
463463

464464
/* from xfs_file.c */
465-
enum xfs_prealloc_flags {
466-
XFS_PREALLOC_SET = (1 << 1),
467-
XFS_PREALLOC_CLEAR = (1 << 2),
468-
XFS_PREALLOC_SYNC = (1 << 3),
469-
XFS_PREALLOC_INVISIBLE = (1 << 4),
470-
};
471-
472-
int xfs_update_prealloc_flags(struct xfs_inode *ip,
473-
enum xfs_prealloc_flags flags);
474465
int xfs_break_layouts(struct inode *inode, uint *iolock,
475466
enum layout_break_reason reason);
476467

fs/xfs/xfs_ioctl.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1464,7 +1464,7 @@ xfs_ioc_getbmap(
14641464

14651465
if (bmx.bmv_count < 2)
14661466
return -EINVAL;
1467-
if (bmx.bmv_count > ULONG_MAX / recsize)
1467+
if (bmx.bmv_count >= INT_MAX / recsize)
14681468
return -ENOMEM;
14691469

14701470
buf = kvcalloc(bmx.bmv_count, sizeof(*buf), GFP_KERNEL);

fs/xfs/xfs_pnfs.c

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,40 @@ xfs_fs_get_uuid(
7070
return 0;
7171
}
7272

73+
/*
74+
* We cannot use file based VFS helpers such as file_modified() to update
75+
* inode state as we modify the data/metadata in the inode here. Hence we have
76+
* to open code the timestamp updates and SUID/SGID stripping. We also need
77+
* to set the inode prealloc flag to ensure that the extents we allocate are not
78+
* removed if the inode is reclaimed from memory before xfs_fs_block_commit()
79+
* is from the client to indicate that data has been written and the file size
80+
* can be extended.
81+
*/
82+
static int
83+
xfs_fs_map_update_inode(
84+
struct xfs_inode *ip)
85+
{
86+
struct xfs_trans *tp;
87+
int error;
88+
89+
error = xfs_trans_alloc(ip->i_mount, &M_RES(ip->i_mount)->tr_writeid,
90+
0, 0, 0, &tp);
91+
if (error)
92+
return error;
93+
94+
xfs_ilock(ip, XFS_ILOCK_EXCL);
95+
xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
96+
97+
VFS_I(ip)->i_mode &= ~S_ISUID;
98+
if (VFS_I(ip)->i_mode & S_IXGRP)
99+
VFS_I(ip)->i_mode &= ~S_ISGID;
100+
xfs_trans_ichgtime(tp, ip, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
101+
ip->i_diflags |= XFS_DIFLAG_PREALLOC;
102+
103+
xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
104+
return xfs_trans_commit(tp);
105+
}
106+
73107
/*
74108
* Get a layout for the pNFS client.
75109
*/
@@ -164,10 +198,12 @@ xfs_fs_map_blocks(
164198
* that the blocks allocated and handed out to the client are
165199
* guaranteed to be present even after a server crash.
166200
*/
167-
error = xfs_update_prealloc_flags(ip,
168-
XFS_PREALLOC_SET | XFS_PREALLOC_SYNC);
201+
error = xfs_fs_map_update_inode(ip);
202+
if (!error)
203+
error = xfs_log_force_inode(ip);
169204
if (error)
170205
goto out_unlock;
206+
171207
} else {
172208
xfs_iunlock(ip, lock_flags);
173209
}
@@ -255,7 +291,7 @@ xfs_fs_commit_blocks(
255291
length = end - start;
256292
if (!length)
257293
continue;
258-
294+
259295
/*
260296
* Make sure reads through the pagecache see the new data.
261297
*/

0 commit comments

Comments
 (0)