| From jejb@kernel.org Wed Sep 3 08:10:42 2008 |
| From: Jeff Layton <jlayton@redhat.com> |
| Date: Tue, 2 Sep 2008 19:25:05 GMT |
| Subject: cifs: fix O_APPEND on directio mounts |
| To: jejb@kernel.org, stable@kernel.org |
| Message-ID: <200809021925.m82JP5Xb008422@hera.kernel.org> |
| |
| From: Jeff Layton <jlayton@redhat.com> |
| |
| commit 838726c4756813576078203eb7e1e219db0da870 upstream |
| |
| The direct I/O write codepath for CIFS is done through |
| cifs_user_write(). That function does not currently call |
| generic_write_checks() so the file position isn't being properly set |
| when the file is opened with O_APPEND. It's also not doing the other |
| "normal" checks that should be done for a write call. |
| |
| The problem is currently that when you open a file with O_APPEND on a |
| mount with the directio mount option, the file position is set to the |
| beginning of the file. This makes any subsequent writes clobber the data |
| in the file starting at the beginning. |
| |
| This seems to fix the problem in cursory testing. It is, however |
| important to note that NFS disallows the combination of |
| (O_DIRECT|O_APPEND). If my understanding is correct, the concern is |
| races with multiple clients appending to a file clobbering each others' |
| data. Since the write model for CIFS and NFS is pretty similar in this |
| regard, CIFS is probably subject to the same sort of races. What's |
| unclear to me is why this is a particular problem with O_DIRECT and not |
| with buffered writes... |
| |
| Regardless, disallowing O_APPEND on an entire mount is probably not |
| reasonable, so we'll probably just have to deal with it and reevaluate |
| this flag combination when we get proper support for O_DIRECT. In the |
| meantime this patch at least fixes the existing problem. |
| |
| Signed-off-by: Jeff Layton <jlayton@redhat.com> |
| Signed-off-by: Steve French <sfrench@us.ibm.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> |
| |
| --- |
| fs/cifs/file.c | 4 ++++ |
| 1 file changed, 4 insertions(+) |
| |
| --- a/fs/cifs/file.c |
| +++ b/fs/cifs/file.c |
| @@ -835,6 +835,10 @@ ssize_t cifs_user_write(struct file *fil |
| return -EBADF; |
| open_file = (struct cifsFileInfo *) file->private_data; |
| |
| + rc = generic_write_checks(file, poffset, &write_size, 0); |
| + if (rc) |
| + return rc; |
| + |
| xid = GetXid(); |
| |
| if (*poffset > file->f_path.dentry->d_inode->i_size) |