Clang (22.1.8 on the host, 14.0.6 in the Debian VM) now builds the tree
warning-free with --enable-warnings, in-tree and out-of-tree, with -j, and
the full VM battery passes with CC=clang: 12/12. ASan/UBSan builds run the
real tool paths (mkfs, check, tune, resize, rollback save/restore/reject)
with halt_on_error and report zero errors. Documented in tests/vm/RESULTS.md;
the CI gained clang+warnings, out-of-tree and sanitizer jobs.
Fixes found by this work:
- reiserfscore/Makefile.am: reiserfs_err.c and reiserfs_err.h shared one
recipe. With -j, make runs the recipe once per stale target, so two
compile_et invocations race: one deletes reiserfs_err.c ("no input
files") while another job compiles it. Attached the recipe to
reiserfs_err.c only; reiserfs_err.h merely depends on it. Reproduced
with clang+ASan, stable across 5 clang / 3 asan / 3 gcc parallel builds.
- reiserfscore/Makefile.am: the rule read the source-local
reiserfs_err.et, so VPATH builds could not find it. Use $(srcdir)/.
make distcheck now passes.
- reiserfscore/reiserfslib.c: reiserfs_close() did fsync(fs->fs_dev) after
reiserfs_free() released fs -- a 4-byte heap-use-after-free, flagged
exactly at reiserfslib.c:406/407 by ASan (found by clang+ASan; gcc never
caught it because the freed heap usually still holds the old value).
Moved fsync before the free; reiserfs_flush() has already written
journal and dirty buffers at that point and free_buffers() never
writes, so the ordering preserves durability semantics.
- fsck/semantic_check.c, fsck/semantic_rebuild.c: three %K arguments passed
&tmp_deh->deh2_dir_id where a struct reiserfs_key * is expected. It
printed the right values only because deh2_dir_id sits at offset 4 and
the key is read from the pointer as if dir_id sat at offset 0. Build a
proper local key with set_key_dirid()/set_key_objectid() (the same
idiom debugreiserfs/scan.c already uses). Verified -Waddress-of-packed-
member (clang-only) is gone and output is unchanged.
- reiserfscore/hashes.c: four unreachable *(int *)0 = 0 stores (guarded by
len >= 16/12/8/4 tests that cannot hold after the while (len >= 16)
loop, len <= 15). Removed; clang's -Wnull-dereference goes quiet.
Behavior verified: keyed_hash/yura_hash/r5_hash outputs for 16 inputs
from 0 to 17 bytes are byte-identical before and after.
run_in_vm.sh: propagate CC into the guest, force a remount of /mnt/data
(the host rewrites the backing file under the guest), exclude generated
reiserfs_err.[ch] from the source tar. run_tests.sh: report which compiler
the battery actually used.
The VM harness (tests/vm/) builds and exercises the tools on real Debian and
Alpine guests, driving the serial console (no sshd/cloud-init dependency).
Results on Debian 12 (glibc 2.36, gcc 12.2): 12/12 checks pass, 0 warnings
with -Wall -Wextra, including a real 128M -> 256M resize and the rollback
save/restore/reject paths.
Results on Alpine 3.20 (musl 1.2.5): the build cannot start. configure stops
at "reiserfsprogs requires a method to add printf modifiers" because
reiserfscore/prints.c depends on glibc's register_printf_specifier(3) and
<printf.h>, which musl does not provide. This is pre-existing and out of
scope here; measured at 341 use sites across 20+ files, all routed through
reiserfs_warning()/reiserfs_info(). Documented in tests/vm/RESULTS.md.
Fix the one real defect the harness surfaced:
fsck/semantic_check.c: check_check_regular_file() is called with NULL for
new_ih (line 466), so the block that would fill *new_ih is unreachable and
check_semantic_pass() never returns RELOCATED through that path. The
"case RELOCATED:" arm still reads relocated_ih.ih_key, which gcc 12.2
reports as "may be used uninitialized" (-Wmaybe-uninitialized); gcc 16 does
not, because it proves the path dead. Zero-initialise the struct and say why.
Verified: 0 warnings on gcc 12.2 after the change, and the Debian battery
stays at 12/12.
Build: add --enable-warnings (-Wall -Wextra); the tree is now warning-free
with it, so distributions can opt in without risking a surprise -Werror
breakage from a future compiler.
Real bugs fixed (each verified by a reproducer):
- lib/io.c: open_rollback_file() read 28 bytes into char[28] with no NUL
and then strcmp()'d it against a 28-character magic, reading one byte
past the array. The condition was also inverted, so a *valid* rollback
file was rejected -- but only sometimes: the missing NUL made the result
depend on stack garbage (verified: -O0 rejects, -O2 accepts). The whole
--save-rollback/--rollback-changes path was unreliable. Now the buffer is
sized for the terminator, zeroed, and the comparison is `!= 0`.
Verified end to end: save -> reopen -> restore reports "2 blocks
restored, 0 skipped"; a random file is rejected.
- reiserfscore/reiserfslib.c, reiserfscore/journal.c: asprintf() was
called with &fs->fs_file_name / &fs->fs_j_file_name, but those fields
are `char *`, not `char **`. Passing a `char **` where `char *` is
expected wrote through the wrong pointer -- silent memory corruption on
any allocation. Fixed by using the new checked helper.
- reiserfscore/node_formats.c: `code < 0` on an unsigned parameter was
dead code; the guard never fired.
- resize_reiserfs: missing `break` after -j made -j fall into -f, so
"-j dev" silently also forced the operation. Sign-compare on the
count_blocks() ternary and on count + blocks.
- lib/misc.c: strncat with bound == strlen(src) (equivalent to strcat) into
char[256] from a char[256] dirent name -- up to 261 bytes. Replaced with
snprintf plus an explicit fit check.
- lib/misc.c: die() used vsprintf into a fixed 1024-byte buffer.
- strncpy with bound == destination size (mkreiserfs, tune, progbar) never
terminated; replaced with snprintf.
Allocator: drop the homegrown magic-header allocator. It wrote "_mem_begin_"
/ "mem_end" through strcpy inside the allocation, kept the size in an
`int`, and its only safety check (checkmem) was never called -- every call
site is commented out. Replaced with a size_t header that is over-aligned
via max_align_t, so malloc alignment guarantees still hold. expandmem()
now takes the old size from the header instead of a caller-supplied value
that could disagree with it.
Warnings: annotate the deliberate fallthroughs (fsck pass cascade, unit
suffix parsing, percentage parsing), fix the remaining sign-compare and
missing-field-initializer sites, and compute addresses of packed members
via char* + offsetof instead of &member.
Portability: asm/types.h -> linux/types.h, malloc.h and sys/vfs.h dropped
from includes.h (nothing used them), sys/vfs.h -> sys/statfs.h for
statfs(2).
Version: reiserfsprogs.spec said 3.6.23 while configure.ac said 3.6.25.
Remove the dead `#if 0` main() from hashes.c.
Add .github/workflows/build.yml: gcc, clang, --enable-warnings and
--enable-io-failure-emulation, each followed by a format/check/inspect
smoke test on a real image.
Verified: ./autogen.sh && ./configure --enable-warnings && make is clean
from scratch (0 warnings, 51 compile/link steps). mkreiserfs, reiserfsck
--check, debugreiserfs -S, reiserfstune -l and a real resize_reiserfs
128M -> 256M all succeed, and fsck reports the resized image clean.
The autotools setup could not bootstrap with modern autoconf/libtool:
`autoreconf -fi` failed with "possibly undefined macro: AC_PROG_LIBTOOL",
and several other macros used here were removed or deprecated long ago.
configure.ac:
- AC_PROG_LIBTOOL -> LT_INIT (with AC_CONFIG_MACRO_DIRS/AC_CONFIG_AUX_DIR);
add AM_PROG_AR required by newer libtool/automake.
- AM_CONFIG_HEADER -> AC_CONFIG_HEADERS; AC_OUTPUT(files...) -> AC_OUTPUT
with an explicit AC_CONFIG_FILES list.
- AC_TRY_COMPILE -> AC_COMPILE_IFELSE; drop AC_HEADER_STDC,
AC_PROG_GCC_TRADITIONAL, AC_STRUCT_ST_RDEV, AC_C_CONST and AC_C_INLINE
(all obsolete or unconditionally available).
- Drop AC_FUNC_MEMCMP/STRFTIME/VPRINTF; the corresponding HAVE_* macros
were never referenced. Keep the register_printf_* check but test the
ac_cv_func_* names that AC_CHECK_FUNCS actually sets.
- Pin -std=gnu11 (the sources predate C99 and rely on implicit-int-era
behaviour that newer C standards reject) and fix the -Wuninitialized
probe, which never set its cache variable.
- off_t/blkcnt_t sizeof fallbacks 64 -> 8 (bytes, not bits).
reiserfscore.pc.in: substitute @PACKAGE_VERSION@; REISERFSPROGS_VERSION was
never defined, so the generated .pc always carried an empty Version.
lib/parse_time.c: define _GNU_SOURCE, not _XOPEN_SOURCE. The valueless
_XOPEN_SOURCE disables _DEFAULT_SOURCE, which hides loff_t in <sys/types.h>
and broke the build. Every other translation unit already uses _GNU_SOURCE.
reiserfscore/xattr.c: drop the unused <sys/acl.h> include; no libacl symbol
is used (the reiserfs_acl_* structs come from reiserfs_fs.h), and the header
is not a declared build dependency.
Add autogen.sh to regenerate the build system after a fresh clone, and
update .gitignore for the build-aux/m4 layout and generated artifacts.
Verified: ./autogen.sh && ./configure && make completes on gcc 16 /
automake 1.18 / autoconf 2.72. mkreiserfs, reiserfsck --check,
debugreiserfs -S and reiserfstune all run against a freshly formatted
64 MiB image.
is called with a NULL "tb" argument, but the inline implementation of that
function was not prepared to handle it:
Core was generated by `/Data/Compile/Sources/reiserfsprogs-3.6.25/fsck/.libs/lt-reiserfsck --fix-fixab'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x00007fe0d63ef48d in buffer_info_init_bh (tb=0x0, bi=0x7fff03ad3630, bh=0xb60db0) at ../include/reiserfs_fs.h:1584
1584 bi->bi_fs = tb->tb_fs;
(gdb) bt
#0 0x00007fe0d63ef48d in buffer_info_init_bh (tb=0x0, bi=0x7fff03ad3630, bh=0xb60db0) at ../include/reiserfs_fs.h:1584
#1 0x00007fe0d63f21f9 in delete_item (fs=0xb40710, bh=0xb60db0, item_num=0) at lbalance.c:1157
#2 0x000000000040a3aa in pass0_correct_leaf (fs=0xb40710, bh=0xb60db0) at pass0.c:768
#3 0x000000000040deab in do_pass_0 (fs=0xb40710) at pass0.c:1928
#4 0x000000000040f74f in misc_set_bit (nr=4257615, addr=0x7fff03ad3770) at ../include/misc.h:94
#5 0x0000000000406cb0 in rebuild_tree (fs=0xb40710) at main.c:941
#6 0x00000000004088ba in misc_set_bit (nr=140733255074915, addr=0x42a630 <__libc_csu_init+64>) at ../include/misc.h:92
#7 0x00007fe0d5e58291 in __libc_start_main (main=0x4081a4 <main+64>, argc=7, argv=0x7fff03ad38e8, init=<optimized out>, fini=<optimized out>,
rtld_fini=<optimized out>, stack_end=0x7fff03ad38d8) at ../csu/libc-start.c:289
#8 0x00000000004048ca in deregister_tm_clones ()
#9 0x00007fff03ad38d8 in ?? ()
#10 0x000000000000001c in ?? ()
#11 0x0000000000000007 in ?? ()
#12 0x00007fff03ad43ec in ?? ()
#13 0x00007fff03ad4430 in ?? ()
#14 0x00007fff03ad443e in ?? ()
#15 0x00007fff03ad4446 in ?? ()
#16 0x00007fff03ad444c in ?? ()
#17 0x00007fff03ad4454 in ?? ()
#18 0x00007fff03ad4463 in ?? ()
#19 0x0000000000000000 in ?? ()
(gdb) up
#1 0x00007fe0d63f21f9 in delete_item (fs=0xb40710, bh=0xb60db0, item_num=0) at lbalance.c:1157
1157 buffer_info_init_bh(NULL, &bi, bh);
Signed-off-by: Lucas C. Villa Real <lucasvr@gobolinux.org>
We dump failure messages to the terminal regardless of whether the caller
wants them or not. This can be annoying when used as part of a general
tool that tests to see which file system is on a device by calling
its open routine.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
new_insert_key only makes any sense when it's associated with a
new_insert_ptr, which is initialized to NULL and changed to a
buffer_head when we also initialize new_insert_key. We can key off
of that to avoid the uninitialized warning.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
There are several cases in which inline functions are declared in
header files. This causes warnings with newer versions of gcc.
This patch converts them to static inlines.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
There are many cases where functions take pointers to keys, item heads,
paths, etc and don't modify them. Let's annotate them properly.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
There are many calls in which strings are passed as "char *" yet aren't
modified. This patch marks them const so callers can depend on it.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
This adds reiserfs_iterate_dir and reiserfs_iterate_file_data to
iterate over directories and file block/tail mapping.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
The memsets to initialize the stat data items were using the
size of the pointer rather than the structure.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
gcc with -Wall was complaining about defined but unused code. I've removed
the functions/variables that weren't used.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
Initializing structures with little endian values requires a const
value. This means that the endian routines that are macro blocks
cause build failures.
This patch introduces constant_ macros that don't involve an
assignment. These can only be used with constant values or
unexpected results may occur.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
In order to use reiserfs functionality in external programs, we need to
make a shared library and includes available.
This converts both libmisc and libreiserfscore into shared libraries, and
links libmisc into libreiserfscore. We also export the includes and
a pkg-config file.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
By annotating structure members and other on-disk structures as little
endian, we can catch bugs more easily.
The code was already endian safe except for three minor spots:
1) The badblock list code was initializing its key in an endian-unsafe way.
2) Indirect block pointer sequence printing was treating cpu-order values
as little endian.
3) reiserfsck was trying to do endian switching on the lost+found key,
but it was before it was even initialized to anything, so we can
just remove that completely.
These issues were fixed as part of this commit.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
The bitops functions were declared as 'extern inline' in include/misc.h but
weren't actually defined there. They were also defined as inline in
lib/misc.c, which can't actually happen if they're used elsewhere.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
The upstream kernel got rid of the per-endian implementations of
the offset_v2 helpers in favor of explicitly accessing by little
endian bitranges.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
reiserfs_invalidate_buffer contains an argument to specify whether a block
should be freed after being invalidated. It is always passed as 1, so
let's just get rid of it.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
We open code buffer_info initialization all over the place. Let's
factor that out so we don't reproduce the code everywhere.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
This patch does a quick search and replace:
B_N_PITEM_HEAD() -> item_head()
B_N_PDELIM_KEY() -> internal_key()
B_N_PKEY() -> leaf_key()
B_N_PITEM() -> item_body()
And the item_head version:
B_I_PITEM() -> ih_item_body()
I_ENTRY_COUNT() -> ih_entry_count()
And the treepath variants:
get_ih() -> tp_item_head()
PATH_PITEM_HEAD() -> tp_item_head()
get_item() -> tp_item_body()
... which makes the code much easier on the eyes.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
Over the years, the reiserfs kernel code has been gradually cleaned up
to be more readable. This patch does the same to the progs code.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
In order to convert reiserfscore into a externally-usable library,
we need to rename structures so they won't collide with other
namespaces.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
We have met a situation when reiserfsck segfaulted on one of our
partitions. I have performed a small research and the cause appeared
to be wrong detection of transaction count.
Our FS had only one journal transaction, however trans_count which was
passed to progbar_update() as maximum transaction value was detected
as (last_transaction - first_transaction) which in our case was 0.
Thus progbar_update() resulted in segmentation fault:
(gdb) bt
"Replaying journal", curr=1, max=0, dpynum=1) at progbar.c:80
Here is part of debugreiserfs -j output
Journal header (block #8210 of ./partition.bug):
j_last_flush_trans_id 0
j_first_unflushed_offset 0
j_mount_id 10
Device [0x0]
Magic [0x4f0a099e]
Size 8193 blocks (including 1 for journal header) (first block 18)
Max transaction length 1024 blocks
Max batch size 900 blocks
Max commit age 30
Mountid 10, transid 10, desc 18, length 2, commit 21
I successfully reproduced it on versions 3.6.21 (which we were using)
and 3.6.23 (which I believe is the latest).
Attaching patch. Let me know your opinion. If I am wrong, please
comment, if I am correct, feel free to include patch into sources.
Thanks.
Signed-off-by: Victor Leschuk <vleschuk@gmail.com>
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
While Namesys drove almost all reiserfs development and was instrumental
in supporting it for many years, it has been offline for the past
6 years. All of its founders and employees have moved on to other
endeavors or are otherwise unavailable. This can be confusing to users
since the documentation points there either for background information
or to have bugs fixed.
This patch removes most of the non-historical references to
namesys.com and directs users to the reiserfs-devel list on vger.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
lgcosta
Lucas C. Villa Real
Jeff Mahoney
David Prévot
Jeff Mahoney
Victor Leschuck