From: Chao Yu <yuchao0@huawei.com> To: Eric Biggers <ebiggers@kernel.org> Cc: <linux-fscrypt@vger.kernel.org>, Theodore Ts'o <tytso@mit.edu>, <linux-api@vger.kernel.org>, <linux-f2fs-devel@lists.sourceforge.net>, <linux-fsdevel@vger.kernel.org>, Jaegeuk Kim <jaegeuk@kernel.org>, <linux-ext4@vger.kernel.org>, Victor Hsieh <victorhsieh@google.com> Subject: Re: [PATCH 3/6] fs-verity: add FS_IOC_READ_VERITY_METADATA ioctl Date: Sun, 7 Feb 2021 16:32:17 +0800 [thread overview] Message-ID: <fbe787cc-fcba-7c97-d5ca-cb67345d0c8c@huawei.com> (raw) In-Reply-To: <YB+ead3SvsQy5ULH@sol.localdomain> On 2021/2/7 16:01, Eric Biggers wrote: > On Sun, Feb 07, 2021 at 03:46:43PM +0800, Chao Yu wrote: >> Hi Eric, >> >> On 2021/1/16 2:18, Eric Biggers wrote: >>> +static int f2fs_ioc_read_verity_metadata(struct file *filp, unsigned long arg) >>> +{ >>> + if (!f2fs_sb_has_verity(F2FS_I_SB(file_inode(filp)))) >>> + return -EOPNOTSUPP; >> >> One case is after we update kernel image, f2fs module may no longer support >> compress algorithm which current file was compressed with, to avoid triggering >> IO with empty compress engine (struct f2fs_compress_ops pointer): >> >> It needs to add f2fs_is_compress_backend_ready() check condition here? >> >> Thanks, >> >>> + >>> + return fsverity_ioctl_read_metadata(filp, (const void __user *)arg); >>> +} > > In that case it wouldn't have been possible to open the file, because > f2fs_file_open() checks for it. So it's not necessary to repeat the same check > in every operation on the file descriptor. Oh, yes, it's safe now. I'm thinking we need to remove the check in f2fs_file_open(), because the check will fail metadata access/update (via f{g,s}etxattr/ioctl), however original intention of that check is only to avoid syscalls to touch compressed data w/o the engine, anyway this is another topic. The whole patchset looks fine to me, feel free to add: Reviewed-by: Chao Yu <yuchao0@huawei.com> Thanks, > > - Eric > . >
WARNING: multiple messages have this Message-ID (diff)
From: Chao Yu <yuchao0@huawei.com> To: Eric Biggers <ebiggers@kernel.org> Cc: Theodore Ts'o <tytso@mit.edu>, linux-api@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org, Jaegeuk Kim <jaegeuk@kernel.org>, linux-ext4@vger.kernel.org, Victor Hsieh <victorhsieh@google.com> Subject: Re: [f2fs-dev] [PATCH 3/6] fs-verity: add FS_IOC_READ_VERITY_METADATA ioctl Date: Sun, 7 Feb 2021 16:32:17 +0800 [thread overview] Message-ID: <fbe787cc-fcba-7c97-d5ca-cb67345d0c8c@huawei.com> (raw) In-Reply-To: <YB+ead3SvsQy5ULH@sol.localdomain> On 2021/2/7 16:01, Eric Biggers wrote: > On Sun, Feb 07, 2021 at 03:46:43PM +0800, Chao Yu wrote: >> Hi Eric, >> >> On 2021/1/16 2:18, Eric Biggers wrote: >>> +static int f2fs_ioc_read_verity_metadata(struct file *filp, unsigned long arg) >>> +{ >>> + if (!f2fs_sb_has_verity(F2FS_I_SB(file_inode(filp)))) >>> + return -EOPNOTSUPP; >> >> One case is after we update kernel image, f2fs module may no longer support >> compress algorithm which current file was compressed with, to avoid triggering >> IO with empty compress engine (struct f2fs_compress_ops pointer): >> >> It needs to add f2fs_is_compress_backend_ready() check condition here? >> >> Thanks, >> >>> + >>> + return fsverity_ioctl_read_metadata(filp, (const void __user *)arg); >>> +} > > In that case it wouldn't have been possible to open the file, because > f2fs_file_open() checks for it. So it's not necessary to repeat the same check > in every operation on the file descriptor. Oh, yes, it's safe now. I'm thinking we need to remove the check in f2fs_file_open(), because the check will fail metadata access/update (via f{g,s}etxattr/ioctl), however original intention of that check is only to avoid syscalls to touch compressed data w/o the engine, anyway this is another topic. The whole patchset looks fine to me, feel free to add: Reviewed-by: Chao Yu <yuchao0@huawei.com> Thanks, > > - Eric > . > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
next prev parent reply other threads:[~2021-02-07 8:35 UTC|newest] Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-01-15 18:18 [PATCH 0/6] fs-verity: add an ioctl to read verity metadata Eric Biggers 2021-01-15 18:18 ` [f2fs-dev] " Eric Biggers 2021-01-15 18:18 ` [PATCH 1/6] fs-verity: factor out fsverity_get_descriptor() Eric Biggers 2021-01-15 18:18 ` [f2fs-dev] " Eric Biggers 2021-01-28 1:04 ` Jaegeuk Kim 2021-01-28 1:04 ` [f2fs-dev] " Jaegeuk Kim 2021-01-15 18:18 ` [PATCH 2/6] fs-verity: don't pass whole descriptor to fsverity_verify_signature() Eric Biggers 2021-01-15 18:18 ` [f2fs-dev] " Eric Biggers 2021-01-28 1:04 ` Jaegeuk Kim 2021-01-28 1:04 ` [f2fs-dev] " Jaegeuk Kim 2021-01-28 3:24 ` Amy Parker 2021-01-28 3:24 ` [f2fs-dev] " Amy Parker 2021-01-15 18:18 ` [PATCH 3/6] fs-verity: add FS_IOC_READ_VERITY_METADATA ioctl Eric Biggers 2021-01-15 18:18 ` [f2fs-dev] " Eric Biggers 2021-01-28 1:03 ` Jaegeuk Kim 2021-01-28 1:03 ` [f2fs-dev] " Jaegeuk Kim 2021-02-07 7:46 ` Chao Yu 2021-02-07 7:46 ` Chao Yu 2021-02-07 8:01 ` Eric Biggers 2021-02-07 8:01 ` [f2fs-dev] " Eric Biggers 2021-02-07 8:32 ` Chao Yu [this message] 2021-02-07 8:32 ` Chao Yu 2021-01-15 18:18 ` [PATCH 4/6] fs-verity: support reading Merkle tree with ioctl Eric Biggers 2021-01-15 18:18 ` [f2fs-dev] " Eric Biggers 2021-01-28 1:10 ` Jaegeuk Kim 2021-01-28 1:10 ` [f2fs-dev] " Jaegeuk Kim 2021-01-28 2:17 ` Eric Biggers 2021-01-28 2:17 ` [f2fs-dev] " Eric Biggers 2021-01-15 18:18 ` [PATCH 5/6] fs-verity: support reading descriptor " Eric Biggers 2021-01-15 18:18 ` [f2fs-dev] " Eric Biggers 2021-01-28 1:11 ` Jaegeuk Kim 2021-01-28 1:11 ` [f2fs-dev] " Jaegeuk Kim 2021-01-15 18:18 ` [PATCH 6/6] fs-verity: support reading signature " Eric Biggers 2021-01-15 18:18 ` [f2fs-dev] " Eric Biggers 2021-01-28 1:11 ` Jaegeuk Kim 2021-01-28 1:11 ` [f2fs-dev] " Jaegeuk Kim 2021-01-22 23:26 ` [PATCH 0/6] fs-verity: add an ioctl to read verity metadata Victor Hsieh 2021-01-22 23:26 ` [f2fs-dev] " Victor Hsieh via Linux-f2fs-devel 2021-01-25 18:41 ` Eric Biggers 2021-01-25 18:41 ` [f2fs-dev] " Eric Biggers 2021-02-01 17:41 ` Eric Biggers 2021-02-01 17:41 ` [f2fs-dev] " Eric Biggers
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=fbe787cc-fcba-7c97-d5ca-cb67345d0c8c@huawei.com \ --to=yuchao0@huawei.com \ --cc=ebiggers@kernel.org \ --cc=jaegeuk@kernel.org \ --cc=linux-api@vger.kernel.org \ --cc=linux-ext4@vger.kernel.org \ --cc=linux-f2fs-devel@lists.sourceforge.net \ --cc=linux-fscrypt@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=tytso@mit.edu \ --cc=victorhsieh@google.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.