diff options
author | Matthew Ahrens <[email protected]> | 2020-07-11 17:18:02 -0700 |
---|---|---|
committer | GitHub <[email protected]> | 2020-07-11 17:18:02 -0700 |
commit | e59a377a8fdbf4e66864d1654c0055fdff5f12f4 (patch) | |
tree | 609771a9e795d30ef8b6d5c451a3863af304224a /module/zfs | |
parent | 217f48373f9878a3cd714b8007444f46101aad9e (diff) |
filesystem_limit/snapshot_limit is incorrectly enforced against root
The filesystem_limit and snapshot_limit properties limit the number of
filesystems or snapshots that can be created below this dataset.
According to the manpage, "The limit is not enforced if the user is
allowed to change the limit." Two types of users are allowed to change
the limit:
1. Those that have been delegated the `filesystem_limit` or
`snapshot_limit` permission, e.g. with
`zfs allow USER filesystem_limit DATASET`. This works properly.
2. A user with elevated system privileges (e.g. root). This does not
work - the root user will incorrectly get an error when trying to create
a snapshot/filesystem, if it exceeds the `_limit` property.
The problem is that `priv_policy_ns()` does not work if the `cred_t` is
not that of the current process. This happens when
`dsl_enforce_ds_ss_limits()` is called in syncing context (as part of a
sync task's check func) to determine the permissions of the
corresponding user process.
This commit fixes the issue by passing the `task_struct` (typedef'ed as
a `proc_t`) to syncing context, and then using `has_capability()` to
determine if that process is privileged. Note that we still need to
pass the `cred_t` to syncing context so that we can check if the user
was delegated this permission with `zfs allow`.
This problem only impacts Linux. Wrappers are added to FreeBSD but it
continues to use `priv_check_cred()`, which works on arbitrary `cred_t`.
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Ryan Moeller <[email protected]>
Signed-off-by: Matthew Ahrens <[email protected]>
Closes #8226
Closes #10545
Diffstat (limited to 'module/zfs')
-rw-r--r-- | module/zfs/dmu_objset.c | 8 | ||||
-rw-r--r-- | module/zfs/dmu_recv.c | 17 | ||||
-rw-r--r-- | module/zfs/dsl_dataset.c | 14 | ||||
-rw-r--r-- | module/zfs/dsl_dir.c | 29 | ||||
-rw-r--r-- | module/zfs/zcp.c | 1 | ||||
-rw-r--r-- | module/zfs/zcp_synctask.c | 2 |
6 files changed, 49 insertions, 22 deletions
diff --git a/module/zfs/dmu_objset.c b/module/zfs/dmu_objset.c index dd72a7dc2..30be3c4f2 100644 --- a/module/zfs/dmu_objset.c +++ b/module/zfs/dmu_objset.c @@ -1096,6 +1096,7 @@ dmu_objset_create_impl(spa_t *spa, dsl_dataset_t *ds, blkptr_t *bp, typedef struct dmu_objset_create_arg { const char *doca_name; cred_t *doca_cred; + proc_t *doca_proc; void (*doca_userfunc)(objset_t *os, void *arg, cred_t *cr, dmu_tx_t *tx); void *doca_userarg; @@ -1140,7 +1141,7 @@ dmu_objset_create_check(void *arg, dmu_tx_t *tx) } error = dsl_fs_ss_limit_check(pdd, 1, ZFS_PROP_FILESYSTEM_LIMIT, NULL, - doca->doca_cred); + doca->doca_cred, doca->doca_proc); if (error != 0) { dsl_dir_rele(pdd, FTAG); return (error); @@ -1268,6 +1269,7 @@ dmu_objset_create(const char *name, dmu_objset_type_t type, uint64_t flags, doca.doca_name = name; doca.doca_cred = CRED(); + doca.doca_proc = curproc; doca.doca_flags = flags; doca.doca_userfunc = func; doca.doca_userarg = arg; @@ -1296,6 +1298,7 @@ typedef struct dmu_objset_clone_arg { const char *doca_clone; const char *doca_origin; cred_t *doca_cred; + proc_t *doca_proc; } dmu_objset_clone_arg_t; /*ARGSUSED*/ @@ -1324,7 +1327,7 @@ dmu_objset_clone_check(void *arg, dmu_tx_t *tx) } error = dsl_fs_ss_limit_check(pdd, 1, ZFS_PROP_FILESYSTEM_LIMIT, NULL, - doca->doca_cred); + doca->doca_cred, doca->doca_proc); if (error != 0) { dsl_dir_rele(pdd, FTAG); return (SET_ERROR(EDQUOT)); @@ -1383,6 +1386,7 @@ dmu_objset_clone(const char *clone, const char *origin) doca.doca_clone = clone; doca.doca_origin = origin; doca.doca_cred = CRED(); + doca.doca_proc = curproc; int rv = dsl_sync_task(clone, dmu_objset_clone_check, dmu_objset_clone_sync, &doca, diff --git a/module/zfs/dmu_recv.c b/module/zfs/dmu_recv.c index 17ebeb682..2f3507914 100644 --- a/module/zfs/dmu_recv.c +++ b/module/zfs/dmu_recv.c @@ -126,6 +126,7 @@ typedef struct dmu_recv_begin_arg { const char *drba_origin; dmu_recv_cookie_t *drba_cookie; cred_t *drba_cred; + proc_t *drba_proc; dsl_crypto_params_t *drba_dcp; } dmu_recv_begin_arg_t; @@ -398,7 +399,7 @@ recv_begin_check_existing_impl(dmu_recv_begin_arg_t *drba, dsl_dataset_t *ds, * against that limit. */ error = dsl_fs_ss_limit_check(ds->ds_dir, 1, ZFS_PROP_SNAPSHOT_LIMIT, - NULL, drba->drba_cred); + NULL, drba->drba_cred, drba->drba_proc); if (error != 0) return (error); @@ -679,14 +680,16 @@ dmu_recv_begin_check(void *arg, dmu_tx_t *tx) * filesystems and increment those counts during begin_sync). */ error = dsl_fs_ss_limit_check(ds->ds_dir, 1, - ZFS_PROP_FILESYSTEM_LIMIT, NULL, drba->drba_cred); + ZFS_PROP_FILESYSTEM_LIMIT, NULL, + drba->drba_cred, drba->drba_proc); if (error != 0) { dsl_dataset_rele(ds, FTAG); return (error); } error = dsl_fs_ss_limit_check(ds->ds_dir, 1, - ZFS_PROP_SNAPSHOT_LIMIT, NULL, drba->drba_cred); + ZFS_PROP_SNAPSHOT_LIMIT, NULL, + drba->drba_cred, drba->drba_proc); if (error != 0) { dsl_dataset_rele(ds, FTAG); return (error); @@ -1152,6 +1155,7 @@ dmu_recv_begin(char *tofs, char *tosnap, dmu_replay_record_t *drr_begin, drc->drc_force = force; drc->drc_resumable = resumable; drc->drc_cred = CRED(); + drc->drc_proc = curproc; drc->drc_clone = (origin != NULL); if (drc->drc_drrb->drr_magic == BSWAP_64(DMU_BACKUP_MAGIC)) { @@ -1199,6 +1203,7 @@ dmu_recv_begin(char *tofs, char *tosnap, dmu_replay_record_t *drr_begin, drba.drba_origin = origin; drba.drba_cookie = drc; drba.drba_cred = CRED(); + drba.drba_proc = curproc; if (drc->drc_featureflags & DMU_BACKUP_FEATURE_RESUMING) { err = dsl_sync_task(tofs, @@ -3133,7 +3138,8 @@ dmu_recv_end_check(void *arg, dmu_tx_t *tx) return (error); } error = dsl_dataset_snapshot_check_impl(origin_head, - drc->drc_tosnap, tx, B_TRUE, 1, drc->drc_cred); + drc->drc_tosnap, tx, B_TRUE, 1, + drc->drc_cred, drc->drc_proc); dsl_dataset_rele(origin_head, FTAG); if (error != 0) return (error); @@ -3141,7 +3147,8 @@ dmu_recv_end_check(void *arg, dmu_tx_t *tx) error = dsl_destroy_head_check_impl(drc->drc_ds, 1); } else { error = dsl_dataset_snapshot_check_impl(drc->drc_ds, - drc->drc_tosnap, tx, B_TRUE, 1, drc->drc_cred); + drc->drc_tosnap, tx, B_TRUE, 1, + drc->drc_cred, drc->drc_proc); } return (error); } diff --git a/module/zfs/dsl_dataset.c b/module/zfs/dsl_dataset.c index 4986bc449..c04f47d36 100644 --- a/module/zfs/dsl_dataset.c +++ b/module/zfs/dsl_dataset.c @@ -1465,7 +1465,7 @@ dsl_dataset_snapshot_reserve_space(dsl_dataset_t *ds, dmu_tx_t *tx) int dsl_dataset_snapshot_check_impl(dsl_dataset_t *ds, const char *snapname, - dmu_tx_t *tx, boolean_t recv, uint64_t cnt, cred_t *cr) + dmu_tx_t *tx, boolean_t recv, uint64_t cnt, cred_t *cr, proc_t *proc) { int error; uint64_t value; @@ -1510,7 +1510,7 @@ dsl_dataset_snapshot_check_impl(dsl_dataset_t *ds, const char *snapname, */ if (cnt != 0 && cr != NULL) { error = dsl_fs_ss_limit_check(ds->ds_dir, cnt, - ZFS_PROP_SNAPSHOT_LIMIT, NULL, cr); + ZFS_PROP_SNAPSHOT_LIMIT, NULL, cr, proc); if (error != 0) return (error); } @@ -1611,7 +1611,7 @@ dsl_dataset_snapshot_check(void *arg, dmu_tx_t *tx) if (error == 0) { error = dsl_fs_ss_limit_check(ds->ds_dir, cnt, ZFS_PROP_SNAPSHOT_LIMIT, NULL, - ddsa->ddsa_cr); + ddsa->ddsa_cr, ddsa->ddsa_proc); dsl_dataset_rele(ds, FTAG); } @@ -1649,7 +1649,7 @@ dsl_dataset_snapshot_check(void *arg, dmu_tx_t *tx) if (error == 0) { /* passing 0/NULL skips dsl_fs_ss_limit_check */ error = dsl_dataset_snapshot_check_impl(ds, - atp + 1, tx, B_FALSE, 0, NULL); + atp + 1, tx, B_FALSE, 0, NULL, NULL); dsl_dataset_rele(ds, FTAG); } @@ -1927,6 +1927,7 @@ dsl_dataset_snapshot(nvlist_t *snaps, nvlist_t *props, nvlist_t *errors) ddsa.ddsa_props = props; ddsa.ddsa_errors = errors; ddsa.ddsa_cr = CRED(); + ddsa.ddsa_proc = curproc; if (error == 0) { error = dsl_sync_task(firstname, dsl_dataset_snapshot_check, @@ -1974,7 +1975,7 @@ dsl_dataset_snapshot_tmp_check(void *arg, dmu_tx_t *tx) /* NULL cred means no limit check for tmp snapshot */ error = dsl_dataset_snapshot_check_impl(ds, ddsta->ddsta_snapname, - tx, B_FALSE, 0, NULL); + tx, B_FALSE, 0, NULL, NULL); if (error != 0) { dsl_dataset_rele(ds, FTAG); return (error); @@ -3477,7 +3478,7 @@ dsl_dataset_promote_check(void *arg, dmu_tx_t *tx) /* Check that there is enough space and limit headroom here */ err = dsl_dir_transfer_possible(origin_ds->ds_dir, hds->ds_dir, - 0, ss_mv_cnt, ddpa->used, ddpa->cr); + 0, ss_mv_cnt, ddpa->used, ddpa->cr, ddpa->proc); if (err != 0) goto out; @@ -3904,6 +3905,7 @@ dsl_dataset_promote(const char *name, char *conflsnap) ddpa.ddpa_clonename = name; ddpa.err_ds = fnvlist_alloc(); ddpa.cr = CRED(); + ddpa.proc = curproc; error = dsl_sync_task(name, dsl_dataset_promote_check, dsl_dataset_promote_sync, &ddpa, diff --git a/module/zfs/dsl_dir.c b/module/zfs/dsl_dir.c index 977600116..da6103b4b 100644 --- a/module/zfs/dsl_dir.c +++ b/module/zfs/dsl_dir.c @@ -749,7 +749,8 @@ typedef enum { } enforce_res_t; static enforce_res_t -dsl_enforce_ds_ss_limits(dsl_dir_t *dd, zfs_prop_t prop, cred_t *cr) +dsl_enforce_ds_ss_limits(dsl_dir_t *dd, zfs_prop_t prop, + cred_t *cr, proc_t *proc) { enforce_res_t enforce = ENFORCE_ALWAYS; uint64_t obj; @@ -764,7 +765,13 @@ dsl_enforce_ds_ss_limits(dsl_dir_t *dd, zfs_prop_t prop, cred_t *cr) if (crgetzoneid(cr) != GLOBAL_ZONEID) return (ENFORCE_ALWAYS); - if (secpolicy_zfs(cr) == 0) + /* + * We are checking the saved credentials of the user process, which is + * not the current process. Note that we can't use secpolicy_zfs(), + * because it only works if the cred is that of the current process (on + * Linux). + */ + if (secpolicy_zfs_proc(cr, proc) == 0) return (ENFORCE_NEVER); #endif @@ -799,7 +806,7 @@ dsl_enforce_ds_ss_limits(dsl_dir_t *dd, zfs_prop_t prop, cred_t *cr) */ int dsl_fs_ss_limit_check(dsl_dir_t *dd, uint64_t delta, zfs_prop_t prop, - dsl_dir_t *ancestor, cred_t *cr) + dsl_dir_t *ancestor, cred_t *cr, proc_t *proc) { objset_t *os = dd->dd_pool->dp_meta_objset; uint64_t limit, count; @@ -819,7 +826,7 @@ dsl_fs_ss_limit_check(dsl_dir_t *dd, uint64_t delta, zfs_prop_t prop, * are allowed to change the limit on the current dataset, but there * is another limit in the tree above. */ - enforce = dsl_enforce_ds_ss_limits(dd, prop, cr); + enforce = dsl_enforce_ds_ss_limits(dd, prop, cr, proc); if (enforce == ENFORCE_NEVER) return (0); @@ -871,7 +878,7 @@ dsl_fs_ss_limit_check(dsl_dir_t *dd, uint64_t delta, zfs_prop_t prop, if (dd->dd_parent != NULL) err = dsl_fs_ss_limit_check(dd->dd_parent, delta, prop, - ancestor, cr); + ancestor, cr, proc); return (err); } @@ -1840,6 +1847,7 @@ typedef struct dsl_dir_rename_arg { const char *ddra_oldname; const char *ddra_newname; cred_t *ddra_cred; + proc_t *ddra_proc; } dsl_dir_rename_arg_t; typedef struct dsl_valid_rename_arg { @@ -2018,7 +2026,8 @@ dsl_dir_rename_check(void *arg, dmu_tx_t *tx) } error = dsl_dir_transfer_possible(dd->dd_parent, - newparent, fs_cnt, ss_cnt, myspace, ddra->ddra_cred); + newparent, fs_cnt, ss_cnt, myspace, + ddra->ddra_cred, ddra->ddra_proc); if (error != 0) { dsl_dir_rele(newparent, FTAG); dsl_dir_rele(dd, FTAG); @@ -2138,6 +2147,7 @@ dsl_dir_rename(const char *oldname, const char *newname) ddra.ddra_oldname = oldname; ddra.ddra_newname = newname; ddra.ddra_cred = CRED(); + ddra.ddra_proc = curproc; return (dsl_sync_task(oldname, dsl_dir_rename_check, dsl_dir_rename_sync, &ddra, @@ -2146,7 +2156,8 @@ dsl_dir_rename(const char *oldname, const char *newname) int dsl_dir_transfer_possible(dsl_dir_t *sdd, dsl_dir_t *tdd, - uint64_t fs_cnt, uint64_t ss_cnt, uint64_t space, cred_t *cr) + uint64_t fs_cnt, uint64_t ss_cnt, uint64_t space, + cred_t *cr, proc_t *proc) { dsl_dir_t *ancestor; int64_t adelta; @@ -2160,11 +2171,11 @@ dsl_dir_transfer_possible(dsl_dir_t *sdd, dsl_dir_t *tdd, return (SET_ERROR(ENOSPC)); err = dsl_fs_ss_limit_check(tdd, fs_cnt, ZFS_PROP_FILESYSTEM_LIMIT, - ancestor, cr); + ancestor, cr, proc); if (err != 0) return (err); err = dsl_fs_ss_limit_check(tdd, ss_cnt, ZFS_PROP_SNAPSHOT_LIMIT, - ancestor, cr); + ancestor, cr, proc); if (err != 0) return (err); diff --git a/module/zfs/zcp.c b/module/zfs/zcp.c index 305d09239..793e0e4f0 100644 --- a/module/zfs/zcp.c +++ b/module/zfs/zcp.c @@ -1150,6 +1150,7 @@ zcp_eval(const char *poolname, const char *program, boolean_t sync, runinfo.zri_outnvl = outnvl; runinfo.zri_result = 0; runinfo.zri_cred = CRED(); + runinfo.zri_proc = curproc; runinfo.zri_timed_out = B_FALSE; runinfo.zri_canceled = B_FALSE; runinfo.zri_sync = sync; diff --git a/module/zfs/zcp_synctask.c b/module/zfs/zcp_synctask.c index d9b48a709..4e0fa0d85 100644 --- a/module/zfs/zcp_synctask.c +++ b/module/zfs/zcp_synctask.c @@ -186,6 +186,7 @@ zcp_synctask_promote(lua_State *state, boolean_t sync, nvlist_t *err_details) ddpa.ddpa_clonename = dsname; ddpa.err_ds = err_details; ddpa.cr = ri->zri_cred; + ddpa.proc = ri->zri_proc; /* * If there was a snapshot name conflict, then err_ds will be filled @@ -269,6 +270,7 @@ zcp_synctask_snapshot(lua_State *state, boolean_t sync, nvlist_t *err_details) ddsa.ddsa_errors = NULL; ddsa.ddsa_props = NULL; ddsa.ddsa_cr = ri->zri_cred; + ddsa.ddsa_proc = ri->zri_proc; ddsa.ddsa_snaps = fnvlist_alloc(); fnvlist_add_boolean(ddsa.ddsa_snaps, dsname); |