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 <mahrens@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: loli10K <ezomori.nozomu@gmail.com>
Closes #8278

diff --git a/module/zfs/zap_leaf.c b/module/zfs/zap_leaf.c
index d168ad29ae3387d472e90e8400ccbbb54dddbe52..b421dd5038353556395b8de646776b46a4bf76a1 100644 (file)
--- 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 fb03d7c9716a9f80d84f8ec8a6cade08e7ba2b73..16b9638787e24e06cee92b51366bbedb5f6ca40b 100755 (executable)
--- 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 6a90665f4b5fdd5d51852ea865d9b7da23453753..3d8c1e7be18d006c0746f875add97d644a24740c 100755 (executable)
--- 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))