1. Patchew
  2. linux
  3. View series

[PATCH v2] ntfs: fix missing kstrdup() error check in ntfs_write_volume_label()

Zhan Xusheng posted 1 patch 3 weeks, 1 day ago 
fs/ntfs/super.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
[PATCH v2] ntfs: fix missing kstrdup() error check in ntfs_write_volume_label()
Posted by Zhan Xusheng 3 weeks, 1 day ago 
ntfs_write_volume_label() does not check the return value of
kstrdup().  If the allocation fails, vol->volume_label is set to
NULL while the function returns success.  A subsequent
FS_IOC_GETFSLABEL then returns an empty string even though the
on-disk label was updated correctly.

Fix by allocating the new label before taking vol_ni->mrec_lock and
updating any on-disk metadata, so an -ENOMEM from kstrdup() leaves
both the in-memory and on-disk labels untouched and consistent.  On
success the preallocated copy replaces the old vol->volume_label.
Also move mark_inode_dirty_sync() into the success path so that it
is not called when no metadata was actually modified.

Fixes: 6251f0b0de7d ("ntfs: update super block operations")
Suggested-by: Hyunchul Lee <hyc.lee@gmail.com>
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 fs/ntfs/super.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
index 22dc7865eca7..ba99c497ee38 100644
--- a/fs/ntfs/super.c
+++ b/fs/ntfs/super.c
@@ -413,6 +413,7 @@ int ntfs_write_volume_label(struct ntfs_volume *vol, char *label)
 {
 	struct ntfs_inode *vol_ni = NTFS_I(vol->vol_ino);
 	struct ntfs_attr_search_ctx *ctx;
+	char *new_label;
 	__le16 *uname;
 	int uname_len, ret;
 
@@ -425,7 +426,7 @@ int ntfs_write_volume_label(struct ntfs_volume *vol, char *label)
 		return uname_len;
 	}
 
-	if (uname_len  > NTFS_MAX_LABEL_LEN) {
+	if (uname_len > NTFS_MAX_LABEL_LEN) {
 		ntfs_error(vol->sb,
 			   "Volume label is too long (max %d characters).",
 			   NTFS_MAX_LABEL_LEN);
@@ -433,11 +434,22 @@ int ntfs_write_volume_label(struct ntfs_volume *vol, char *label)
 		return -EINVAL;
 	}
 
+	/*
+	 * Allocate the in-memory label copy up front. If kstrdup() fails we
+	 * bail out before touching on-disk metadata, so the in-memory label
+	 * and the on-disk label stay in sync.
+	 */
+	new_label = kstrdup(label, GFP_KERNEL);
+	if (!new_label) {
+		kvfree(uname);
+		return -ENOMEM;
+	}
+
 	mutex_lock(&vol_ni->mrec_lock);
 	ctx = ntfs_attr_get_search_ctx(vol_ni, NULL);
 	if (!ctx) {
 		ret = -ENOMEM;
-		goto  out;
+		goto out;
 	}
 
 	if (!ntfs_attr_lookup(AT_VOLUME_NAME, NULL, 0, 0, 0, NULL, 0,
@@ -450,12 +462,14 @@ int ntfs_write_volume_label(struct ntfs_volume *vol, char *label)
 out:
 	mutex_unlock(&vol_ni->mrec_lock);
 	kvfree(uname);
-	mark_inode_dirty_sync(vol->vol_ino);
 
 	if (ret >= 0) {
 		kfree(vol->volume_label);
-		vol->volume_label = kstrdup(label, GFP_KERNEL);
+		vol->volume_label = new_label;
+		mark_inode_dirty_sync(vol->vol_ino);
 		ret = 0;
+	} else {
+		kfree(new_label);
 	}
 	return ret;
 }
-- 
2.43.0
Re: [PATCH v2] ntfs: fix missing kstrdup() error check in ntfs_write_volume_label()
Posted by Namjae Jeon 3 weeks ago 
On Fri, May 8, 2026 at 4:29 PM Zhan Xusheng <zhanxusheng1024@gmail.com> wrote:
>
> ntfs_write_volume_label() does not check the return value of
> kstrdup().  If the allocation fails, vol->volume_label is set to
> NULL while the function returns success.  A subsequent
> FS_IOC_GETFSLABEL then returns an empty string even though the
> on-disk label was updated correctly.
>
> Fix by allocating the new label before taking vol_ni->mrec_lock and
> updating any on-disk metadata, so an -ENOMEM from kstrdup() leaves
> both the in-memory and on-disk labels untouched and consistent.  On
> success the preallocated copy replaces the old vol->volume_label.
> Also move mark_inode_dirty_sync() into the success path so that it
> is not called when no metadata was actually modified.
>
> Fixes: 6251f0b0de7d ("ntfs: update super block operations")
> Suggested-by: Hyunchul Lee <hyc.lee@gmail.com>
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
Applied it to #ntfs-next.
Thanks!
Re: [PATCH v2] ntfs: fix missing kstrdup() error check in ntfs_write_volume_label()
Posted by Hyunchul Lee 3 weeks ago 
2026년 5월 8일 (금) 오후 4:29, Zhan Xusheng <zhanxusheng1024@gmail.com>님이 작성:
>
> ntfs_write_volume_label() does not check the return value of
> kstrdup().  If the allocation fails, vol->volume_label is set to
> NULL while the function returns success.  A subsequent
> FS_IOC_GETFSLABEL then returns an empty string even though the
> on-disk label was updated correctly.
>
> Fix by allocating the new label before taking vol_ni->mrec_lock and
> updating any on-disk metadata, so an -ENOMEM from kstrdup() leaves
> both the in-memory and on-disk labels untouched and consistent.  On
> success the preallocated copy replaces the old vol->volume_label.
> Also move mark_inode_dirty_sync() into the success path so that it
> is not called when no metadata was actually modified.
>
> Fixes: 6251f0b0de7d ("ntfs: update super block operations")
> Suggested-by: Hyunchul Lee <hyc.lee@gmail.com>
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>

Looks good to me.

Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>

> ---
>  fs/ntfs/super.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
> index 22dc7865eca7..ba99c497ee38 100644
> --- a/fs/ntfs/super.c
> +++ b/fs/ntfs/super.c
> @@ -413,6 +413,7 @@ int ntfs_write_volume_label(struct ntfs_volume *vol, char *label)
>  {
>         struct ntfs_inode *vol_ni = NTFS_I(vol->vol_ino);
>         struct ntfs_attr_search_ctx *ctx;
> +       char *new_label;
>         __le16 *uname;
>         int uname_len, ret;
>
> @@ -425,7 +426,7 @@ int ntfs_write_volume_label(struct ntfs_volume *vol, char *label)
>                 return uname_len;
>         }
>
> -       if (uname_len  > NTFS_MAX_LABEL_LEN) {
> +       if (uname_len > NTFS_MAX_LABEL_LEN) {
>                 ntfs_error(vol->sb,
>                            "Volume label is too long (max %d characters).",
>                            NTFS_MAX_LABEL_LEN);
> @@ -433,11 +434,22 @@ int ntfs_write_volume_label(struct ntfs_volume *vol, char *label)
>                 return -EINVAL;
>         }
>
> +       /*
> +        * Allocate the in-memory label copy up front. If kstrdup() fails we
> +        * bail out before touching on-disk metadata, so the in-memory label
> +        * and the on-disk label stay in sync.
> +        */
> +       new_label = kstrdup(label, GFP_KERNEL);
> +       if (!new_label) {
> +               kvfree(uname);
> +               return -ENOMEM;
> +       }
> +
>         mutex_lock(&vol_ni->mrec_lock);
>         ctx = ntfs_attr_get_search_ctx(vol_ni, NULL);
>         if (!ctx) {
>                 ret = -ENOMEM;
> -               goto  out;
> +               goto out;
>         }
>
>         if (!ntfs_attr_lookup(AT_VOLUME_NAME, NULL, 0, 0, 0, NULL, 0,
> @@ -450,12 +462,14 @@ int ntfs_write_volume_label(struct ntfs_volume *vol, char *label)
>  out:
>         mutex_unlock(&vol_ni->mrec_lock);
>         kvfree(uname);
> -       mark_inode_dirty_sync(vol->vol_ino);
>
>         if (ret >= 0) {
>                 kfree(vol->volume_label);
> -               vol->volume_label = kstrdup(label, GFP_KERNEL);
> +               vol->volume_label = new_label;
> +               mark_inode_dirty_sync(vol->vol_ino);
>                 ret = 0;
> +       } else {
> +               kfree(new_label);
>         }
>         return ret;
>  }
> --
> 2.43.0
>


-- 
Thanks,
Hyunchul

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.