Re: [PATCH] fs/ntfs3: reject FS_IOC_SETFSLABEL on readonly mounts

[liubaolin]

Dear maintainer,

This patch fixes how `ntfs3` handles `FS_IOC_SETFSLABEL` on readonly mounts.

While looking at this ioctl path, I noticed that on a readonly-mounted 
`ntfs3` filesystem, users can still call `FS_IOC_SETFSLABEL` to change 
the volume label, and the operation returns success. If the label is 
read again immediately afterwards, the current mount session shows the 
new value, but that value is not actually written to disk. After 
unmounting and mounting the filesystem again, the label goes back to the 
old one.

For a readonly mount, this operation should not be allowed in the first 
place, so the more reasonable behavior here is to return `-EROFS` directly.



To reproduce the issue, I wrote a simple test program, 
`ntfs3_label_ioctl.c`, which uses `FS_IOC_GETFSLABEL` and 
`FS_IOC_SETFSLABEL` directly to read and write the volume label. Running 
this program against a readonly-mounted `ntfs3` image makes the problem 
easy to observe.
（The source code for ntfs3_label_ioctl.c is listed at the end of the 
email.）

*********************************************************************************************************

First, build the test program:
gcc -O2 -Wall -o ntfs3_label_ioctl ntfs3_label_ioctl.c

Then prepare an NTFS image:
truncate -s 128M /tmp/ntfs3.img
mkntfs -F -q /tmp/ntfs3.img
mkdir -p /tmp/ntfs3-mnt

Mount it read-write once and explicitly set the label to `OLDLABEL`:
sudo mount -o loop -t ntfs3 /tmp/ntfs3.img /tmp/ntfs3-mnt
./ntfs3_label_ioctl set OLDLABEL /tmp/ntfs3-mnt
./ntfs3_label_ioctl get /tmp/ntfs3-mnt
sudo umount /tmp/ntfs3-mnt

At this point, the output should be:
$ ./ntfs3_label_ioctl set OLDLABEL /tmp/ntfs3-mnt
SETFSLABEL succeeded

$ ./ntfs3_label_ioctl get /tmp/ntfs3-mnt
label='OLDLABEL'

Then mount the image readonly:
sudo mount -o loop,ro -t ntfs3 /tmp/ntfs3.img /tmp/ntfs3-mnt

Read the current label:
./ntfs3_label_ioctl get /tmp/ntfs3-mnt

Try to set a new label:
./ntfs3_label_ioctl set NEWLABEL /tmp/ntfs3-mnt

Read the label again:
./ntfs3_label_ioctl get /tmp/ntfs3-mnt

Finally, unmount and mount it readonly again, then read the label once more:
sudo umount /tmp/ntfs3-mnt
sudo mount -o loop,ro -t ntfs3 /tmp/ntfs3.img /tmp/ntfs3-mnt
./ntfs3_label_ioctl get /tmp/ntfs3-mnt
**********************************************************************************************************

On the unpatched kernel, I see the following behavior:

$ ./ntfs3_label_ioctl get /tmp/ntfs3-mnt
label='OLDLABEL'

$ ./ntfs3_label_ioctl set NEWLABEL /tmp/ntfs3-mnt
SETFSLABEL succeeded

$ ./ntfs3_label_ioctl get /tmp/ntfs3-mnt
label='NEWLABEL'

$ sudo umount /tmp/ntfs3-mnt
$ sudo mount -o loop,ro -t ntfs3 /tmp/ntfs3.img /tmp/ntfs3-mnt
$ ./ntfs3_label_ioctl get /tmp/ntfs3-mnt
label='OLDLABEL'

So `SETFSLABEL` succeeds on a readonly mount, and the label visible in 
the current mount session changes to `NEWLABEL`, but after remounting it 
goes back to `OLDLABEL`. This shows that the change was not actually 
persisted to disk, and only affected the current mount session.
**********************************************************************************************************

With this patch applied, the same test behaves like this:

$ ./ntfs3_label_ioctl get /tmp/ntfs3-mnt
label='OLDLABEL'

$ ./ntfs3_label_ioctl set NEWLABEL /tmp/ntfs3-mnt
SETFSLABEL failed: errno=30 (Read-only file system)

$ ./ntfs3_label_ioctl get /tmp/ntfs3-mnt
label='OLDLABEL'

$ sudo umount /tmp/ntfs3-mnt
$ sudo mount -o loop,ro -t ntfs3 /tmp/ntfs3.img /tmp/ntfs3-mnt
$ ./ntfs3_label_ioctl get /tmp/ntfs3-mnt
label='OLDLABEL'

After the patch, the ioctl correctly returns `-EROFS` on readonly 
mounts, and the inconsistent behavior between the current mount session 
and a remount is gone.

Thanks,
Baolin


**********************************************************************************************************
ntfs3_label_ioctl.c is as follows:
#define _GNU_SOURCE
#include <errno.h>
#include <fcntl.h>
#include <linux/fs.h>
#include <stdio.h>
#include <string.h>
#include <sys/ioctl.h>
#include <unistd.h>

static int do_get(int fd)
{
	char label[FSLABEL_MAX] = {0};
	int ret;

	ret = ioctl(fd, FS_IOC_GETFSLABEL, label);
	if (ret < 0) {
		perror("GETFSLABEL");
		return 1;
	}

	printf("label='%s'\n", label);
	return 0;
}

static int do_set(int fd, const char *newlabel)
{
	char label[FSLABEL_MAX] = {0};
	int ret;

	strncpy(label, newlabel, sizeof(label) - 1);

	ret = ioctl(fd, FS_IOC_SETFSLABEL, label);
	if (ret < 0) {
		printf("SETFSLABEL failed: errno=%d (%s)\n", errno, strerror(errno));
		return 1;
	}

	printf("SETFSLABEL succeeded\n");
	return 0;
}

static void usage(const char *prog)
{
	fprintf(stderr,
		"Usage:\n"
		"  %s get [path]\n"
		"  %s set <newlabel> [path]\n",
		prog, prog);
}

int main(int argc, char *argv[])
{
	const char *path = "/tmp/ntfs3-mnt";
	int fd, ret;

	if (argc < 2) {
		usage(argv[0]);
		return 1;
	}

	if (!strcmp(argv[1], "get")) {
		if (argc >= 3)
			path = argv[2];
	} else if (!strcmp(argv[1], "set")) {
		if (argc < 3) {
			usage(argv[0]);
			return 1;
		}
		if (argc >= 4)
			path = argv[3];
	} else {
		usage(argv[0]);
		return 1;
	}

	fd = open(path, O_RDONLY | O_DIRECTORY);
	if (fd < 0) {
		perror("open");
		return 1;
	}

	if (!strcmp(argv[1], "get"))
		ret = do_get(fd);
	else
		ret = do_set(fd, argv[2]);

	close(fd);
	return ret;
}

在 2026/6/30 08:59, Baolin Liu 写道:
> From: Baolin Liu <liubaolin@xxxxxxxxxx>
>
> FS_IOC_SETFSLABEL does not check for readonly mounts and
> can report success without persisting the label change.
>
> Return -EROFS on readonly mounts.
>
> Fixes: 21dc07ac9c25 ("ntfs3: add FS_IOC_SETFSLABEL ioctl")
> Signed-off-by: Baolin Liu <liubaolin@xxxxxxxxxx>
> ---
>   fs/ntfs3/file.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c
> index b041639ab406..f43baa46ec5c 100644
> --- a/fs/ntfs3/file.c
> +++ b/fs/ntfs3/file.c
> @@ -105,6 +105,9 @@ static int ntfs_ioctl_set_volume_label(struct ntfs_sb_info *sbi, u8 __user *buf)
>   	if (!capable(CAP_SYS_ADMIN))
>   		return -EPERM;
>   
> +	if (sb_rdonly(sbi->sb))
> +		return -EROFS;
> +
>   	if (copy_from_user(user, buf, FSLABEL_MAX))
>   		return -EFAULT;
>