Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression in 2.2.0 when creating fs with sharesmb=off and kernel was built with CONFIG_USER_NS=n #15419

Closed
mscdex opened this issue Oct 18, 2023 · 2 comments
Labels
Type: Defect Incorrect behavior (e.g. crash, hang) Type: Regression Indicates a functional regression

Comments

@mscdex
Copy link

mscdex commented Oct 18, 2023

System information

Type Version/Name
Distribution Name Buildroot
Distribution Version 2023.02.5
Kernel Version 6.1
Architecture amd64
OpenZFS Version 2.2.0

Describe the problem you're observing

Using zpool create ... -O sharesmb=off -O sharenfs=off ... fails now with 2.2.0 with an error saying:

'sharesmb' cannot be set while dataset 'zoned' property is set

despite not having even set zoned in my zpool create command. I don't have a complicated setup -- I'm not using zones, namespaces, or even multiple datasets, just a single "global" zfs.

After some digging into what causes this error message, it seems zfs thinks I'm trying to create a filesystem in a non-global zone (even when trying again and explicitly passing -O zoned=off to my zpool create).

I then found in the original PR that added this zone support that it's implicitly requiring CONFIG_USER_NS to be enabled in the kernel. If it's not enabled, then getzoneid() will return 0. When the callers of this function then compare the result to the GLOBAL_ZONEID, which is a non-zero value, it will always assume we're not dealing with the global zone, which then causes the aforementioned error.

It seems like the easy fix (although perhaps not robust as I'm not familiar with namespaces) would be to check if /proc/self/ns/user exists and if it doesn't, just return GLOBAL_ZONEID. Perhaps it may be enough to tweak the existing code such that if readlink() < 0, check for errno == ENOENT and in that case return GLOBAL_ZONEID.

Describe how to reproduce the problem

Try to create a zpool with -O sharesmb=off -O sharenfs=off with zfs 2.2.0.

Include any warning/errors/backtraces from the system logs

@mscdex mscdex added the Type: Defect Incorrect behavior (e.g. crash, hang) label Oct 18, 2023
@usaleem-ix
Copy link
Contributor

I agree with the analysis above and I think that #15407 is also caused by getzoneid().

I could not reproduce both issues until I disabled CONFIG_USER_NS in the kernel.

The case where /proc/self/ns/user does not exist (by disabling CONFIG_USER_NS), and 0 is returned by getzoneid(), actually causes share_mount_one() and zfs_valid_prop_list() to fail as we are checking for getzoneid() != GLOBAL_ZONEID in both places, in case zoned is set to off.

While explicitly mounting a dataset using zfs mount, it fails with permission denied error as zoned is not set and getzoneid() returns 0 in this case:

	} else if (!zoned && getzoneid() != GLOBAL_ZONEID) {
		if (!explicit)
			return (0);

		(void) fprintf(stderr, gettext("cannot %s '%s': "
		    "permission denied\n"), cmdname,
		    zfs_get_name(zhp));
		return (1);
	}

getzoneid() was updated as part of this commit that adds Linux namespace delegation support. Before that, it was returning GLOBAL_ZONEID up till zfs-2.1.13 release.

I can also confirm that with the approach mentioned to fix getzoneid() above in the issue description works for explicitly mounting the dataset and the permission denied error does not occur anymore.

@mscdex
Copy link
Author

mscdex commented Jan 18, 2024

Closing now as this was apparently fixed with #15560.

@mscdex mscdex closed this as completed Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Defect Incorrect behavior (e.g. crash, hang) Type: Regression Indicates a functional regression
Projects
None yet
Development

No branches or pull requests

3 participants