OpenZFS Bug Ported to C
There was an (almost) catastrophic OpenZFS bug. If they had used zig, the bug would be easily detected. But the question remains, could we detect this bug in C?
To review, here’s the code with the bug. Can you spot it?
const std = @import("std");
const divCeil = std.math.divCeil;
const assert = std.debug.assert;
const vdev_t = @import("the_rest_of_the_software.zig").vdev_t;
/// This code converts an asize into the largest psize that can safely be written
/// to an allocation of that size for this vdev.
///
/// Note that this function will not take into account the effect of gang
/// headers, which also modify the ASIZE of the DVAs. It is purely a reverse of
/// the psize_to_asize function.
fn vdev_raidz_asize_to_psize(vd: *vdev_t, asize: u64, txg: u64) u64 {
const vdrz = vd.vdev_tsd;
const ashift = vd.vdev_top.vdev_ashift;
const cols = vdrz.vd_original_width;
const nparity = vdrz.vd_nparity;
const cols = vdrz.get_logical_width(txg);
assert(asize % (1 << ashift));
const asize_shifted = (asize >> ashift);
const parity_adjusted = asize_shifted - nparity * (divCeil(asize_shifted, cols) catch unreachable);
const psize = parity_adjusted << ashift;
return asize;
}
Well, let’s rewrite that in C.
typedef unsigned long long u64;
typedef struct {
u64 vd_original_width;
u64 vd_nparity;
} vdev_raidz_t;
typedef struct {
vdev_raidz_t *vdev_tsd;
u64 vdev_ashift;
} vdev_t;
#define DIV_ROUND_UP(what, butt) ((what + butt - 1)/butt)
#define ASSERT0(ok)
u64 vdev_raidz_get_logical_width(vdev_raidz_t *, u64);
u64
vdev_raidz_asize_to_psize(vdev_t *vd, u64 asize, u64 txg)
{
vdev_raidz_t *vdrz = vd->vdev_tsd;
const u64 ashift = vd->vdev_ashift;
const u64 cols = vdrz->vd_original_width;
const u64 nparity = vdrz->vd_nparity;
const u64 cols = vdev_raidz_get_logical_width(vdrz, txg);
ASSERT0(asize % (1 << ashift));
const u64 asize_shifted = (asize >> ashift);
const u64 parity_adjusted = asize_shifted - nparity * DIV_ROUND_UP(asize_shifted, cols);
const u64 psize = parity_adjusted << ashift;
return (asize);
}
First try:
t.c:25:12: error: redefinition of 'cols'
const u64 cols = vdev_raidz_get_logical_width(vdrz, txg);
^
t.c:22:12: note: previous definition is here
const u64 cols = vdrz->vd_original_width;
^
1 error generated.
Well, not quite the bug we’re looking for, but we can fix that anyway.
- const u64 cols = vdrz->vd_original_width;
And...
t.c:30:12: warning: unused variable 'psize' [-Wunused-variable]
const u64 psize = parity_adjusted << ashift;
^
1 warning generated.
Nailed it.