On Fri, Apr 7, 2017 at 4:21 PM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote: > On Fri, Apr 7, 2017 at 11:01 AM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: >> truncate an overlayfs inode was checking IS_APPEND() on >> overlay inode, but overlay inode does not have the S_APPEND flag. >> >> Move the IS_APPEND() check to after we have the upperdentry >> and pass it the real upper inode. > > Not sure the reordering is worth it. Just use d_real_inode() in the Hmm, I meant move after d_real() which actually does a copy up. If we use d_real_inode() before d_real() then we prevent truncate of a lower with chattr +a. That is not consistent at all with open(O_TRUNC) and ftruncate and with checks for IS_IMMUTABLE() which only test upper (and I think it is better this way). > IS_APPEND() check. OTOH it shouldn't matter (well, except whether the > file is copied up or not in the error caae ). But mainly I just feel > when there's a choice of a simpler way we should use that. > > Also it's usually better not to mix fixes with cleanups (the d_inode() thing). > Sure, I can split that or drop the cleanup altogether. > Thanks, > Miklos > >> >> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> >> --- >> fs/open.c | 12 ++++++------ >> 1 file changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/fs/open.c b/fs/open.c >> index b7d5ab1..425db52 100644 >> --- a/fs/open.c >> +++ b/fs/open.c >> @@ -87,10 +87,6 @@ long vfs_truncate(const struct path *path, loff_t length) >> if (error) >> goto mnt_drop_write_and_out; >> >> - error = -EPERM; >> - if (IS_APPEND(inode)) >> - goto mnt_drop_write_and_out; >> - >> /* >> * If this is an overlayfs then do as if opening the file so we get >> * write access on the upper inode, not on the overlay inode. For >> @@ -101,7 +97,11 @@ long vfs_truncate(const struct path *path, loff_t length) >> if (IS_ERR(upperdentry)) >> goto mnt_drop_write_and_out; >> >> - error = get_write_access(upperdentry->d_inode); >> + error = -EPERM; >> + if (IS_APPEND(d_inode(upperdentry))) >> + goto mnt_drop_write_and_out; >> + >> + error = get_write_access(d_inode(upperdentry)); >> if (error) >> goto mnt_drop_write_and_out; >> >> @@ -120,7 +120,7 @@ long vfs_truncate(const struct path *path, loff_t length) >> error = do_truncate(path->dentry, length, 0, NULL); >> >> put_write_and_out: >> - put_write_access(upperdentry->d_inode); >> + put_write_access(d_inode(upperdentry)); >> mnt_drop_write_and_out: >> mnt_drop_write(path->mnt); >> out: >> -- >> 2.7.4 >>
