Skip to content

Commit d5811e6

Browse files
author
Trond Myklebust
committed
NFS: Fix size read races in truncate, fallocate and copy offload
If the pre-operation file size is read before locking the inode and quiescing O_DIRECT writes, then nfs_truncate_last_folio() might end up overwriting valid file data. Fixes: b1817b1 ("NFS: Protect against 'eof page pollution'") Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
1 parent 803e186 commit d5811e6

3 files changed

Lines changed: 27 additions & 14 deletions

File tree

fs/nfs/inode.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -716,7 +716,7 @@ nfs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
716716
{
717717
struct inode *inode = d_inode(dentry);
718718
struct nfs_fattr *fattr;
719-
loff_t oldsize = i_size_read(inode);
719+
loff_t oldsize;
720720
int error = 0;
721721
kuid_t task_uid = current_fsuid();
722722
kuid_t owner_uid = inode->i_uid;
@@ -727,6 +727,10 @@ nfs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
727727
if (attr->ia_valid & (ATTR_KILL_SUID | ATTR_KILL_SGID))
728728
attr->ia_valid &= ~ATTR_MODE;
729729

730+
if (S_ISREG(inode->i_mode))
731+
nfs_file_block_o_direct(NFS_I(inode));
732+
733+
oldsize = i_size_read(inode);
730734
if (attr->ia_valid & ATTR_SIZE) {
731735
BUG_ON(!S_ISREG(inode->i_mode));
732736

@@ -774,10 +778,8 @@ nfs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
774778
trace_nfs_setattr_enter(inode);
775779

776780
/* Write all dirty data */
777-
if (S_ISREG(inode->i_mode)) {
778-
nfs_file_block_o_direct(NFS_I(inode));
781+
if (S_ISREG(inode->i_mode))
779782
nfs_sync_inode(inode);
780-
}
781783

782784
fattr = nfs_alloc_fattr_with_label(NFS_SERVER(inode));
783785
if (fattr == NULL) {

fs/nfs/io.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ nfs_start_io_write(struct inode *inode)
8484
nfs_file_block_o_direct(NFS_I(inode));
8585
return err;
8686
}
87+
EXPORT_SYMBOL_GPL(nfs_start_io_write);
8788

8889
/**
8990
* nfs_end_io_write - declare that the buffered write operation is done
@@ -97,6 +98,7 @@ nfs_end_io_write(struct inode *inode)
9798
{
9899
up_write(&inode->i_rwsem);
99100
}
101+
EXPORT_SYMBOL_GPL(nfs_end_io_write);
100102

101103
/* Call with exclusively locked inode->i_rwsem */
102104
static void nfs_block_buffered(struct nfs_inode *nfsi, struct inode *inode)

fs/nfs/nfs42proc.c

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,6 @@ static int nfs42_proc_fallocate(struct rpc_message *msg, struct file *filep,
114114
exception.inode = inode;
115115
exception.state = lock->open_context->state;
116116

117-
nfs_file_block_o_direct(NFS_I(inode));
118117
err = nfs_sync_inode(inode);
119118
if (err)
120119
goto out;
@@ -138,13 +137,17 @@ int nfs42_proc_allocate(struct file *filep, loff_t offset, loff_t len)
138137
.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_ALLOCATE],
139138
};
140139
struct inode *inode = file_inode(filep);
141-
loff_t oldsize = i_size_read(inode);
140+
loff_t oldsize;
142141
int err;
143142

144143
if (!nfs_server_capable(inode, NFS_CAP_ALLOCATE))
145144
return -EOPNOTSUPP;
146145

147-
inode_lock(inode);
146+
err = nfs_start_io_write(inode);
147+
if (err)
148+
return err;
149+
150+
oldsize = i_size_read(inode);
148151

149152
err = nfs42_proc_fallocate(&msg, filep, offset, len);
150153

@@ -155,7 +158,7 @@ int nfs42_proc_allocate(struct file *filep, loff_t offset, loff_t len)
155158
NFS_SERVER(inode)->caps &= ~(NFS_CAP_ALLOCATE |
156159
NFS_CAP_ZERO_RANGE);
157160

158-
inode_unlock(inode);
161+
nfs_end_io_write(inode);
159162
return err;
160163
}
161164

@@ -170,7 +173,9 @@ int nfs42_proc_deallocate(struct file *filep, loff_t offset, loff_t len)
170173
if (!nfs_server_capable(inode, NFS_CAP_DEALLOCATE))
171174
return -EOPNOTSUPP;
172175

173-
inode_lock(inode);
176+
err = nfs_start_io_write(inode);
177+
if (err)
178+
return err;
174179

175180
err = nfs42_proc_fallocate(&msg, filep, offset, len);
176181
if (err == 0)
@@ -179,7 +184,7 @@ int nfs42_proc_deallocate(struct file *filep, loff_t offset, loff_t len)
179184
NFS_SERVER(inode)->caps &= ~(NFS_CAP_DEALLOCATE |
180185
NFS_CAP_ZERO_RANGE);
181186

182-
inode_unlock(inode);
187+
nfs_end_io_write(inode);
183188
return err;
184189
}
185190

@@ -189,14 +194,17 @@ int nfs42_proc_zero_range(struct file *filep, loff_t offset, loff_t len)
189194
.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_ZERO_RANGE],
190195
};
191196
struct inode *inode = file_inode(filep);
192-
loff_t oldsize = i_size_read(inode);
197+
loff_t oldsize;
193198
int err;
194199

195200
if (!nfs_server_capable(inode, NFS_CAP_ZERO_RANGE))
196201
return -EOPNOTSUPP;
197202

198-
inode_lock(inode);
203+
err = nfs_start_io_write(inode);
204+
if (err)
205+
return err;
199206

207+
oldsize = i_size_read(inode);
200208
err = nfs42_proc_fallocate(&msg, filep, offset, len);
201209
if (err == 0) {
202210
nfs_truncate_last_folio(inode->i_mapping, oldsize,
@@ -205,7 +213,7 @@ int nfs42_proc_zero_range(struct file *filep, loff_t offset, loff_t len)
205213
} else if (err == -EOPNOTSUPP)
206214
NFS_SERVER(inode)->caps &= ~NFS_CAP_ZERO_RANGE;
207215

208-
inode_unlock(inode);
216+
nfs_end_io_write(inode);
209217
return err;
210218
}
211219

@@ -416,7 +424,7 @@ static ssize_t _nfs42_proc_copy(struct file *src,
416424
struct nfs_server *src_server = NFS_SERVER(src_inode);
417425
loff_t pos_src = args->src_pos;
418426
loff_t pos_dst = args->dst_pos;
419-
loff_t oldsize_dst = i_size_read(dst_inode);
427+
loff_t oldsize_dst;
420428
size_t count = args->count;
421429
ssize_t status;
422430

@@ -461,6 +469,7 @@ static ssize_t _nfs42_proc_copy(struct file *src,
461469
&src_lock->open_context->state->flags);
462470
set_bit(NFS_CLNT_DST_SSC_COPY_STATE,
463471
&dst_lock->open_context->state->flags);
472+
oldsize_dst = i_size_read(dst_inode);
464473

465474
status = nfs4_call_sync(dst_server->client, dst_server, &msg,
466475
&args->seq_args, &res->seq_res, 0);

0 commit comments

Comments
 (0)