On Tue, Jun 09, 2020 at 03:01:37PM +0900, Daeho Jeong wrote:
> From: Daeho Jeong <[email protected]>
>
> Added a new ioctl to send discard commands or/and zero out
> to whole data area of a regular file for security reason.
With this ioctl available, what is the exact procedure to write and then later
securely erase a file on f2fs? In particular, how can the user prevent f2fs
from making multiple copies of file data blocks as part of garbage collection?
> +static int f2fs_secure_erase(struct block_device *bdev, block_t block,
> + block_t len, u32 flags)
> +{
> + struct request_queue *q = bdev_get_queue(bdev);
> + sector_t sector = SECTOR_FROM_BLOCK(block);
> + sector_t nr_sects = SECTOR_FROM_BLOCK(len);
> + int ret = 0;
> +
> + if (!q)
> + return -ENXIO;
Why can the request_queue be NULL here?
> +
> + if (flags & F2FS_TRIM_FILE_DISCARD)
> + ret = blkdev_issue_discard(bdev, sector, nr_sects, GFP_NOFS,
> + blk_queue_secure_erase(q) ?
> + BLKDEV_DISCARD_SECURE : 0);
> +
> + if (!ret && flags & F2FS_TRIM_FILE_ZEROOUT)
> + ret = blkdev_issue_zeroout(bdev, sector, nr_sects, GFP_NOFS, 0);
> +
> + return ret;
> +}
> +
> +static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
> +{
> + struct inode *inode = file_inode(filp);
> + struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> + struct address_space *mapping = inode->i_mapping;
> + struct block_device *prev_bdev = NULL;
> + loff_t file_size;
> + pgoff_t index, pg_start = 0, pg_end;
> + block_t prev_block = 0, len = 0;
> + u32 flags;
> + int ret = 0;
> +
> + if (!S_ISREG(inode->i_mode) || f2fs_is_atomic_file(inode) ||
> + f2fs_compressed_file(inode))
> + return -EINVAL;
Is it valid to check f2fs_is_atomic_file() and f2fs_compressed_file() outside of
inode_lock()?
> +
> + if (f2fs_readonly(sbi->sb))
> + return -EROFS;
Isn't this redundant with mnt_want_write_file()?
Also, shouldn't write access to the file be required, i.e.
(filp->f_mode & FMODE_WRITE)? Then the f2fs_readonly() and
mnt_want_write_file() checks would be unnecessary.
> +
> + if (f2fs_lfs_mode(sbi))
> + return -EOPNOTSUPP;
Doesn't this check have to be serialized with remount?
> +
> + if (get_user(flags, (u32 __user *)arg))
> + return -EFAULT;
> + if (!(flags & F2FS_TRIM_FILE_MASK))
> + return -EINVAL;
Need to reject unknown flags:
if (flags & ~F2FS_TRIM_FILE_MASK)
return -EINVAL;
> +
> + if (flags & F2FS_TRIM_FILE_DISCARD && !f2fs_hw_support_discard(sbi))
> + return -EOPNOTSUPP;
> +
> + ret = mnt_want_write_file(filp);
> + if (ret)
> + return ret;
> +
> + inode_lock(inode);
> +
> + file_size = i_size_read(inode);
> + if (!file_size)
> + goto err;
->i_size is stable while holding inode_lock(). So just use ->i_size instead of
i_size_read().
> + pg_end = (pgoff_t)round_up(file_size, PAGE_SIZE) >> PAGE_SHIFT;
This can be simplified to:
pg_end = DIV_ROUND_UP(file_size, PAGE_SIZE);
- Eric
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
