Set ZFS_MAXPROPLEN and ZPOOL_MAXPROPLEN to ZAP_MAXVALUELEN

[0mp]

Motivation and Context

So far, the values of ZFS_MAXPROPLEN and ZPOOL_MAXPROPLEN were equal to
MAXPATHLEN, which is:

• 1024 on FreeBSD and
• 4096 on Linux.
  This wasn't ideal. Some of the surprising outcomes of this implementation are:

1. When creating a pool user property with zpool-set(8), libzfs makes
   sure that the length of the property's value is less than
   ZFS_MAXPROPLEN. However, the ZFS kernel module does not do that.
   Instead, it checks the length against ZAP_MAXVALUELEN. As a result,
   it is possible to create a property the length of which is going to
   be larger than zpool(8) is ready to read.
2. A pool user property created on Linux is too big to be read on
   FreeBSD.

Description

This change sets both ZFS_MAXPROPLEN and ZPOOL_MAXPROPLEN to ZAP_MAXVALUELEN, which is 8192 at the moment.

This PR also fixes some documentation issues in zpoolprops.7 and cleans up user_property tests.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.

How Has This Been Tested?

I've run tests of the following tags: zpool_set, zfs_set, and zfs_clone.

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)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[amotin]

As a result, it is possible to create a property the length of which is going to be larger than zpool(8) is ready to read.

And you are legalizing it with this change if the property set on newer ZFS version is read on older. Though considering already existing discrepancy between FreeBSD and Linux I tend to agree that this is a step forward, just a bit doubtful. On a quick look I haven't found buffer overflows there, but it seems that both zfs_prop_get() and zpool_prop_get() are truncating the string instead of reporting error, that may cause unpredictable behavior.

[rincebrain]

"In theory", we should probably, going forward, mark a feature flag active the first time someone writes one over 1024.

We can't do anything about older systems, but we can ensure older systems know there are dragons if they read things after we noticed this oversight...

[behlendorf]

Certainly a step in the right direction. Technically we probably should have a feature flag for this, but in this case I think we can manage without one and avoid that additional complexity.

@0mp would you mind just rebasing this PR and it'll be good to go.

[0mp]

Certainly a step in the right direction. Technically we probably should have a feature flag for this, but in this case I think we can manage without one and avoid that additional complexity.

@0mp would you mind just rebasing this PR and it'll be good to go.

Sure!