Project

General

Profile

Bug #12877

Port OpenZFS #7780 - Add basic zfs ioc input nvpair validation

Added by Jason King about 2 months ago. Updated about 1 month ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
zfs - Zettabyte File System
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

From the OpenZFS commit msg:

We want newer versions of libzfs_core to run against an existing zfs kernel module (i.e. a deferred reboot or module reload after an update).

Programmatically document, via a zfs_ioc_key_t, the valid arguments for the ioc commands that rely on nvpair input arguments (i.e. nonlegacy commands from libzfs_core). Automatically verify the expected pairs before dispatching a command.

This initial phase focuses on the non-legacy ioctls. A follow-on change can address the legacy ioctl input from the zfs_cmd_t.

The zfs_ioc_key_t for zfs_keys_channel_program looks like:

static const zfs_ioc_key_t zfs_keys_channel_program[] = {
       {"program",     DATA_TYPE_STRING,               0},
       {"arg",         DATA_TYPE_UNKNOWN,              0},
       {"sync",        DATA_TYPE_BOOLEAN_VALUE,        ZK_OPTIONAL},
       {"instrlimit",  DATA_TYPE_UINT64,               ZK_OPTIONAL},
       {"memlimit",    DATA_TYPE_UINT64,               ZK_OPTIONAL},
};

Introduce four input errors to identify specific input failures (in addition to generic argument value errors like EINVAL, ERANGE, EBADF, and E2BIG).

  • ZFS_ERR_IOC_CMD_UNAVAIL the ioctl number is not supported by kernel
  • ZFS_ERR_IOC_ARG_UNAVAIL an input argument is not supported by kernel
  • ZFS_ERR_IOC_ARG_REQUIRED a required input argument is missing
  • ZFS_ERR_IOC_ARG_BADTYPE an input argument has an invalid type

Reviewed-by: Matthew Ahrens <>
Reviewed-by: Brian Behlendorf <>
Signed-off-by: Don Brady <>
Closes #7780

While we don't need to be concerned about mismatched kernel module/userland bits, the centralized (and consistent) ioctl validation is useful without regard to mismatched bits, it also simplifies the ioctl processing, and having this will make porting additional OpenZFS features easier.

Additionally, we need part of:

commit 2e0358cbcab49f7be18762e8cb51e642188709e7
Author: Brian Behlendorf <behlendorf1@llnl.gov>
Date:   Fri Dec 13 14:49:33 2013 -0800

    Sync /dev/zfs ioctl ordering

    In order to minimize any future disruption caused by the addition
    and removal /dev/zfs ioctls this patch makes the following changes.

    1) Sync ZoL's ioctl ordering such that it matches Illumos.  For
       historic reasons the ZFS_IOC_DESTROY_SNAPS and ZFS_IOC_POOL_REGUID
       ioctls were out of order.

    2) Move Linux and FreeBSD specific ioctls in to their own reserved
       ranges.  This allows us to preserve the existing ordering when
       new ioctls are added by either Illumos or FreeBSD.  When an
       ioctl is no longer needed it should be retired in place.

    This change alters the ZFS user/kernel ABI so make sure you rebuild
    both your user and kernel modules.  However, it should allow for a
    much stabler interface going forward.

    Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
    Signed-off-by: Ned Bass <bass6@llnl.gov>
    Closes #1973

... and #5704 didn't include the setup/cleanup bits for the functional/libzfs tests since it was just a single test. We will now also need to port those over for the ioctl tests.

Obviously the ioctl resync bit is superfluous, but it includes support for a sparse IOC space which is useful in trying to avoid divergence with OpenZFS.


Related issues

Related to illumos gate - Bug #12895: zfs_onexit_fd_hold fails to release non-zfs fdsClosed

Actions

History

#1

Updated by Jason King about 1 month ago

  • Description updated (diff)
#2

Updated by Jason King about 1 month ago

  • Description updated (diff)
#3

Updated by Electric Monk about 1 month ago

  • Gerrit CR set to 773
#4

Updated by Jason King about 1 month ago

To test, I ran the zfs test suite (along with the fix for #12895 -- the updated tests in this change will trigger an ASSERT failure without the fix for #12895). All of the expected tests pass (including the new ioctl test added by this change). All of the failures have existing tickets filed against them.

#5

Updated by Dan McDonald about 1 month ago

  • Related to Bug #12895: zfs_onexit_fd_hold fails to release non-zfs fds added
#6

Updated by Electric Monk about 1 month ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit 7ac89354c798225fea6296348415955ccd95fb80

commit  7ac89354c798225fea6296348415955ccd95fb80
Author: Don Brady <don.brady@delphix.com>
Date:   2020-07-07T17:55:10.000Z

    12877 Port OpenZFS #7780 - Add basic zfs ioc input nvpair validation
    12895 zfs_onexit_fd_hold fails to release non-zfs fds
    Portions contributed by: Brian Behlendorf <behlendorf1@llnl.gov>
    Portions contributed by: George Wilson <george.wilson@delphix.com>
    Portions contributed by: Simon Klinkert <simon.klinkert@gmail.com>
    Portions contributed by: Jason King <jason.king@joyent.com>
    Reviewed by: Matthew Ahrens <mahrens@delphix.com>
    Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
    Reviewed by: Toomas Soome <tsoome@me.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

Also available in: Atom PDF