Fix filesystem_limit enforcement

[loli10K]

Motivation and Context

Fix #8226
Alternative version #8228

filesystem_limit should not be enforced for users allowed to change it.

Current behaviour:

root@linux:~# # misc functions
root@linux:~# function is_linux() {
>    if [[ "$(uname)" == "Linux" ]]; then
>       return 0
>    else
>       return 1
>    fi
> }
root@linux:~# # setup
root@linux:~# POOLNAME='testpool'
root@linux:~# if is_linux; then
>    TMPDIR='/var/tmp'
>    mountpoint -q $TMPDIR || mount -t tmpfs tmpfs $TMPDIR
>    zpool destroy $POOLNAME
>    rm -f $TMPDIR/zpool_$POOLNAME.dat
>    truncate -s 128m $TMPDIR/zpool_$POOLNAME.dat
>    zpool create $POOLNAME $TMPDIR/zpool_$POOLNAME.dat
> else
>    TMPDIR='/tmp'
>    zpool destroy $POOLNAME
>    rm -f $TMPDIR/zpool_$POOLNAME.dat
>    truncate -s 128m $TMPDIR/zpool_$POOLNAME.dat
>    zpool create $POOLNAME $TMPDIR/zpool_$POOLNAME.dat
> fi
root@linux:~# useradd staff1
root@linux:~# sudo zfs create -o filesystem_limit=3 $POOLNAME/limited
root@linux:~# sudo zfs allow -u staff1 create,mount $POOLNAME/limited
root@linux:~# zfs create $POOLNAME/limited/fs1
root@linux:~# zfs create $POOLNAME/limited/fs2
root@linux:~# zfs create $POOLNAME/limited/fs3
root@linux:~# zfs create $POOLNAME/limited/fs4
cannot create 'testpool/limited/fs4': out of space
root@linux:~# zfs create $POOLNAME/limited/fs4
cannot create 'testpool/limited/fs4': out of space
root@linux:~# zfs create $POOLNAME/limited/fs5
cannot create 'testpool/limited/fs5': out of space
root@linux:~# 

Patch applied:

root@linux:~# modprobe zfs
root@linux:~# # misc functions
root@linux:~# function is_linux() {
>    if [[ "$(uname)" == "Linux" ]]; then
>       return 0
>    else
>       return 1
>    fi
> }
root@linux:~# # setup
root@linux:~# POOLNAME='testpool'
root@linux:~# if is_linux; then
>    TMPDIR='/var/tmp'
>    mountpoint -q $TMPDIR || mount -t tmpfs tmpfs $TMPDIR
>    zpool destroy $POOLNAME
>    rm -f $TMPDIR/zpool_$POOLNAME.dat
>    truncate -s 128m $TMPDIR/zpool_$POOLNAME.dat
>    zpool create $POOLNAME $TMPDIR/zpool_$POOLNAME.dat
> else
>    TMPDIR='/tmp'
>    zpool destroy $POOLNAME
>    rm -f $TMPDIR/zpool_$POOLNAME.dat
>    truncate -s 128m $TMPDIR/zpool_$POOLNAME.dat
>    zpool create $POOLNAME $TMPDIR/zpool_$POOLNAME.dat
> fi
cannot open 'testpool': no such pool
root@linux:~# useradd staff1
root@linux:~# sudo zfs create -o filesystem_limit=3 $POOLNAME/limited
root@linux:~# sudo zfs allow -u staff1 create,mount $POOLNAME/limited
root@linux:~# zfs create $POOLNAME/limited/fs1
root@linux:~# zfs create $POOLNAME/limited/fs2
root@linux:~# zfs create $POOLNAME/limited/fs3
root@linux:~# zfs create $POOLNAME/limited/fs4
root@linux:~# zfs create $POOLNAME/limited/fs4
cannot create 'testpool/limited/fs4': dataset already exists
root@linux:~# zfs create $POOLNAME/limited/fs5

Description

The proposed solution is to verify the CRED() only in open context and then use the kcred for the same secpolicy_zfs() checks in syncing context: this allows the check to be correctly performed a second time while still verifying the caller credentials.

How Has This Been Tested?

Update ZFS Test Suite

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the ZFS on Linux code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• All new and existing tests passed.
• All commit messages are properly formatted and contain Signed-off-by.

[codecov]

Codecov Report

Merging #8280 into master will increase coverage by 8.64%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #8280      +/-   ##
==========================================
+ Coverage   68.24%   76.89%   +8.64%     
==========================================
  Files         335      368      +33     
  Lines      109770   114827    +5057     
==========================================
+ Hits        74917    88293   +13376     
+ Misses      34853    26534    -8319

Flag	Coverage Δ
#kernel	76.57% <100%> (+9.5%)	⬆️
#user	66.67% <25%> (+6.34%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6955b40...9259403. Read the comment docs.

[behlendorf]

@loli10K thanks for picking up this work. I'm going to close #8228, and let's more forward with your more targeted alternate solution.

[loli10K]

This solution seems to work for "filesystem_limit" but not with "snapshot_limit": for instance in dsl_dataset_snapshot_check() we call dsl_fs_ss_limit_check() only in syncing context:

https://github.com/zfsonlinux/zfs/blob/master/module/zfs/dsl_dataset.c#L1463.

[tcaputi]

@loli10K any updates coming here?

[loli10K]

@tcaputi this code won't work without modifying every dsl_fs_ss_limit_check() consumer calling the function only in syncing context, which could add slowness in day-to-day operations.

I don't own the hardware to test such a change performance-wise, i'm going to close this.