On Fri 21-06-19 16:56:58, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> The chattr manpage has this to say about immutable files:
>
> "A file with the 'i' attribute cannot be modified: it cannot be deleted
> or renamed, no link can be created to this file, most of the file's
> metadata can not be modified, and the file can not be opened in write
> mode."
>
> Once the flag is set, it is enforced for quite a few file operations,
> such as fallocate, fpunch, fzero, rm, touch, open, etc. However, we
> don't check for immutability when doing a write(), a PROT_WRITE mmap(),
> a truncate(), or a write to a previously established mmap.
>
> If a program has an open write fd to a file that the administrator
> subsequently marks immutable, the program still can change the file
> contents. Weird!
>
> The ability to write to an immutable file does not follow the manpage
> promise that immutable files cannot be modified. Worse yet it's
> inconsistent with the behavior of other syscalls which don't allow
> modifications of immutable files.
>
> Therefore, add the necessary checks to make the write, mmap, and
> truncate behavior consistent with what the manpage says and consistent
> with other syscalls on filesystems which support IMMUTABLE.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
Looks good to me. You can add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> ---
> fs/attr.c | 13 ++++++-------
> mm/filemap.c | 3 +++
> mm/memory.c | 3 +++
> mm/mmap.c | 8 ++++++--
> 4 files changed, 18 insertions(+), 9 deletions(-)
>
>
> diff --git a/fs/attr.c b/fs/attr.c
> index d22e8187477f..1fcfdcc5b367 100644
> --- a/fs/attr.c
> +++ b/fs/attr.c
> @@ -233,19 +233,18 @@ int notify_change(struct dentry * dentry, struct iattr * attr, struct inode **de
>
> WARN_ON_ONCE(!inode_is_locked(inode));
>
> - if (ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID | ATTR_TIMES_SET)) {
> - if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
> - return -EPERM;
> - }
> + if (IS_IMMUTABLE(inode))
> + return -EPERM;
> +
> + if ((ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID | ATTR_TIMES_SET)) &&
> + IS_APPEND(inode))
> + return -EPERM;
>
> /*
> * If utimes(2) and friends are called with times == NULL (or both
> * times are UTIME_NOW), then we need to check for write permission
> */
> if (ia_valid & ATTR_TOUCH) {
> - if (IS_IMMUTABLE(inode))
> - return -EPERM;
> -
> if (!inode_owner_or_capable(inode)) {
> error = inode_permission(inode, MAY_WRITE);
> if (error)
> diff --git a/mm/filemap.c b/mm/filemap.c
> index aac71aef4c61..dad85e10f5f8 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -2935,6 +2935,9 @@ inline ssize_t generic_write_checks(struct kiocb *iocb, struct iov_iter *from)
> loff_t count;
> int ret;
>
> + if (IS_IMMUTABLE(inode))
> + return -EPERM;
> +
> if (!iov_iter_count(from))
> return 0;
>
> diff --git a/mm/memory.c b/mm/memory.c
> index ddf20bd0c317..4311cfdade90 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2235,6 +2235,9 @@ static vm_fault_t do_page_mkwrite(struct vm_fault *vmf)
>
> vmf->flags = FAULT_FLAG_WRITE|FAULT_FLAG_MKWRITE;
>
> + if (vmf->vma->vm_file && IS_IMMUTABLE(file_inode(vmf->vma->vm_file)))
> + return VM_FAULT_SIGBUS;
> +
> ret = vmf->vma->vm_ops->page_mkwrite(vmf);
> /* Restore original flags so that caller is not surprised */
> vmf->flags = old_flags;
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 7e8c3e8ae75f..ac1e32205237 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1483,8 +1483,12 @@ unsigned long do_mmap(struct file *file, unsigned long addr,
> case MAP_SHARED_VALIDATE:
> if (flags & ~flags_mask)
> return -EOPNOTSUPP;
> - if ((prot&PROT_WRITE) && !(file->f_mode&FMODE_WRITE))
> - return -EACCES;
> + if (prot & PROT_WRITE) {
> + if (!(file->f_mode & FMODE_WRITE))
> + return -EACCES;
> + if (IS_IMMUTABLE(file_inode(file)))
> + return -EPERM;
> + }
>
> /*
> * Make sure we don't allow writing to an append-only
>
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
