[PATCH] ubifs: account for short copies in ubifs_write_end

[PATCH] ubifs: account for short copies in ubifs_write_end

[Hungyu Lin]

ubifs_write_end() updates i_size and ui_size to pos + len even when
fewer bytes were copied. A one-byte write at EOF from an unreadable
buffer can fail with EFAULT while extending the file by one byte.
Partial writes can also extend the file past the last byte copied.

Use pos + copied for size updates and cancel the budget when no data
was copied. Keep the appending decision based on the requested length
to match the inode budgeting and ui_mutex locking in ubifs_write_begin().
If a short copy ends within the old size, release any unused inode budget
without reducing the size, and still unlock ui_mutex.

Fixes: 1e51764a3c2a ("UBIFS: add new flash file system")
Link: https://lore.kernel.org/linux-mtd/20261027061226.3506517-1-lin.nan@h3c.com/
Cc: stable@vger.kernel.org
Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
---
 fs/ubifs/file.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 99dd52715ee..8ebd3f4cddf 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -523,8 +523,8 @@ static int ubifs_write_end(const struct kiocb *iocb,
 	struct inode *inode = mapping->host;
 	struct ubifs_inode *ui = ubifs_inode(inode);
 	struct ubifs_info *c = inode->i_sb->s_fs_info;
-	loff_t end_pos = pos + len;
-	int appending = !!(end_pos > inode->i_size);
+	loff_t end_pos = pos + copied;
+	int appending = !!(pos + len > inode->i_size);
 
 	dbg_gen("ino %llu, pos %llu, pg %lu, len %u, copied %d, i_size %lld",
 		inode->i_ino, pos, folio->index, len, copied, inode->i_size);
@@ -552,6 +552,11 @@ static int ubifs_write_end(const struct kiocb *iocb,
 		goto out;
 	}
 
+	if (unlikely(!copied)) {
+		cancel_budget(c, folio, ui, appending);
+		goto out;
+	}
+
 	if (len == folio_size(folio))
 		folio_mark_uptodate(folio);
 
@@ -562,14 +567,18 @@ static int ubifs_write_end(const struct kiocb *iocb,
 	}
 
 	if (appending) {
-		i_size_write(inode, end_pos);
-		ui->ui_size = end_pos;
-		/*
-		 * We do not set @I_DIRTY_PAGES (which means that
-		 * the inode has dirty pages), this was done in
-		 * filemap_dirty_folio().
-		 */
-		__mark_inode_dirty(inode, I_DIRTY_DATASYNC);
+		if (end_pos > inode->i_size) {
+			i_size_write(inode, end_pos);
+			ui->ui_size = end_pos;
+			/*
+			 * We do not set @I_DIRTY_PAGES (which means that
+			 * the inode has dirty pages), this was done in
+			 * filemap_dirty_folio().
+			 */
+			__mark_inode_dirty(inode, I_DIRTY_DATASYNC);
+		} else if (!ui->dirty) {
+			ubifs_release_dirty_inode_budget(c, ui);
+		}
 		ubifs_assert(c, mutex_is_locked(&ui->ui_mutex));
 		mutex_unlock(&ui->ui_mutex);
 	}

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] ubifs: account for short copies in ubifs_write_end

[Zhihao Cheng]

在 2026/11/5 3:11, Hungyu Lin 写道:
> ubifs_write_end() updates i_size and ui_size to pos + len even when
> fewer bytes were copied. A one-byte write at EOF from an unreadable
> buffer can fail with EFAULT while extending the file by one byte.
> Partial writes can also extend the file past the last byte copied.
> 
> Use pos + copied for size updates and cancel the budget when no data
> was copied. Keep the appending decision based on the requested length
> to match the inode budgeting and ui_mutex locking in ubifs_write_begin().
> If a short copy ends within the old size, release any unused inode budget
> without reducing the size, and still unlock ui_mutex.
> 
> Fixes: 1e51764a3c2a ("UBIFS: add new flash file system")
> Link: https://lore.kernel.org/linux-mtd/20261027061226.3506517-1-lin.nan@h3c.com/
> Cc: stable@vger.kernel.org
> Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
> ---
>   fs/ubifs/file.c | 29 +++++++++++++++++++----------
>   1 file changed, 19 insertions(+), 10 deletions(-)
Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>
> 
> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index 99dd52715ee..8ebd3f4cddf 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -523,8 +523,8 @@ static int ubifs_write_end(const struct kiocb *iocb,
>   	struct inode *inode = mapping->host;
>   	struct ubifs_inode *ui = ubifs_inode(inode);
>   	struct ubifs_info *c = inode->i_sb->s_fs_info;
> -	loff_t end_pos = pos + len;
> -	int appending = !!(end_pos > inode->i_size);
> +	loff_t end_pos = pos + copied;
> +	int appending = !!(pos + len > inode->i_size);
>   
>   	dbg_gen("ino %llu, pos %llu, pg %lu, len %u, copied %d, i_size %lld",
>   		inode->i_ino, pos, folio->index, len, copied, inode->i_size);
> @@ -552,6 +552,11 @@ static int ubifs_write_end(const struct kiocb *iocb,
>   		goto out;
>   	}
>   
> +	if (unlikely(!copied)) {
> +		cancel_budget(c, folio, ui, appending);
> +		goto out;
> +	}
> +
>   	if (len == folio_size(folio))
>   		folio_mark_uptodate(folio);
>   
> @@ -562,14 +567,18 @@ static int ubifs_write_end(const struct kiocb *iocb,
>   	}
>   
>   	if (appending) {
> -		i_size_write(inode, end_pos);
> -		ui->ui_size = end_pos;
> -		/*
> -		 * We do not set @I_DIRTY_PAGES (which means that
> -		 * the inode has dirty pages), this was done in
> -		 * filemap_dirty_folio().
> -		 */
> -		__mark_inode_dirty(inode, I_DIRTY_DATASYNC);
> +		if (end_pos > inode->i_size) {
> +			i_size_write(inode, end_pos);
> +			ui->ui_size = end_pos;
> +			/*
> +			 * We do not set @I_DIRTY_PAGES (which means that
> +			 * the inode has dirty pages), this was done in
> +			 * filemap_dirty_folio().
> +			 */
> +			__mark_inode_dirty(inode, I_DIRTY_DATASYNC);
> +		} else if (!ui->dirty) {
> +			ubifs_release_dirty_inode_budget(c, ui);
> +		}
>   		ubifs_assert(c, mutex_is_locked(&ui->ui_mutex));
>   		mutex_unlock(&ui->ui_mutex);
>   	}
> 
> base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
>