Home - Waterfall Grid T-Grid Console Builders Recent Builds Buildslaves Changesources - JSON API - About

Console View


Tags: Architectures Platforms default
Legend:   Passed Failed Warnings Failed Again Running Exception Offline No data

Architectures Platforms default
szubersk
initramfs: use `mount.zfs` instead of `mount`

A followup to d7a67402a85252e163aa8a9b69e7eda499db8c61

For `mount -t zfs -o opts ds mp` command line
some implementations of `mount(8)`, e. g. Busybox in Debian
work as follows:

```
newfstatat(AT_FDCWD, "ds", 0x7fff826f4ab0, 0) = -1
mount("ds", "mp", "zfs", MS_SILENT, NULL) = 0
```

The logic above skips completely `mount.zfs` and prevents us
from reading filesystem properties and applying mount options.

For comparison, the coreutils `mount(8)` implementation does:

```
openat(AT_FDCWD, "/proc/filesystems", O_RDONLY|O_CLOEXEC) = 3
// figure out that zfs is a `nodev` filesystem and look for a helper
newfstatat(AT_FDCWD, "/sbin/mount.zfs" ...) = 0
execve("/sbin/mount.zfs" ...) = 0
```

Using `mount.zfs` in initramfs would help circumvent deficiencies
of some of `mount(8)` implementations. `mount -t zfs` translates
to `mount.zfs` invocation, except for cases when explicitly disabled
by `-i`.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: szubersk <szuberskidamian@gmail.com>
Closes #13305
(cherry picked from commit 35d81a75a8c13e011e19fd12cf553d9c5849386e)

Pull-request: #13996 part 1/1
Chris Lindee
zcp: Support nvlist arrays as arguments

Support passing a list of more complicated data structures to channel
programs.  This can provide a mechanism for passing a list of varying
content structure (or even types).

Signed-off-by: Chris Lindee <chris.lindee+github@gmail.com>
Closes: #13988

Pull-request: #13988 part 8/8
Chris Lindee
zcp: Support nvlist arrays as arguments

Support passing a list of more complicated data structures to channel
programs.  This can provide a mechanism for passing a list of varying
content structure (or even types).

Signed-off-by: Chris Lindee <chris.lindee+github@gmail.com>

Pull-request: #13988 part 8/8
  • Debian 10 arm64 (BUILD): cloning zfs -  stdio
  • Kernel.org Built-in x86_64 (BUILD): cloning zfs -  stdio
Chris Lindee
cstyle: Allow URLs in C++ comments

If a C++ comment contained a URL, the `://` part of the URL would
trigger an error because there was no trailing blank, but trailing
blanks make for an invalid URL.  Modify the check to ignore text
within the C++ comment.

Signed-off-by: Chris Lindee <chris.lindee+github@gmail.com>
Closes: #13987

Pull-request: #13987 part 1/1
Chris Lindee
cstyle: Allow URLs in C++ comments

If a C++ comment contained a URL, the `://` part of the URL would
trigger an error because there was no trailing blank, but trailing
blanks make for an invalid URL.  Modify the check to ignore text
within the C++ comment.

Signed-off-by: Chris Lindee <chris.lindee+github@gmail.com>

Pull-request: #13987 part 1/1
  • Kernel.org Built-in x86_64 (BUILD): cloning zfs -  stdio
Richard Yao
Cleanup: Address Clang's static analyzer's unused code complaints

These were categorized as the following:

* Dead assignment 23

* Dead increment 4

* Dead initialization 6

* Dead nested assignment 18

Most of these are harmless, but since actual issues can hide among them,
we correct them.

That said, there were a few return values that were being ignored that
appeared to merit some correction:

* `destroy_callback()` in `cmd/zfs/zfs_main.c` ignored the error from
  `destroy_batched()`. We handle it by returning -1 if there is an
  error.

* `zfs_do_upgrade()` in `cmd/zfs/zfs_main.c` ignored the error from
  `zfs_for_each()`. We handle it by doing a binary OR of the error
  value from the subsequent `zfs_for_each()` call to the existing
  value. This is how errors are mostly handled inside `zfs_for_each()`.
  The error value here is passed to exit from the zfs command, so doing
  a binary or on it is better than what we did previously.

* `get_zap_prop()` in `module/zfs/zcp_get.c` ignored the error from
  `dsl_prop_get_ds()` when the property is not of type string. We
  return an error when it does. There is a small concern that the
  `zfs_get_temporary_prop()` call would handle things, but in the case
  that it does not, we would be pushing an uninitialized numval onto
  the lua stack. It is expected that `dsl_prop_get_ds()` will succeed
  anytime that `zfs_get_temporary_prop()` does, so that not giving it a
  chance to fix things is not a problem.

* `draid_merge_impl()` in `tests/zfs-tests/cmd/draid.c` used
  `nvlist_add_nvlist()` twice in ways in which errors are expected to
  be impossible, so we switch to `fnvlist_add_nvlist()`.

A few notable ones did not merit use of the return value, so we
suppressed it with `(void)`:

* `write_free_diffs()` in `lib/libzfs/libzfs_diff.c` ignored the error
  value from `describe_free()`. A look through the commit history
  revealed that this was intentional.

* `arc_evict_hdr()` in `module/zfs/arc.c` did not need to use the
  returned handle from `arc_hdr_realloc()` because it is already
  referenced in lists.

* `spa_vdev_detach()` in `module/zfs/spa.c` has a comment explicitly
  saying not to use the error from `vdev_label_init()` because whatever
  causes the error could be the reason why a detach is being done.

Unfortunately, I am not presently able to analyze the kernel modules
with Clang's static analyzer, so I could have missed some cases of this.
In cases where reports were present in code that is duplicated between
Linux and FreeBSD, I made a conscious effort to fix the FreeBSD version
too.

After this commit is merged, regressions like dee8934 should become
extremely obvious with Clang's static analyzer since a regression would
appear in the results as the only instance of unused code. That assumes
that Coverity does not catch the issue first.

My local branch with fixes from all of my outstanding non-draft pull
requests shows 118 reports from Clang's static anlayzer after this
patch. That is down by 51 from 169.

Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>

Pull-request: #13986 part 1/1
Richard Yao
Cleanup: Address Clang's static analyzer's unused code complaints

These were categorized as the following:

* Dead assignment 23

* Dead increment 4

* Dead initialization 6

* Dead nested assignment 18

Most of these are harmless, but since actual issues can hide among them,
we correct them.

That said, there were a few return values that were being ignored that
appeared to merit some correction:

* `destroy_callback()` in `cmd/zfs/zfs_main.c` ignored the error from
  `destroy_batched()`. We handle it by returning -1 if there is an
  error.

* `zfs_do_upgrade()` in `cmd/zfs/zfs_main.c` ignored the error from
  `zfs_for_each()`. We handle it by doing a binary OR of the error
  value from the subsequent `zfs_for_each()` call to the existing
  value. This is how errors are mostly handled inside `zfs_for_each()`.

* `get_zap_prop()` in `module/zfs/zcp_get.c` ignored the
  `dsl_prop_get_ds()` when the property is not of type string. We
  return an error if it does. There is a small concern that the
  `zfs_get_temporary_prop()` call would handle things, but in the case
  that it does not, we would be pushing an uninitialized numval onto
  the lua stack. It is expected that `dsl_prop_get_ds()` will succeed
  anytime that `zfs_get_temporary_prop()` does, so that not giving it a
  chance to fix things is not a problem.

* `draid_merge_impl()` in `tests/zfs-tests/cmd/draid.c` used
  `nvlist_add_nvlist()` twice in ways in which errors are expected to
  be impossible, so we switch to `fnvlist_add_nvlist()`.

A few notable ones did not merit use of the return value, so we
suppressed it with `(void)`:

* `write_free_diffs()` in `lib/libzfs/libzfs_diff.c` ignored the error
  value from `describe_free()`. A look through the commit history
  revealed that this was intentional.

* `arc_evict_hdr()` in `module/zfs/arc.c` did not need to use the
  returned handle from `arc_hdr_realloc()` because it is already
  referenced in lists.

* `spa_vdev_detach()` in `module/zfs/spa.c` has a comment explicitly
  saying not to use the error from `vdev_label_init()` because whatever
  causes the error could be the reason why a detach is being done.

Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>

Pull-request: #13986 part 1/1
Richard Yao
Cleanup: Address Clang's static analyzer's unused code complaints

These were categorized as the following:

* Dead assignment 23

* Dead increment 4

* Dead initialization 6

* Dead nested assignment 18

Most of these are harmless, but since actual issues can hide among them,
we correct them.

That said, there were a few return values that were being ignored that
appeared to merit some correction:

* `destroy_callback()` in `cmd/zfs/zfs_main.c` ignored the error from
  `destroy_batched()`. We handle it by returning -1 if there is an
  error.

* `zfs_do_upgrade()` in `cmd/zfs/zfs_main.c` ignored the error from
  `zfs_for_each()`. We handle it by doing a binary OR of the error
  value from the subsequent `zfs_for_each()` call to the existing
  value. This is how errors are mostly handled inside `zfs_for_each()`.

* `get_zap_prop()` in `module/zfs/zcp_get.c` ignored the
  `dsl_prop_get_ds()` when the property is not of type string. We
  return an error if it does. There is a small concern that the
  `zfs_get_temporary_prop()` call would handle things, but in the case
  that it does not, we would be pushing an uninitialized numval onto
  the lua stack. It is expected that `dsl_prop_get_ds()` will succeed
  anytime that `zfs_get_temporary_prop()` does, so that not giving it a
  chance to fix things is not a problem.

* `draid_merge_impl()` in `tests/zfs-tests/cmd/draid.c` used
  `nvlist_add_nvlist()` twice in ways in which errors are expected to
  be impossible, so we switch to `fnvlist_add_nvlist()`.

A few notable ones did not merit use of the return value, so we
suppressed it with `(void)`:

* `write_free_diffs()` in `lib/libzfs/libzfs_diff.c` ignored the error
  value from `describe_free()`. A look through the commit history
  revealed that this was intentional.

* `arc_evict_hdr()` in `module/zfs/arc.c` did not need to use the
  returned handle from `arc_hdr_realloc()` because it is already
  referenced in lists.

* `spa_vdev_detach()` in `module/zfs/spa.c` has a comment explicitly
  saying not to use the error from `vdev_label_init()` because whatever
  causes the error could be the reason why a detach is being done.

Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>

Pull-request: #13986 part 1/1
Richard Yao
Cleanup: Address Clang's static analyzer's unused code complaints

These were categorized as the following:

* Dead assignment 23

* Dead increment 4

* Dead initialization 6

* Dead nested assignment 18

Most of these are harmless, but since actual issues can hide among them,
we correct them.

That said, there were a few return values that were being ignored that
appeared to merit some correction:

* `destroy_callback()` in `cmd/zfs/zfs_main.c` ignored the error from
  `destroy_batched()`. We handle it by returning -1 if there is an
  error.

* `zfs_do_upgrade()` in `cmd/zfs/zfs_main.c` ignored the error from
  `zfs_for_each()`. We handle it by doing a binary OR of the error
  value from the subsequent `zfs_for_each()` call to the existing
  value. This is how errors are mostly handled inside `zfs_for_each()`.

* `get_zap_prop()` in `module/zfs/zcp_get.c` ignored the
  `dsl_prop_get_ds()` when the property is not of type string. We
  return an error if it does. There is a small concern that the
  `zfs_get_temporary_prop()` call would handle things, but in the case
  that it does not, we would be pushing an uninitialized numval onto
  the lua stack. It is expected that `dsl_prop_get_ds()` will succeed
  anytime that `zfs_get_temporary_prop()` does, so that not giving it a
  chance to fix things is not a problem.

* `draid_merge_impl()` in `tests/zfs-tests/cmd/draid.c` used
  `nvlist_add_nvlist()` twice in ways in which errors are expected to
  be impossible, so we switch to `fnvlist_add_nvlist()`.

A few notable ones did not merit use of the return value, so we
suppressed it with `(void)`:

* `write_free_diffs()` in `lib/libzfs/libzfs_diff.c` ignored the error
  value from `describe_free()`. A look through the commit history
  revealed that this was intentional.

* `arc_evict_hdr()` in `module/zfs/arc.c` did not need to use the
  returned handle from `arc_hdr_realloc()` because it is already
  referenced in lists.

* `spa_vdev_detach()` in `module/zfs/spa.c` has a comment explicitly
  saying not to use the error from `vdev_label_init()` because whatever
  causes the error could be the reason why a detach is being done.

Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>

Pull-request: #13986 part 1/1
Richard Yao
Cleanup: Address Clang's static analyzer's unused code complaints

These were categorized as the following:

* Dead assignment 23

* Dead increment 4

* Dead initialization 6

* Dead nested assignment 18

Most of these are harmless, but since actual issues can hide among them,
we correct them.

That said, there were a few return values that were being ignored that
appeared to merit some correction:

* `destroy_callback()` in `cmd/zfs/zfs_main.c` ignored the error from
  `destroy_batched()`. We handle it by returning -1 if there is an
  error.

* `zfs_do_upgrade()` in `cmd/zfs/zfs_main.c` ignored the error from
  `zfs_for_each()`. We handle it by doing a binary OR of the error
  value from the subsequent `zfs_for_each()` call to the existing
  value. This is how errors are mostly handled inside `zfs_for_each()`.

* `get_zap_prop()` in `module/zfs/zcp_get.c` ignored the
  `dsl_prop_get_ds()` when the property is not of type string. We
  return an error if it does. There is a small concern that the
  `zfs_get_temporary_prop()` call would handle things, but in the case
  that it does not, we would be pushing an uninitialized numval onto
  the lua stack. It is expected that `dsl_prop_get_ds()` will succeed
  anytime that `zfs_get_temporary_prop()` does, so that not giving it a
  chance to fix things is not a problem.

* `draid_merge_impl()` in `tests/zfs-tests/cmd/draid.c` used
  `nvlist_add_nvlist()` twice in ways in which errors are expected to
  be impossible, so we switch to `fnvlist_add_nvlist()`.

A few notable ones did not merit use of the return value, so we
suppressed it with `(void)`:

* `write_free_diffs()` in `lib/libzfs/libzfs_diff.c` ignored the error
  value from `describe_free()`. A look through the commit history
  revealed that this was intentional.

* `arc_evict_hdr()` in `module/zfs/arc.c` did not need to use the
  returned handle from `arc_hdr_realloc()` because it is already
  referenced in lists.

* `spa_vdev_detach()` in `module/zfs/spa.c` has a comment explicitly
  saying not to use the error from `vdev_label_init()` because whatever
  causes the error could be the reason why a detach is being done.

Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>

Pull-request: #13986 part 1/1
Gionatan Danti
Remove ambiguity on demand vs prefetch stats

arc_summary currently list prefetch stats as "demand prefetch"
However, a hit/miss can be due to demand or prefetch, not both.
To remove any confusion, this patch removes the "Demand" word
from the affected lines.

Signed-off-by: Gionatan Danti <g.danti@assyoma.it>

Pull-request: #13985 part 2/2
Gionatan Danti
Signed-off-by: Gionatan Danti <g.danti@assyoma.it>

Pull-request: #13985 part 1/2
Gionatan Danti
Remove ambiguity on demand vs prefetch stats

Pull-request: #13985 part 2/2
Gionatan Danti
Signed-off-by: Gionatan Danti <g.danti@assyoma.it>

Pull-request: #13985 part 1/2
Andrew Innes
Replace 'long' with os defined ZFS_MODULE_LONG for LLP64 on windows

Unortunately, Windows defines 'long' as 32-bit even on x64
compiles. We create two new macros ZFS_MODULE_LONG
and ZFS_MODULE_ULONG. These two will be 'long' on Unix, and
let the toolchain handle the size of it.

On Windows the two macros are defined as 'int64_t'/'uint64_t'.

Signed-off-by: Andrew Innes <andrew.c12@gmail.com>
Co-Authored-By: Jorgen Lundman <lundman@lundman.net>

Pull-request: #13984 part 1/1
Andrew Innes
Replace 'long' with os defined ZFS_MODULE_LONG to support LLP64 used by windows

Unortunately, Windows defines 'long' as 32-bit even on x64
compiles. We create two new macros ZFS_MODULE_LONG
and ZFS_MODULE_ULONG. These two will be 'long' on Unix, and
let the toolchain handle the size of it.

On Windows the two macros are defined as 'int64_t'/'uint64_t'.

Signed-off-by: Andrew Innes <andrew.c12@gmail.com>
Co-Authored-By: Jorgen Lundman <lundman@lundman.net>

Pull-request: #13984 part 1/1
GitHub
Fixed order of code

Signed-off-by: Andrew Innes <andrew.c12@gmail.com>

Pull-request: #13984 part 2/2
Andrew Innes
Upstream: Replace ZFS_MODULE_PARAM 'long' usage

Unortunately, Windows defines 'long' as 32-bit even on x64
compiles. We create two new macros ZFS_MODULE_LONG
and ZFS_MODULE_ULONG. These two will be 'long' on Unix, and
let the toolchain handle the size of it.

On Windows the two macros are defined as 'int64_t'/'uint64_t'.

Signed-off-by: Andrew Innes <andrew.c12@gmail.com>
Co-Authored-By: Jorgen Lundman <lundman@lundman.net>

Pull-request: #13984 part 1/1
Richard Yao
Linux: Upgrade random_get_pseudo_bytes() to xoshiro256++ 1.0

The motivation for upgrading our PRNG is the recent buildbot failures in
the ZTS' tests/functional/fault/decompress_fault test. The probability
of a failure in that test is 0.8^256, which is ~1.6e-25 out of 1, yet we
have observed multiple test failures in it. This suggests a problem with
our random number generation.

The xorshift128+ generator that we were using has been replaced by newer
generators that have "better statistical properties". After doing some
reading, it turns out that these generators have "low linear complexity
of the lowest bits", which could explain the ZTS test failures.

We do two things to try to fix this:

1. We upgrade from xorshift128+ to xoshiro256++ 1.0.

2. We tweak random_get_pseudo_bytes() to copy the higher order
  bytes first.

It is hoped that this will fix the test failures in
tests/functional/fault/decompress_fault, although I have not done
simulations. I am skeptical that any simulations I do on a PRNG with a
period of 2^256 - 1 would be meaningful.

Since we have raised the minimum kernel version to 3.10 since this was
first implemented, we have the option of using the Linux kernel's
get_random_int(). However, I am not currently prepared to do performance
tests to ensure that this would not be a regression (for the time
being), so we opt for upgrading our PRNG to a newer one from Sebastiano
Vigna.

Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>

Pull-request: #13983 part 1/1
Richard Yao
Linux: Upgrade random_get_pseudo_bytes() to xoshiro256++ 1.0

The motivation for upgrading our PRNG is the recent buildbot failures in
the ZTS' tests/functional/fault/decompress_fault test. The probability
of a failure in that test is 0.8^256, which is ~1.6e-25 out of 1, yet we
have observed multiple test failures in it. This suggests a problem with
our random number generation.

The xorshift128+ generator that we were using has been replaced by newer
generators that have "better statistical properties". After doing some
reading, it turns out that these generators have "low linear complexity
of the lowest bits", which could explains the ZTS test failures.

We do two things to try to fix this:

1. We upgrade from xorshift128+ to xoshiro256++ 1.0.

2. We tweak random_get_pseudo_bytes() to copy the higher order
  bytes first.

It is hoped that this will fix the test failures in
tests/functional/fault/decompress_fault, although I have not done
simulations. I am skeptical that any simulations I do on a PRNG with a
period of 2^256 - 1 would be meaningful.

Since we have raised the minimum kernel version to 3.10 since this was
first implemented, we have the option of using the Linux kernel's
get_random_int(). However, I am not currently prepared to do performance
tests to ensure that this would not be a regression (for the time
being), so we opt for upgrading our PRNG to a newer one from Sebastiano
Vigna.

Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>

Pull-request: #13983 part 1/1
  • Debian 10 arm64 (BUILD): cloning zfs -  stdio
  • Kernel.org Built-in x86_64 (BUILD): cloning zfs -  stdio
Richard Yao
Linux: Upgrade random_get_pseudo_bytes() to xoshiro256++ 1.0

The motivation for upgrading our PRNG is the recent buildbot failures in
the ZTS' tests/functional/fault/decompress_fault test. The probability
of a failure in that test is 0.8^256, which is ~1.6e-25 out of 1, yet we
have observed multiple test failures in it. This suggests a problem with
our random number generation.

The xorshift128+ generator that we were using has been replaced by newer
generators that have "better statistical properties". Some of the newer
random number generators are used in java.util.random as of Java 17.
After examining the choices, I have decided to replace the current PRNG
with xoshiro256++ 1.0.

It is hoped that this will fix the test failures in
tests/functional/fault/decompress_fault, although I have not done
simulations. I am skeptical that any simulations I do on a PRNG with a
period of 2^256 - 1 would be meaningful.

Since we have raised the minimum kernel version to 3.10 since this was
first implemented, we have the option of using the Linux kernel's
get_random_int(). However, I am not currently prepared to do performance
tests to ensure that this would not be a regression (for the time
being), so we opt for upgrading our PRNG to a newer one from Sebastiano
Vigna.

Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>

Pull-request: #13983 part 1/1
Richard Yao
Linux: Upgrade random_get_pseudo_bytes() to xoshiro256++ 1.0

The motivation for upgrading our PRNG is the recent buildbot failures in
the ZTS' tests/functional/fault/decompress_fault test. The probability
of a failure in that test is 0.8^256, which is ~1.6e-25 out of 1, yet we
have observed multiple test failures in it. This suggests a problem with
our random number generation.

The xorshift128+ generator that we were using has been replaced by newer
generators that have "better statistical properties". Some of the newer
random number generators are used in java.util.random as of Java 17.
After examining the choices, I have decided to replace the current PRNG
with xoshiro256++ 1.0.

It is hoped that this will fix the test failures in
tests/functional/fault/decompress_fault, although I have not done
simulations. I am skeptical that any simulations I do on a PRNG with a
period of 2^256 - 1 would be meaningful.

Since we have raised the minimum kernel version to 3.10 since this was
first implemented, we have the option of using the Linux kernel's
get_random_int(). However, I am not currently prepared to do performance
tests to ensure that this would not be a regression (for the time
being), so we opt for upgrading our PRNG to a newer one from Sebastiano
Vigna.

Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>

Pull-request: #13983 part 1/1
Richard Yao
zio_crypt functions should use random_get_bytes()

We should not use random_get_pseudo_bytes() to generate random numbers
for encryption because we only guarantee cryptographic quality
randomness from random_get_bytes().

In practice, FreeBSD implements random_get_pseudo_bytes() using
cryptographic grade randomness, so we only use a weak random number
generator on Linux.

I implemented the Linux SPL random_get_pseudo_bytes() random number
generator in 0b43696e6676391e5bee8ba49e76e599bac1d89d. I never intended
for it to be used by encryption. Unfortunately, I did not participate in
the review process for the encryption support, so I am only seeing this
now.

I am not a cryptoanalyst, so I cannot say whether this caused a
weakness. However, I can say that we should switch to using a
cryptographic grade random number generator since that is best practice.

Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>

Pull-request: #13981 part 1/1
Richard Yao
zio_crypt functions should use random_get_bytes()

We should not use random_get_pseudo_bytes() to generate random numbers
for encryption because we only guarantee cryptographic quality
randomness from random_get_bytes().

In practice, FreeBSD implements random_get_pseudo_bytes() using
cryptographic grade randomness, so we only use a weak random number
generator on Linux.

I implemented the Linux SPL random_get_pseudo_bytes() random number
generator in 0b43696e6676391e5bee8ba49e76e599bac1d89d. I never intended
for it to be used by encryption. Unfortunately, I did not participate in
the review process for the encryption support, so I am only seeing this
now.

When I wrote the Linux SPL random_get_pseudo_bytes() function, I took
care to make the probability of number reuse extremely low.  The
pseudo-random number generator generates 128-bit numbers with a period
of 2^128 - 1. It is seeded using a cryptographic grade random function.
Each successive CPU is given a starting position 2^64 apart.  Number
reuse from generating 2^64 numbers on one CPU at a impossibly fast rate
of 1 per nanosecond to reach the region of another CPU would take over
584 years.

Had we not truncated the 16-byte random numbers to 12 bytes for AES'
4-byte rolling counter, I would have been able to conclude that systems
that do not reboot would never have the same numbers and then calculate
an extremely low probability for number reuse if the system reboots.
However, the truncation of the numbers for AES' rolling counter means
that method of analysis does not work here.

While I have a degree in Applied Mathematics and Statistics, I am not a
cryptoanalyst, so I cannot say whether this caused a weakness. However,
I can say that we should switch to using a cryptographic grade random
number generator since that is best practice.

Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>

Pull-request: #13981 part 1/1
Richard Yao
zio_crypt functions should use random_get_bytes()

We should not use random_get_pseudo_bytes() to generate random numbers
for encryption because we only guarantee cryptographic quality
randomness from random_get_bytes().

In practice, FreeBSD implements random_get_pseudo_bytes() using
cryptographic grade randomness, so we only use a weak random number
generator on Linux.

I implemented the Linux SPL random_get_pseudo_bytes() random number
generator in 0b43696e6676391e5bee8ba49e76e599bac1d89d. I never intended
for it to be used by encryption. Unfortunately, I did not participate in
the review process for the encryption support, so I am only seeing this
now.

When I wrote the Linux SPL random_get_pseudo_bytes() function, I took
care to make the probability of number reuse extremely low.  The
pseudo-random number generator generates 128-bit numbers with a period
of 2^128 - 1. It is seeded using a cryptographic grade random function.
Each successive CPU is given a starting position 2^64 apart.  Number
reuse from generating 2^64 numbers on one CPU at a impossibly fast rate
of 1 per nanosecond to reach the region of another CPU would take over
584 years.

Had we not truncated the 16-byte random numbers for AES' 4-byte rolling
counter, I would have been able to conclude that systems that do not
reboot would never have the same numbers and then calculate an extremely
low probability for number reuse if the system reboots. However, the
truncation of the numbers for AES' rolling counter means that method of
analysis does not work here.

While I have a degree in Applied Mathematics and Statistics, I am not a
cryptoanalyst, so I cannot say whether this caused a weakness. However,
I can say that we should switch to using a cryptographic grade random
number generator since that is best practice.

Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>

Pull-request: #13981 part 1/1
  • Debian 8 ppc64 (BUILD): cloning zfs -  stdio
  • Ubuntu 18.04 i386 (BUILD): cloning zfs -  stdio
Richard Yao
zio_crypt functions should use random_get_bytes()

We should not use random_get_pseudo_bytes() to generate random numbers
for encryption because we only guarantee cryptographic quality
randomness from random_get_bytes().

In practice, FreeBSD implements random_get_pseudo_bytes() using
cryptographic grade randomness, so we only use a weak random number
generator on Linux.

I implemented the Linux SPL random_get_pseudo_bytes() random number
generator in 0b43696e6676391e5bee8ba49e76e599bac1d89d. I never intended
for it to be used by encryption. Unfortunately, I did not participate in
the review process for the encryption support, so I am only seeing this
now.

When I wrote the Linux SPL random_get_pseudo_bytes() function, I took
care to make the probability of number reuse extremely low.  The
pseudo-random number generator generates 128-bit numbers with a period
of 2^128 - 1. It is seeded using a cryptographic grade random function.
Each successive CPU is given a starting position 2^64 apart.  Number
reuse from generating 2^64 numbers on one CPU at a impossibly fast rate
of 1 per nanosecond to reach the region of another CPU would take over
584 years.

Had we not truncated 4-bytes for AES' rolling counter, I would have been
able to conclude that systems that do not reboot would never have the
same numbers and then calculate an extremely low probability for number
reuse if the system reboots. However, the truncation of the numbers for
AES' rolling counter means that method of analysis does not work here.

While I have a degree in Applied Mathematics and Statistics, I am not a
cryptoanalyst, so I cannot say whether this caused a weakness. However,
I can say that we should switch to using a cryptographic grade random
number generator since that is best practice.

Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>

Pull-request: #13981 part 1/1
  • Debian 10 arm64 (BUILD): cloning zfs -  stdio
Finix
Avoid unnecessary metaslab_check_free calling

Because zio_free_sync has called metaslab_check_free, zio_free could
check it only for GANG|dedup etc.

Signed-off-by: Finix1979 <yancw@info2soft.com>

Pull-request: #13977 part 1/1
Tino Reichardt
Fix declarations of non-global variables

This patch inserts the `static` keyword to non-global variables,
which where found by the analysis tool smatch.

Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de>

Pull-request: #13970 part 1/1
Tino Reichardt
Fix declarations of non-global variables

This patch inserts the `static` keyword to non-global variables,
which where found by the analysis tool smatch.

Signed-Off-by: Tino Reichardt milky-zfs@mcmilk.de

Pull-request: #13970 part 1/1
Tino Reichardt
Fix declarations of non-global variables

This patch inserts the `static` keyword to non-global variables,
which where found by the analysis smatch.

Signed-off-by: Tino Reichardt milky-zfs@mcmilk.de

Pull-request: #13970 part 1/1
Umer Saleem
Add membar_sync abi change

It appears membar_sync was not present in libzfs.abi with other
membar_* functions. This commit updates libzfs.abi for membar_sync.

Signed-off-by: Umer Saleem <usaleem@ixsystems.com>

Pull-request: #13969 part 2/2
Umer Saleem
Expose libzutil error info in libpc_handle_t

In libzutil, for zpool_search_import and zpool_find_config, we use
libpc_handle_t internally, which does not maintain error code and it is
not exposed in the interface. Due to this, the error information is not
propagated to the caller. Instead, an error message is printed on
stderr.

This commit adds lpc_error field in libpc_handle_t and exposes it in
the interface, which can be used by the users of libzutil to get the
appropriate error information and handle it accordingly.

Users of the API can also control if they want to print the error
message on stderr.

Signed-off-by: Umer Saleem <usaleem@ixsystems.com>

Pull-request: #13969 part 1/2
Andrew Innes
Revert "Upstream: Replace ZFS_MODULE_PARAM 'long' usage"

This reverts commit 0cc906678509d9b56e5481e81757c77660152b9e.

Co-Authored-By: Jorgen Lundman <lundman@lundman.net>

Pull-request: #13960 part 2/2
Andrew Innes
Upstream: Replace ZFS_MODULE_PARAM 'long' usage

Unortunately, Windows defines 'long' as 32-bit even on x64
compiles. We create two new macros ZFS_MODULE_LONG
and ZFS_MODULE_ULONG. These two will be 'long' on Unix, and
let the toolchain handle the size of it.

On Windows the two macros are defined as 'int64_t'/'uint64_t'.

Clean up source to get Linux builds working (#119)

* Move perfmon functions to windows

from zfs_ioctl.h to zfs_ioctl_os.c

* remove static for zfs_dirty_data_sync_percent

* uint64_t to ZFS_MODULE_ULONG

* add ifdef _WIN32 to zvol.c

* define posix_memalign_free for other os

* add vdev_file_t for other os

* remove duplicate check_file

* Create build_for_wsl.yaml

* move functions to zvol_os.c

and add them to the header zvol_impl.h
in answer to this discussion
https://github.com/openzfsonwindows/openzfs/pull/119#discussion_r941958584

* Fix code formatting

in reference to https://github.com/openzfsonwindows/openzfs/pull/119#discussion_r941959208

* revert the removal of static in dsl_pool.c

* remove zfs_dirty_data_sync_percent in dsl_pool.h

* make zvol_find_by_name not static

* changed workflow name

* cstyle zpool_vdev_os.c

Fix cstyle complaints (#125)

* clean cstyle

* squash

* squash

* Update zfs_vnops_windows.c

* Update zfs_windows_zvol_wmi.c

* squash

* Additional cstyle fixes

* Update sysctl_os.c

Upstream: Additional MODULE ULONG

Bring per_txg_dirty_frees_percent back to 30

The current value causes significant artificial slowdown during mass
parallel file removal, which can be observed both on FreeBSD and Linux
when running real workloads.

Sample results from Linux doing make -j 96 clean after an allyesconfig
modules build:

before: 4.14s user 6.79s system 48% cpu 22.631 total
after: 4.17s user 6.44s system 153% cpu 6.927 total

FreeBSD results in the ticket.

Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Reviewed-by: George Melikov <mail@gmelikov.ru>
Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
Closes #13932
Closes #13938

Co-Authored-By: Jorgen Lundman <lundman@lundman.net>

Pull-request: #13960 part 1/1
Andrew Innes
Windows: use common zoneid_t

Signed-off-by: Andrew Innes <andrew.c12@gmail.com>

Pull-request: #13960 part 286/286
  • Debian 10 arm64 (BUILD): cloning zfs -  stdio
  • Kernel.org Built-in x86_64 (BUILD): cloning zfs -  stdio
Jorgen Lundman
Remove AARCH64 from M1 compiles.

Sadly, no standard NEON on M1, at least, not yet.

Signed-off-by: Jorgen Lundman <lundman@lundman.net>

Pull-request: #12110 part 23/23
Jorgen Lundman
Make sure kcf_mech_tabs is set to zero.

Definitely unsure what is going on here. The address for the tabs
keep moving, as in:

class = 2;
printf("kcf_mech_tabs_tab[class].met_tab is %p and class is %d\n",
  kcf_mech_tabs_tab[class].met_tab, class);
printf("kcf_mech_tabs_tab[  2  ].met_tab is %p and class is %d\n",
  kcf_mech_tabs_tab[2].met_tab, class);

kcf_mech_tabs_tab[class].met_tab is 0xffffff7f8b933d90 and class is 2
kcf_mech_tabs_tab[  2  ].met_tab is 0xffffff7f8b930d90 and class is 2
..................................................^

which is most peculiar, so the memory that the 3 tabs set up, is
full of garbage, no empty slot is found and it fails to add the
ciphers, digests and macs.

If we set a struct to nothing, or to = { 0 }; like:

static kcf_mech_entry_t kcf_digest_mechs_tab[KCF_MAXDIGEST] = { 0 };

The original kcf_mech_tabs_tab is all zero, but when we start to use
it and it shifts by 0x30000 it is garbage. This is true even if we
set it to:

    = {{{0}, 0, 0, {0}}}

But for some bizarre reason, if we set the first element to something
not-zero, like:
  = {
      { "NOTUSED", 0, 0, {0}},
      { {0}, 0, 0, {0}}
    }

Then it works, yes the first entry is busy, but [1] and onwards is
finally set to zero, and the pointer does not slide by 0x30000.

I wish I knew why.

Signed-off-by: Jorgen Lundman <lundman@lundman.net>

Pull-request: #12110 part 19/19
Jorgen Lundman
Make sure kcf_mech_tabs is set to zero.

Definitely unsure what is going on here. The address for the tabs
keep moving, as in:

class = 2;
    printf("kcf_mech_tabs_tab[class].met_tab is %p and class is %d\n",
        kcf_mech_tabs_tab[class].met_tab, class);
    printf("kcf_mech_tabs_tab[  2  ].met_tab is %p and class is %d\n",
        kcf_mech_tabs_tab[2].met_tab, class);

kcf_mech_tabs_tab[class].met_tab is 0xffffff7f8b933d90 and class is 2
kcf_mech_tabs_tab[  2  ].met_tab is 0xffffff7f8b930d90 and class is 2
..................................................^

which is most peculiar, so the memory that the 3 tabs set up, is
full of garbage, no empty slot is found and it fails to add the
ciphers, digests and macs.

If we set a struct to nothing, or to = { 0 }; like:

static kcf_mech_entry_t kcf_digest_mechs_tab[KCF_MAXDIGEST] = { 0 };

The original kcf_mech_tabs_tab is all zero, but when we start to use it
and it shifts by 0x30000 it is garbage. This is true even if we
set it to:

    = {{{0}, 0, 0, {0}}}

But for some bizarre reason, if we set the first element to something
not-zero, like:
  = {
      { "NOTUSED", 0, 0, {0}},
      { {0}, 0, 0, {0}}
    }

Then it works, yes the first entry is busy, but [1] and onwards is finally
set to zero, and the pointer does not slide by 0x30000.

I wish I knew why.

Signed-off-by: Jorgen Lundman <lundman@lundman.net>

Pull-request: #12110 part 19/19