From 60b0a963f56182beb2d57823960f3ddb987a584a Mon Sep 17 00:00:00 2001 From: loli10K Date: Fri, 18 Jan 2019 18:58:46 +0100 Subject: [PATCH] Off-by-one in zap_leaf_array_create() Trying to set user properties with their length 1 byte shorter than the maximum size triggers an assertion failure in zap_leaf_array_create(): panic[cpu0]/thread=ffffff000a092c40: assertion failed: num_integers * integer_size < (8<<10) (0x2000 < 0x2000), file: ../../common/fs/zfs/zap_leaf.c, line: 233 ffffff000a092500 genunix:process_type+167c35 () ffffff000a0925a0 zfs:zap_leaf_array_create+1d2 () ffffff000a092650 zfs:zap_entry_create+1be () ffffff000a092720 zfs:fzap_update+ed () ffffff000a0927d0 zfs:zap_update+1a5 () ffffff000a0928d0 zfs:dsl_prop_set_sync_impl+5c6 () ffffff000a092970 zfs:dsl_props_set_sync_impl+fc () ffffff000a0929b0 zfs:dsl_props_set_sync+79 () ffffff000a0929f0 zfs:dsl_sync_task_sync+10a () ffffff000a092a80 zfs:dsl_pool_sync+3a3 () ffffff000a092b50 zfs:spa_sync+4e6 () ffffff000a092c20 zfs:txg_sync_thread+297 () ffffff000a092c30 unix:thread_start+8 () This patch simply corrects the assertion. Reviewed-by: Matt Ahrens Reviewed-by: Brian Behlendorf Signed-off-by: loli10K Closes #8278 --- module/zfs/zap_leaf.c | 5 +--- .../zfs_set/user_property_001_pos.ksh | 27 ++++++++++++++----- .../zfs_set/user_property_003_neg.ksh | 27 ++++++++++++++----- 3 files changed, 43 insertions(+), 16 deletions(-) diff --git a/module/zfs/zap_leaf.c b/module/zfs/zap_leaf.c index d168ad29ae..b421dd5038 100644 --- a/module/zfs/zap_leaf.c +++ b/module/zfs/zap_leaf.c @@ -45,9 +45,6 @@ static uint16_t *zap_leaf_rehash_entry(zap_leaf_t *l, uint16_t entry); #define CHAIN_END 0xffff /* end of the chunk chain */ -/* half the (current) minimum block size */ -#define MAX_ARRAY_BYTES (8<<10) - #define LEAF_HASH(l, h) \ ((ZAP_LEAF_HASH_NUMENTRIES(l)-1) & \ ((h) >> \ @@ -233,7 +230,7 @@ zap_leaf_array_create(zap_leaf_t *l, const char *buf, int shift = (integer_size - 1) * 8; int len = num_integers; - ASSERT3U(num_integers * integer_size, <, MAX_ARRAY_BYTES); + ASSERT3U(num_integers * integer_size, <=, ZAP_MAXVALUELEN); while (len > 0) { uint16_t chunk = zap_leaf_chunk_alloc(l); diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_set/user_property_001_pos.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_set/user_property_001_pos.ksh index fb03d7c971..16b9638787 100755 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_set/user_property_001_pos.ksh +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_set/user_property_001_pos.ksh @@ -50,17 +50,32 @@ log_assert "ZFS can set any valid user defined property to the non-readonly " \ "dataset." log_onexit cleanup_user_prop $TESTPOOL -typeset -i i=0 -while ((i < 10)); do +typeset -a names=() +typeset -a values=() + +# Longest property name (255 bytes) +names+=("$(awk 'BEGIN { printf "x:"; while (c++ < 253) printf "a" }')") +values+=("too-long-property-name") +# Longest property value (8191 bytes) +names+=("too:long:property:value") +values+=("$(awk 'BEGIN { while (c++ < 8191) printf "A" }')") +# Valid property names +for i in {1..10}; do typeset -i len ((len = RANDOM % 32)) - typeset user_prop=$(valid_user_property $len) + names+=("$(valid_user_property $len)") ((len = RANDOM % 512)) - typeset value=$(user_property_value $len) + values+=("$(user_property_value $len)") +done + +typeset -i i=0 +while ((i < ${#names[@]})); do + typeset name="${names[$i]}" + typeset value="${values[$i]}" for dtst in $TESTPOOL $TESTPOOL/$TESTFS $TESTPOOL/$TESTVOL; do - log_must eval "zfs set $user_prop='$value' $dtst" - log_must eval "check_user_prop $dtst $user_prop '$value'" + log_must eval "zfs set $name='$value' $dtst" + log_must eval "check_user_prop $dtst $name '$value'" done ((i += 1)) diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_set/user_property_003_neg.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_set/user_property_003_neg.ksh index 6a90665f4b..3d8c1e7be1 100755 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_set/user_property_003_neg.ksh +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_set/user_property_003_neg.ksh @@ -48,17 +48,32 @@ verify_runnable "both" log_assert "ZFS can handle invalid user property." log_onexit cleanup_user_prop $TESTPOOL -typeset -i i=0 -while ((i < 10)); do +typeset -a names=() +typeset -a values=() + +# Too long property name (256 bytes) +names+=("$(awk 'BEGIN { printf "x:"; while (c++ < 254) printf "a" }')") +values+=("too-long-property-name") +# Too long property value (8K) +names+=("too:long:property:value") +values+=("$(awk 'BEGIN { while (c++ < 8192) printf "A" }')") +# Invalid property names +for i in {1..10}; do typeset -i len ((len = RANDOM % 32)) - typeset user_prop=$(invalid_user_property $len) + names+=("$(invalid_user_property $len)") ((len = RANDOM % 512)) - typeset value=$(user_property_value $len) + values+=("$(user_property_value $len)") +done + +typeset -i i=0 +while ((i < ${#names[@]})); do + typeset name="${names[$i]}" + typeset value="${values[$i]}" for dtst in $TESTPOOL $TESTPOOL/$TESTFS $TESTPOOL/$TESTVOL ; do - log_mustnot zfs set $user_prop=$value $dtst - log_mustnot check_user_prop $dtst \"$user_prop\" \"$value\" + log_mustnot zfs set $name=$value $dtst + log_mustnot check_user_prop $dtst \"$name\" \"$value\" done ((i += 1))