* [PATCH 0/4] Assorted UBIFS fixes @ 2021-01-22 21:22 ` Richard Weinberger 0 siblings, 0 replies; 22+ messages in thread From: Richard Weinberger @ 2021-01-22 21:22 UTC (permalink / raw) To: linux-mtd; +Cc: richard, david, linux-kernel I'm currently hunting down a filesystem corruption, while reviewing various parts of UBIFS I've found some other bugs. This patches fix these bugs. In another series I'll add a feature to be able to remove stale fscrypt contexts and wrong directory size counters. Richard Weinberger (4): ubifs: Correctly set inode size in ubifs_link() ubifs: Don't add fscrypt context to xattrs ubifs: Update directory size when creating whiteouts ubifs: Harden ubifs_jnl_write_inode() fs/ubifs/dir.c | 31 ++++++++++++++++++++----------- fs/ubifs/journal.c | 6 +++++- fs/ubifs/ubifs.h | 2 +- fs/ubifs/xattr.c | 2 +- 4 files changed, 27 insertions(+), 14 deletions(-) -- 2.26.2 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 0/4] Assorted UBIFS fixes @ 2021-01-22 21:22 ` Richard Weinberger 0 siblings, 0 replies; 22+ messages in thread From: Richard Weinberger @ 2021-01-22 21:22 UTC (permalink / raw) To: linux-mtd; +Cc: david, richard, linux-kernel I'm currently hunting down a filesystem corruption, while reviewing various parts of UBIFS I've found some other bugs. This patches fix these bugs. In another series I'll add a feature to be able to remove stale fscrypt contexts and wrong directory size counters. Richard Weinberger (4): ubifs: Correctly set inode size in ubifs_link() ubifs: Don't add fscrypt context to xattrs ubifs: Update directory size when creating whiteouts ubifs: Harden ubifs_jnl_write_inode() fs/ubifs/dir.c | 31 ++++++++++++++++++++----------- fs/ubifs/journal.c | 6 +++++- fs/ubifs/ubifs.h | 2 +- fs/ubifs/xattr.c | 2 +- 4 files changed, 27 insertions(+), 14 deletions(-) -- 2.26.2 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/4] ubifs: Correctly set inode size in ubifs_link() 2021-01-22 21:22 ` Richard Weinberger @ 2021-01-22 21:22 ` Richard Weinberger -1 siblings, 0 replies; 22+ messages in thread From: Richard Weinberger @ 2021-01-22 21:22 UTC (permalink / raw) To: linux-mtd; +Cc: richard, david, linux-kernel, stable We need to use fscrypt filename handling wrappers when calculating the size of a directory entry, otherwise UBIFS will report the wrong value (size of plain instead of cihper text) for st_size of a directory if fscrypt is enabled. Cc: stable@vger.kernel.org Fixes: f4f61d2cc6d8 ("ubifs: Implement encrypted filenames") Signed-off-by: Richard Weinberger <richard@nod.at> --- fs/ubifs/dir.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c index 9a6b8660425a..04912dedca48 100644 --- a/fs/ubifs/dir.c +++ b/fs/ubifs/dir.c @@ -693,7 +693,7 @@ static int ubifs_link(struct dentry *old_dentry, struct inode *dir, struct inode *inode = d_inode(old_dentry); struct ubifs_inode *ui = ubifs_inode(inode); struct ubifs_inode *dir_ui = ubifs_inode(dir); - int err, sz_change = CALC_DENT_SIZE(dentry->d_name.len); + int err, sz_change; struct ubifs_budget_req req = { .new_dent = 1, .dirtied_ino = 2, .dirtied_ino_d = ALIGN(ui->data_len, 8) }; struct fscrypt_name nm; @@ -731,6 +731,8 @@ static int ubifs_link(struct dentry *old_dentry, struct inode *dir, if (inode->i_nlink == 0) ubifs_delete_orphan(c, inode->i_ino); + sz_change = CALC_DENT_SIZE(fname_len(&nm)); + inc_nlink(inode); ihold(inode); inode->i_ctime = current_time(inode); -- 2.26.2 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 1/4] ubifs: Correctly set inode size in ubifs_link() @ 2021-01-22 21:22 ` Richard Weinberger 0 siblings, 0 replies; 22+ messages in thread From: Richard Weinberger @ 2021-01-22 21:22 UTC (permalink / raw) To: linux-mtd; +Cc: david, richard, linux-kernel, stable We need to use fscrypt filename handling wrappers when calculating the size of a directory entry, otherwise UBIFS will report the wrong value (size of plain instead of cihper text) for st_size of a directory if fscrypt is enabled. Cc: stable@vger.kernel.org Fixes: f4f61d2cc6d8 ("ubifs: Implement encrypted filenames") Signed-off-by: Richard Weinberger <richard@nod.at> --- fs/ubifs/dir.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c index 9a6b8660425a..04912dedca48 100644 --- a/fs/ubifs/dir.c +++ b/fs/ubifs/dir.c @@ -693,7 +693,7 @@ static int ubifs_link(struct dentry *old_dentry, struct inode *dir, struct inode *inode = d_inode(old_dentry); struct ubifs_inode *ui = ubifs_inode(inode); struct ubifs_inode *dir_ui = ubifs_inode(dir); - int err, sz_change = CALC_DENT_SIZE(dentry->d_name.len); + int err, sz_change; struct ubifs_budget_req req = { .new_dent = 1, .dirtied_ino = 2, .dirtied_ino_d = ALIGN(ui->data_len, 8) }; struct fscrypt_name nm; @@ -731,6 +731,8 @@ static int ubifs_link(struct dentry *old_dentry, struct inode *dir, if (inode->i_nlink == 0) ubifs_delete_orphan(c, inode->i_ino); + sz_change = CALC_DENT_SIZE(fname_len(&nm)); + inc_nlink(inode); ihold(inode); inode->i_ctime = current_time(inode); -- 2.26.2 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] ubifs: Correctly set inode size in ubifs_link() 2021-01-22 21:22 ` Richard Weinberger @ 2021-01-23 2:35 ` Zhihao Cheng -1 siblings, 0 replies; 22+ messages in thread From: Zhihao Cheng @ 2021-01-23 2:35 UTC (permalink / raw) To: Richard Weinberger, linux-mtd; +Cc: david, linux-kernel, stable 在 2021/1/23 5:22, Richard Weinberger 写道: Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com> > diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c > index 9a6b8660425a..04912dedca48 100644 > --- a/fs/ubifs/dir.c > +++ b/fs/ubifs/dir.c > @@ -693,7 +693,7 @@ static int ubifs_link(struct dentry *old_dentry, struct inode *dir, > struct inode *inode = d_inode(old_dentry); > struct ubifs_inode *ui = ubifs_inode(inode); > struct ubifs_inode *dir_ui = ubifs_inode(dir); > - int err, sz_change = CALC_DENT_SIZE(dentry->d_name.len); > + int err, sz_change; > struct ubifs_budget_req req = { .new_dent = 1, .dirtied_ino = 2, > .dirtied_ino_d = ALIGN(ui->data_len, 8) }; > struct fscrypt_name nm; > @@ -731,6 +731,8 @@ static int ubifs_link(struct dentry *old_dentry, struct inode *dir, > if (inode->i_nlink == 0) > ubifs_delete_orphan(c, inode->i_ino); > > + sz_change = CALC_DENT_SIZE(fname_len(&nm)); > + > inc_nlink(inode); > ihold(inode); > inode->i_ctime = current_time(inode); > ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] ubifs: Correctly set inode size in ubifs_link() @ 2021-01-23 2:35 ` Zhihao Cheng 0 siblings, 0 replies; 22+ messages in thread From: Zhihao Cheng @ 2021-01-23 2:35 UTC (permalink / raw) To: Richard Weinberger, linux-mtd; +Cc: david, linux-kernel, stable 在 2021/1/23 5:22, Richard Weinberger 写道: Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com> > diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c > index 9a6b8660425a..04912dedca48 100644 > --- a/fs/ubifs/dir.c > +++ b/fs/ubifs/dir.c > @@ -693,7 +693,7 @@ static int ubifs_link(struct dentry *old_dentry, struct inode *dir, > struct inode *inode = d_inode(old_dentry); > struct ubifs_inode *ui = ubifs_inode(inode); > struct ubifs_inode *dir_ui = ubifs_inode(dir); > - int err, sz_change = CALC_DENT_SIZE(dentry->d_name.len); > + int err, sz_change; > struct ubifs_budget_req req = { .new_dent = 1, .dirtied_ino = 2, > .dirtied_ino_d = ALIGN(ui->data_len, 8) }; > struct fscrypt_name nm; > @@ -731,6 +731,8 @@ static int ubifs_link(struct dentry *old_dentry, struct inode *dir, > if (inode->i_nlink == 0) > ubifs_delete_orphan(c, inode->i_ino); > > + sz_change = CALC_DENT_SIZE(fname_len(&nm)); > + > inc_nlink(inode); > ihold(inode); > inode->i_ctime = current_time(inode); > ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/4] ubifs: Don't add fscrypt context to xattrs 2021-01-22 21:22 ` Richard Weinberger @ 2021-01-22 21:22 ` Richard Weinberger -1 siblings, 0 replies; 22+ messages in thread From: Richard Weinberger @ 2021-01-22 21:22 UTC (permalink / raw) To: linux-mtd; +Cc: richard, david, linux-kernel, stable In UBIFS xattrs are inodes too, make sure that these inodes don't get a fscrypt context. Otherwise we will end up with undeletable xattrs which will waste memory on the flash. Cc: stable@vger.kernel.org Fixes: d475a507457b ("ubifs: Add skeleton for fscrypto") Signed-off-by: Richard Weinberger <richard@nod.at> --- fs/ubifs/dir.c | 23 +++++++++++++---------- fs/ubifs/ubifs.h | 2 +- fs/ubifs/xattr.c | 2 +- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c index 04912dedca48..8a34e0118ee9 100644 --- a/fs/ubifs/dir.c +++ b/fs/ubifs/dir.c @@ -68,13 +68,14 @@ static int inherit_flags(const struct inode *dir, umode_t mode) * @c: UBIFS file-system description object * @dir: parent directory inode * @mode: inode mode flags + * @omit_fscrypt_ctx: Don't create a fscrypt context for this inode * * This function finds an unused inode number, allocates new inode and * initializes it. Returns new inode in case of success and an error code in * case of failure. */ struct inode *ubifs_new_inode(struct ubifs_info *c, struct inode *dir, - umode_t mode) + umode_t mode, int omit_fscrypt_ctx) { int err; struct inode *inode; @@ -99,10 +100,12 @@ struct inode *ubifs_new_inode(struct ubifs_info *c, struct inode *dir, current_time(inode); inode->i_mapping->nrpages = 0; - err = fscrypt_prepare_new_inode(dir, inode, &encrypted); - if (err) { - ubifs_err(c, "fscrypt_prepare_new_inode failed: %i", err); - goto out_iput; + if (!omit_fscrypt_ctx) { + err = fscrypt_prepare_new_inode(dir, inode, &encrypted); + if (err) { + ubifs_err(c, "fscrypt_prepare_new_inode failed: %i", err); + goto out_iput; + } } switch (mode & S_IFMT) { @@ -309,7 +312,7 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode, sz_change = CALC_DENT_SIZE(fname_len(&nm)); - inode = ubifs_new_inode(c, dir, mode); + inode = ubifs_new_inode(c, dir, mode, 0); if (IS_ERR(inode)) { err = PTR_ERR(inode); goto out_fname; @@ -385,7 +388,7 @@ static int do_tmpfile(struct inode *dir, struct dentry *dentry, return err; } - inode = ubifs_new_inode(c, dir, mode); + inode = ubifs_new_inode(c, dir, mode, 0); if (IS_ERR(inode)) { err = PTR_ERR(inode); goto out_budg; @@ -971,7 +974,7 @@ static int ubifs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode) sz_change = CALC_DENT_SIZE(fname_len(&nm)); - inode = ubifs_new_inode(c, dir, S_IFDIR | mode); + inode = ubifs_new_inode(c, dir, S_IFDIR | mode, 0); if (IS_ERR(inode)) { err = PTR_ERR(inode); goto out_fname; @@ -1058,7 +1061,7 @@ static int ubifs_mknod(struct inode *dir, struct dentry *dentry, sz_change = CALC_DENT_SIZE(fname_len(&nm)); - inode = ubifs_new_inode(c, dir, mode); + inode = ubifs_new_inode(c, dir, mode, 0); if (IS_ERR(inode)) { kfree(dev); err = PTR_ERR(inode); @@ -1140,7 +1143,7 @@ static int ubifs_symlink(struct inode *dir, struct dentry *dentry, sz_change = CALC_DENT_SIZE(fname_len(&nm)); - inode = ubifs_new_inode(c, dir, S_IFLNK | S_IRWXUGO); + inode = ubifs_new_inode(c, dir, S_IFLNK | S_IRWXUGO, 0); if (IS_ERR(inode)) { err = PTR_ERR(inode); goto out_fname; diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index fc2cdde3b549..dc9b6ba41e77 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -1994,7 +1994,7 @@ int ubifs_update_time(struct inode *inode, struct timespec64 *time, int flags); /* dir.c */ struct inode *ubifs_new_inode(struct ubifs_info *c, struct inode *dir, - umode_t mode); + umode_t mode, int omit_fscrypt_ctx); int ubifs_getattr(const struct path *path, struct kstat *stat, u32 request_mask, unsigned int flags); int ubifs_check_dir_empty(struct inode *dir); diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c index a0b9b349efe6..430abe9e56af 100644 --- a/fs/ubifs/xattr.c +++ b/fs/ubifs/xattr.c @@ -110,7 +110,7 @@ static int create_xattr(struct ubifs_info *c, struct inode *host, if (err) return err; - inode = ubifs_new_inode(c, host, S_IFREG | S_IRWXUGO); + inode = ubifs_new_inode(c, host, S_IFREG | S_IRWXUGO, 1); if (IS_ERR(inode)) { err = PTR_ERR(inode); goto out_budg; -- 2.26.2 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/4] ubifs: Don't add fscrypt context to xattrs @ 2021-01-22 21:22 ` Richard Weinberger 0 siblings, 0 replies; 22+ messages in thread From: Richard Weinberger @ 2021-01-22 21:22 UTC (permalink / raw) To: linux-mtd; +Cc: david, richard, linux-kernel, stable In UBIFS xattrs are inodes too, make sure that these inodes don't get a fscrypt context. Otherwise we will end up with undeletable xattrs which will waste memory on the flash. Cc: stable@vger.kernel.org Fixes: d475a507457b ("ubifs: Add skeleton for fscrypto") Signed-off-by: Richard Weinberger <richard@nod.at> --- fs/ubifs/dir.c | 23 +++++++++++++---------- fs/ubifs/ubifs.h | 2 +- fs/ubifs/xattr.c | 2 +- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c index 04912dedca48..8a34e0118ee9 100644 --- a/fs/ubifs/dir.c +++ b/fs/ubifs/dir.c @@ -68,13 +68,14 @@ static int inherit_flags(const struct inode *dir, umode_t mode) * @c: UBIFS file-system description object * @dir: parent directory inode * @mode: inode mode flags + * @omit_fscrypt_ctx: Don't create a fscrypt context for this inode * * This function finds an unused inode number, allocates new inode and * initializes it. Returns new inode in case of success and an error code in * case of failure. */ struct inode *ubifs_new_inode(struct ubifs_info *c, struct inode *dir, - umode_t mode) + umode_t mode, int omit_fscrypt_ctx) { int err; struct inode *inode; @@ -99,10 +100,12 @@ struct inode *ubifs_new_inode(struct ubifs_info *c, struct inode *dir, current_time(inode); inode->i_mapping->nrpages = 0; - err = fscrypt_prepare_new_inode(dir, inode, &encrypted); - if (err) { - ubifs_err(c, "fscrypt_prepare_new_inode failed: %i", err); - goto out_iput; + if (!omit_fscrypt_ctx) { + err = fscrypt_prepare_new_inode(dir, inode, &encrypted); + if (err) { + ubifs_err(c, "fscrypt_prepare_new_inode failed: %i", err); + goto out_iput; + } } switch (mode & S_IFMT) { @@ -309,7 +312,7 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode, sz_change = CALC_DENT_SIZE(fname_len(&nm)); - inode = ubifs_new_inode(c, dir, mode); + inode = ubifs_new_inode(c, dir, mode, 0); if (IS_ERR(inode)) { err = PTR_ERR(inode); goto out_fname; @@ -385,7 +388,7 @@ static int do_tmpfile(struct inode *dir, struct dentry *dentry, return err; } - inode = ubifs_new_inode(c, dir, mode); + inode = ubifs_new_inode(c, dir, mode, 0); if (IS_ERR(inode)) { err = PTR_ERR(inode); goto out_budg; @@ -971,7 +974,7 @@ static int ubifs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode) sz_change = CALC_DENT_SIZE(fname_len(&nm)); - inode = ubifs_new_inode(c, dir, S_IFDIR | mode); + inode = ubifs_new_inode(c, dir, S_IFDIR | mode, 0); if (IS_ERR(inode)) { err = PTR_ERR(inode); goto out_fname; @@ -1058,7 +1061,7 @@ static int ubifs_mknod(struct inode *dir, struct dentry *dentry, sz_change = CALC_DENT_SIZE(fname_len(&nm)); - inode = ubifs_new_inode(c, dir, mode); + inode = ubifs_new_inode(c, dir, mode, 0); if (IS_ERR(inode)) { kfree(dev); err = PTR_ERR(inode); @@ -1140,7 +1143,7 @@ static int ubifs_symlink(struct inode *dir, struct dentry *dentry, sz_change = CALC_DENT_SIZE(fname_len(&nm)); - inode = ubifs_new_inode(c, dir, S_IFLNK | S_IRWXUGO); + inode = ubifs_new_inode(c, dir, S_IFLNK | S_IRWXUGO, 0); if (IS_ERR(inode)) { err = PTR_ERR(inode); goto out_fname; diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index fc2cdde3b549..dc9b6ba41e77 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -1994,7 +1994,7 @@ int ubifs_update_time(struct inode *inode, struct timespec64 *time, int flags); /* dir.c */ struct inode *ubifs_new_inode(struct ubifs_info *c, struct inode *dir, - umode_t mode); + umode_t mode, int omit_fscrypt_ctx); int ubifs_getattr(const struct path *path, struct kstat *stat, u32 request_mask, unsigned int flags); int ubifs_check_dir_empty(struct inode *dir); diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c index a0b9b349efe6..430abe9e56af 100644 --- a/fs/ubifs/xattr.c +++ b/fs/ubifs/xattr.c @@ -110,7 +110,7 @@ static int create_xattr(struct ubifs_info *c, struct inode *host, if (err) return err; - inode = ubifs_new_inode(c, host, S_IFREG | S_IRWXUGO); + inode = ubifs_new_inode(c, host, S_IFREG | S_IRWXUGO, 1); if (IS_ERR(inode)) { err = PTR_ERR(inode); goto out_budg; -- 2.26.2 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/4] ubifs: Update directory size when creating whiteouts 2021-01-22 21:22 ` Richard Weinberger @ 2021-01-22 21:22 ` Richard Weinberger -1 siblings, 0 replies; 22+ messages in thread From: Richard Weinberger @ 2021-01-22 21:22 UTC (permalink / raw) To: linux-mtd; +Cc: richard, david, linux-kernel, stable Although whiteouts are unlinked files they will get re-linked later, therefore the size of the parent directory needs to be updated too. Cc: stable@vger.kernel.org Fixes: 9e0a1fff8db5 ("ubifs: Implement RENAME_WHITEOUT") Signed-off-by: Richard Weinberger <richard@nod.at> --- fs/ubifs/dir.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c index 8a34e0118ee9..b5d523e5854f 100644 --- a/fs/ubifs/dir.c +++ b/fs/ubifs/dir.c @@ -361,6 +361,7 @@ static int do_tmpfile(struct inode *dir, struct dentry *dentry, struct ubifs_budget_req ino_req = { .dirtied_ino = 1 }; struct ubifs_inode *ui, *dir_ui = ubifs_inode(dir); int err, instantiated = 0; + int sz_change = 0; struct fscrypt_name nm; /* @@ -411,6 +412,8 @@ static int do_tmpfile(struct inode *dir, struct dentry *dentry, mark_inode_dirty(inode); drop_nlink(inode); *whiteout = inode; + sz_change = CALC_DENT_SIZE(fname_len(&nm)); + dir->i_size += sz_change; } else { d_tmpfile(dentry, inode); } @@ -430,6 +433,7 @@ static int do_tmpfile(struct inode *dir, struct dentry *dentry, return 0; out_cancel: + dir->i_size -= sz_change; mutex_unlock(&dir_ui->ui_mutex); out_inode: make_bad_inode(inode); -- 2.26.2 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/4] ubifs: Update directory size when creating whiteouts @ 2021-01-22 21:22 ` Richard Weinberger 0 siblings, 0 replies; 22+ messages in thread From: Richard Weinberger @ 2021-01-22 21:22 UTC (permalink / raw) To: linux-mtd; +Cc: david, richard, linux-kernel, stable Although whiteouts are unlinked files they will get re-linked later, therefore the size of the parent directory needs to be updated too. Cc: stable@vger.kernel.org Fixes: 9e0a1fff8db5 ("ubifs: Implement RENAME_WHITEOUT") Signed-off-by: Richard Weinberger <richard@nod.at> --- fs/ubifs/dir.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c index 8a34e0118ee9..b5d523e5854f 100644 --- a/fs/ubifs/dir.c +++ b/fs/ubifs/dir.c @@ -361,6 +361,7 @@ static int do_tmpfile(struct inode *dir, struct dentry *dentry, struct ubifs_budget_req ino_req = { .dirtied_ino = 1 }; struct ubifs_inode *ui, *dir_ui = ubifs_inode(dir); int err, instantiated = 0; + int sz_change = 0; struct fscrypt_name nm; /* @@ -411,6 +412,8 @@ static int do_tmpfile(struct inode *dir, struct dentry *dentry, mark_inode_dirty(inode); drop_nlink(inode); *whiteout = inode; + sz_change = CALC_DENT_SIZE(fname_len(&nm)); + dir->i_size += sz_change; } else { d_tmpfile(dentry, inode); } @@ -430,6 +433,7 @@ static int do_tmpfile(struct inode *dir, struct dentry *dentry, return 0; out_cancel: + dir->i_size -= sz_change; mutex_unlock(&dir_ui->ui_mutex); out_inode: make_bad_inode(inode); -- 2.26.2 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] ubifs: Update directory size when creating whiteouts 2021-01-22 21:22 ` Richard Weinberger @ 2021-01-23 2:45 ` Zhihao Cheng -1 siblings, 0 replies; 22+ messages in thread From: Zhihao Cheng @ 2021-01-23 2:45 UTC (permalink / raw) To: Richard Weinberger, linux-mtd; +Cc: david, linux-kernel, stable 在 2021/1/23 5:22, Richard Weinberger 写道: > Although whiteouts are unlinked files they will get re-linked later, I just want to make sure, is this where the count is increased? do_rename -> inc_nlink(whiteout) > therefore the size of the parent directory needs to be updated too. > > Cc: stable@vger.kernel.org > Fixes: 9e0a1fff8db5 ("ubifs: Implement RENAME_WHITEOUT") > Signed-off-by: Richard Weinberger <richard@nod.at> > --- > fs/ubifs/dir.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c > index 8a34e0118ee9..b5d523e5854f 100644 > --- a/fs/ubifs/dir.c > +++ b/fs/ubifs/dir.c > @@ -361,6 +361,7 @@ static int do_tmpfile(struct inode *dir, struct dentry *dentry, > struct ubifs_budget_req ino_req = { .dirtied_ino = 1 }; > struct ubifs_inode *ui, *dir_ui = ubifs_inode(dir); > int err, instantiated = 0; > + int sz_change = 0; > struct fscrypt_name nm; > > /* > @@ -411,6 +412,8 @@ static int do_tmpfile(struct inode *dir, struct dentry *dentry, > mark_inode_dirty(inode); > drop_nlink(inode); > *whiteout = inode; > + sz_change = CALC_DENT_SIZE(fname_len(&nm)); > + dir->i_size += sz_change; > } else { > d_tmpfile(dentry, inode); > } > @@ -430,6 +433,7 @@ static int do_tmpfile(struct inode *dir, struct dentry *dentry, > return 0; > > out_cancel: Does this need a judgment? Like this, if (whiteout) dir->i_size -= sz_change; > + dir->i_size -= sz_change; > mutex_unlock(&dir_ui->ui_mutex); > out_inode: > make_bad_inode(inode); > ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] ubifs: Update directory size when creating whiteouts @ 2021-01-23 2:45 ` Zhihao Cheng 0 siblings, 0 replies; 22+ messages in thread From: Zhihao Cheng @ 2021-01-23 2:45 UTC (permalink / raw) To: Richard Weinberger, linux-mtd; +Cc: david, linux-kernel, stable 在 2021/1/23 5:22, Richard Weinberger 写道: > Although whiteouts are unlinked files they will get re-linked later, I just want to make sure, is this where the count is increased? do_rename -> inc_nlink(whiteout) > therefore the size of the parent directory needs to be updated too. > > Cc: stable@vger.kernel.org > Fixes: 9e0a1fff8db5 ("ubifs: Implement RENAME_WHITEOUT") > Signed-off-by: Richard Weinberger <richard@nod.at> > --- > fs/ubifs/dir.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c > index 8a34e0118ee9..b5d523e5854f 100644 > --- a/fs/ubifs/dir.c > +++ b/fs/ubifs/dir.c > @@ -361,6 +361,7 @@ static int do_tmpfile(struct inode *dir, struct dentry *dentry, > struct ubifs_budget_req ino_req = { .dirtied_ino = 1 }; > struct ubifs_inode *ui, *dir_ui = ubifs_inode(dir); > int err, instantiated = 0; > + int sz_change = 0; > struct fscrypt_name nm; > > /* > @@ -411,6 +412,8 @@ static int do_tmpfile(struct inode *dir, struct dentry *dentry, > mark_inode_dirty(inode); > drop_nlink(inode); > *whiteout = inode; > + sz_change = CALC_DENT_SIZE(fname_len(&nm)); > + dir->i_size += sz_change; > } else { > d_tmpfile(dentry, inode); > } > @@ -430,6 +433,7 @@ static int do_tmpfile(struct inode *dir, struct dentry *dentry, > return 0; > > out_cancel: Does this need a judgment? Like this, if (whiteout) dir->i_size -= sz_change; > + dir->i_size -= sz_change; > mutex_unlock(&dir_ui->ui_mutex); > out_inode: > make_bad_inode(inode); > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] ubifs: Update directory size when creating whiteouts 2021-01-23 2:45 ` Zhihao Cheng @ 2021-01-23 10:05 ` Richard Weinberger -1 siblings, 0 replies; 22+ messages in thread From: Richard Weinberger @ 2021-01-23 10:05 UTC (permalink / raw) To: chengzhihao1; +Cc: david, linux-mtd, linux-kernel, stable ----- Ursprüngliche Mail ----- > Von: "chengzhihao1" <chengzhihao1@huawei.com> > An: "richard" <richard@nod.at>, "linux-mtd" <linux-mtd@lists.infradead.org> > CC: "david" <david@sigma-star.at>, "linux-kernel" <linux-kernel@vger.kernel.org>, "stable" <stable@vger.kernel.org> > Gesendet: Samstag, 23. Januar 2021 03:45:15 > Betreff: Re: [PATCH 3/4] ubifs: Update directory size when creating whiteouts > 在 2021/1/23 5:22, Richard Weinberger 写道: >> Although whiteouts are unlinked files they will get re-linked later, > I just want to make sure, is this where the count is increased? > do_rename -> inc_nlink(whiteout) Exactly. The logic is a little wicked, I agree. Let me think again whether there isn't a better way to address the problem. Thanks, //richard P.s: Thanks a lot for reviewing! ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] ubifs: Update directory size when creating whiteouts @ 2021-01-23 10:05 ` Richard Weinberger 0 siblings, 0 replies; 22+ messages in thread From: Richard Weinberger @ 2021-01-23 10:05 UTC (permalink / raw) To: chengzhihao1; +Cc: linux-mtd, david, linux-kernel, stable ----- Ursprüngliche Mail ----- > Von: "chengzhihao1" <chengzhihao1@huawei.com> > An: "richard" <richard@nod.at>, "linux-mtd" <linux-mtd@lists.infradead.org> > CC: "david" <david@sigma-star.at>, "linux-kernel" <linux-kernel@vger.kernel.org>, "stable" <stable@vger.kernel.org> > Gesendet: Samstag, 23. Januar 2021 03:45:15 > Betreff: Re: [PATCH 3/4] ubifs: Update directory size when creating whiteouts > 在 2021/1/23 5:22, Richard Weinberger 写道: >> Although whiteouts are unlinked files they will get re-linked later, > I just want to make sure, is this where the count is increased? > do_rename -> inc_nlink(whiteout) Exactly. The logic is a little wicked, I agree. Let me think again whether there isn't a better way to address the problem. Thanks, //richard P.s: Thanks a lot for reviewing! ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] ubifs: Update directory size when creating whiteouts 2021-01-23 2:45 ` Zhihao Cheng @ 2021-01-25 1:05 ` Zhihao Cheng -1 siblings, 0 replies; 22+ messages in thread From: Zhihao Cheng @ 2021-01-25 1:05 UTC (permalink / raw) To: Richard Weinberger, linux-mtd; +Cc: david, linux-kernel, stable 在 2021/1/23 10:45, Zhihao Cheng 写道: >> @@ -430,6 +433,7 @@ static int do_tmpfile(struct inode *dir, struct >> dentry *dentry, >> return 0; >> out_cancel: Still one question: > Does this need a judgment? Like this, > if (whiteout) > dir->i_size -= sz_change; >> + dir->i_size -= sz_change; >> mutex_unlock(&dir_ui->ui_mutex); >> out_inode: >> make_bad_inode(inode); >> > > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/ ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] ubifs: Update directory size when creating whiteouts @ 2021-01-25 1:05 ` Zhihao Cheng 0 siblings, 0 replies; 22+ messages in thread From: Zhihao Cheng @ 2021-01-25 1:05 UTC (permalink / raw) To: Richard Weinberger, linux-mtd; +Cc: david, linux-kernel, stable 在 2021/1/23 10:45, Zhihao Cheng 写道: >> @@ -430,6 +433,7 @@ static int do_tmpfile(struct inode *dir, struct >> dentry *dentry, >> return 0; >> out_cancel: Still one question: > Does this need a judgment? Like this, > if (whiteout) > dir->i_size -= sz_change; >> + dir->i_size -= sz_change; >> mutex_unlock(&dir_ui->ui_mutex); >> out_inode: >> make_bad_inode(inode); >> > > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] ubifs: Update directory size when creating whiteouts 2021-01-25 1:05 ` Zhihao Cheng @ 2021-01-25 7:55 ` Richard Weinberger -1 siblings, 0 replies; 22+ messages in thread From: Richard Weinberger @ 2021-01-25 7:55 UTC (permalink / raw) To: Zhihao Cheng; +Cc: Richard Weinberger, David Gstir, linux-mtd, LKML, stable On Mon, Jan 25, 2021 at 2:12 AM Zhihao Cheng <chengzhihao1@huawei.com> wrote: > > 在 2021/1/23 10:45, Zhihao Cheng 写道: > > >> @@ -430,6 +433,7 @@ static int do_tmpfile(struct inode *dir, struct > >> dentry *dentry, > >> return 0; > >> out_cancel: > Still one question: > > Does this need a judgment? Like this, The idea was that in the !whiteout case, sz_change is always 0. -- Thanks, //richard ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] ubifs: Update directory size when creating whiteouts @ 2021-01-25 7:55 ` Richard Weinberger 0 siblings, 0 replies; 22+ messages in thread From: Richard Weinberger @ 2021-01-25 7:55 UTC (permalink / raw) To: Zhihao Cheng; +Cc: Richard Weinberger, linux-mtd, David Gstir, LKML, stable On Mon, Jan 25, 2021 at 2:12 AM Zhihao Cheng <chengzhihao1@huawei.com> wrote: > > 在 2021/1/23 10:45, Zhihao Cheng 写道: > > >> @@ -430,6 +433,7 @@ static int do_tmpfile(struct inode *dir, struct > >> dentry *dentry, > >> return 0; > >> out_cancel: > Still one question: > > Does this need a judgment? Like this, The idea was that in the !whiteout case, sz_change is always 0. -- Thanks, //richard ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] ubifs: Update directory size when creating whiteouts 2021-01-25 7:55 ` Richard Weinberger @ 2021-01-25 9:31 ` Zhihao Cheng -1 siblings, 0 replies; 22+ messages in thread From: Zhihao Cheng @ 2021-01-25 9:31 UTC (permalink / raw) To: Richard Weinberger Cc: Richard Weinberger, David Gstir, linux-mtd, LKML, stable 在 2021/1/25 15:55, Richard Weinberger 写道: > > The idea was that in the !whiteout case, sz_change is always 0. > Oh, sz_change was initialized to 0, I missed it. Thanks. ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] ubifs: Update directory size when creating whiteouts @ 2021-01-25 9:31 ` Zhihao Cheng 0 siblings, 0 replies; 22+ messages in thread From: Zhihao Cheng @ 2021-01-25 9:31 UTC (permalink / raw) To: Richard Weinberger Cc: Richard Weinberger, linux-mtd, David Gstir, LKML, stable 在 2021/1/25 15:55, Richard Weinberger 写道: > > The idea was that in the !whiteout case, sz_change is always 0. > Oh, sz_change was initialized to 0, I missed it. Thanks. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/4] ubifs: Harden ubifs_jnl_write_inode() 2021-01-22 21:22 ` Richard Weinberger @ 2021-01-22 21:22 ` Richard Weinberger -1 siblings, 0 replies; 22+ messages in thread From: Richard Weinberger @ 2021-01-22 21:22 UTC (permalink / raw) To: linux-mtd; +Cc: richard, david, linux-kernel Make sure that ubifs_jnl_write_inode() cannot be abused and will not write less data then expected. Signed-off-by: Richard Weinberger <richard@nod.at> --- fs/ubifs/journal.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c index 03410ae0813a..1b28fcc5b9fe 100644 --- a/fs/ubifs/journal.c +++ b/fs/ubifs/journal.c @@ -844,10 +844,12 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) struct ubifs_ino_node *ino, *ino_start; struct ubifs_inode *ui = ubifs_inode(inode); int sync = 0, write_len = 0, ilen = UBIFS_INO_NODE_SZ; - int last_reference = !inode->i_nlink; + int last_reference = !inode->i_nlink, xattr_inos_written = 0; int kill_xattrs = ui->xattr_cnt && last_reference; u8 hash[UBIFS_HASH_ARR_SZ]; + ubifs_assert(c, !ui->xattr); + dbg_jnl("ino %lu, nlink %u", inode->i_ino, inode->i_nlink); /* @@ -917,12 +919,14 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) pack_inode(c, ino, xino, 0); ino = (void *)ino + UBIFS_INO_NODE_SZ; iput(xino); + xattr_inos_written++; kfree(pxent); pxent = xent; key_read(c, &xent->key, &key); } kfree(pxent); + ubifs_assert(c, xattr_inos_written == ui->xattr_cnt); } pack_inode(c, ino, inode, 1); -- 2.26.2 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 4/4] ubifs: Harden ubifs_jnl_write_inode() @ 2021-01-22 21:22 ` Richard Weinberger 0 siblings, 0 replies; 22+ messages in thread From: Richard Weinberger @ 2021-01-22 21:22 UTC (permalink / raw) To: linux-mtd; +Cc: david, richard, linux-kernel Make sure that ubifs_jnl_write_inode() cannot be abused and will not write less data then expected. Signed-off-by: Richard Weinberger <richard@nod.at> --- fs/ubifs/journal.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c index 03410ae0813a..1b28fcc5b9fe 100644 --- a/fs/ubifs/journal.c +++ b/fs/ubifs/journal.c @@ -844,10 +844,12 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) struct ubifs_ino_node *ino, *ino_start; struct ubifs_inode *ui = ubifs_inode(inode); int sync = 0, write_len = 0, ilen = UBIFS_INO_NODE_SZ; - int last_reference = !inode->i_nlink; + int last_reference = !inode->i_nlink, xattr_inos_written = 0; int kill_xattrs = ui->xattr_cnt && last_reference; u8 hash[UBIFS_HASH_ARR_SZ]; + ubifs_assert(c, !ui->xattr); + dbg_jnl("ino %lu, nlink %u", inode->i_ino, inode->i_nlink); /* @@ -917,12 +919,14 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) pack_inode(c, ino, xino, 0); ino = (void *)ino + UBIFS_INO_NODE_SZ; iput(xino); + xattr_inos_written++; kfree(pxent); pxent = xent; key_read(c, &xent->key, &key); } kfree(pxent); + ubifs_assert(c, xattr_inos_written == ui->xattr_cnt); } pack_inode(c, ino, inode, 1); -- 2.26.2 ^ permalink raw reply related [flat|nested] 22+ messages in thread
end of thread, other threads:[~2021-01-26 19:47 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-01-22 21:22 [PATCH 0/4] Assorted UBIFS fixes Richard Weinberger 2021-01-22 21:22 ` Richard Weinberger 2021-01-22 21:22 ` [PATCH 1/4] ubifs: Correctly set inode size in ubifs_link() Richard Weinberger 2021-01-22 21:22 ` Richard Weinberger 2021-01-23 2:35 ` Zhihao Cheng 2021-01-23 2:35 ` Zhihao Cheng 2021-01-22 21:22 ` [PATCH 2/4] ubifs: Don't add fscrypt context to xattrs Richard Weinberger 2021-01-22 21:22 ` Richard Weinberger 2021-01-22 21:22 ` [PATCH 3/4] ubifs: Update directory size when creating whiteouts Richard Weinberger 2021-01-22 21:22 ` Richard Weinberger 2021-01-23 2:45 ` Zhihao Cheng 2021-01-23 2:45 ` Zhihao Cheng 2021-01-23 10:05 ` Richard Weinberger 2021-01-23 10:05 ` Richard Weinberger 2021-01-25 1:05 ` Zhihao Cheng 2021-01-25 1:05 ` Zhihao Cheng 2021-01-25 7:55 ` Richard Weinberger 2021-01-25 7:55 ` Richard Weinberger 2021-01-25 9:31 ` Zhihao Cheng 2021-01-25 9:31 ` Zhihao Cheng 2021-01-22 21:22 ` [PATCH 4/4] ubifs: Harden ubifs_jnl_write_inode() Richard Weinberger 2021-01-22 21:22 ` Richard Weinberger
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.