Fix real bugs and clean up the build with -Wall -Wextra
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.
This commit is contained in:
1 parent
41fb30313e
commit
293e6a7280
28 files changed
+258
-190
No files matched your search
+82
-84
@@ -9,10 +9,11 @@
|
||||
|
||||
#include <stdio.h>
|
||||
#include <stdarg.h>
|
||||
#include <stddef.h>
|
||||
#include <string.h>
|
||||
#include <errno.h>
|
||||
#include <mntent.h>
|
||||
#include <sys/vfs.h>
|
||||
#include <sys/statfs.h>
|
||||
#include <time.h>
|
||||
#include <utime.h>
|
||||
#include <ctype.h>
|
||||
@@ -33,125 +34,117 @@ void die(const char *fmt, ...)
|
||||
va_list args;
|
||||
|
||||
va_start(args, fmt);
|
||||
vsprintf(buf, fmt, args);
|
||||
vsnprintf(buf, sizeof(buf), fmt, args);
|
||||
va_end(args);
|
||||
|
||||
fprintf(stderr, "\n%s\n", buf);
|
||||
abort();
|
||||
}
|
||||
|
||||
#define MEM_BEGIN "_mem_begin_"
|
||||
#define MEM_END "mem_end"
|
||||
#define MEM_FREED "__free_"
|
||||
#define CONTROL_SIZE (strlen (MEM_BEGIN) + 1 + sizeof (int) + strlen (MEM_END) + 1)
|
||||
/*
|
||||
* Small size-tracking allocator. Every block handed out by getmem()/
|
||||
* mem_alloc()/expandmem() is preceded by a header recording its usable
|
||||
* size. get_mem_size() reports it back and expandmem() uses it to grow
|
||||
* in place without the caller having to remember the old size.
|
||||
*
|
||||
* The header is over-aligned so the returned pointer keeps malloc's
|
||||
* alignment guarantees for any type.
|
||||
*/
|
||||
union mem_header {
|
||||
struct {
|
||||
size_t size;
|
||||
} h;
|
||||
max_align_t align;
|
||||
};
|
||||
|
||||
/* asprintf() wrapper: allocation failure is fatal, matching the rest of
|
||||
this file (die()). Use it where the old code ignored asprintf()'s
|
||||
return value and then dereferenced the pointer. */
|
||||
int xasprintf(char **ret, const char *fmt, ...)
|
||||
{
|
||||
va_list args;
|
||||
int rc;
|
||||
|
||||
va_start(args, fmt);
|
||||
rc = vasprintf(ret, fmt, args);
|
||||
va_end(args);
|
||||
|
||||
if (rc < 0)
|
||||
die("xasprintf: out of memory");
|
||||
|
||||
return rc;
|
||||
}
|
||||
|
||||
#define MEM_HEADER_SIZE (sizeof(union mem_header))
|
||||
|
||||
static union mem_header *mem_header_of(const void *p)
|
||||
{
|
||||
return (union mem_header *)((char *)p - MEM_HEADER_SIZE);
|
||||
}
|
||||
|
||||
unsigned int get_mem_size(const char *p)
|
||||
{
|
||||
const char *begin;
|
||||
|
||||
begin = p - strlen(MEM_BEGIN) - 1 - sizeof(int);
|
||||
return *(int *)(begin + strlen(MEM_BEGIN) + 1);
|
||||
return mem_header_of(p)->h.size;
|
||||
}
|
||||
|
||||
void checkmem(const char *p, int size)
|
||||
void *mem_alloc(int size)
|
||||
{
|
||||
const char *begin;
|
||||
const char *end;
|
||||
union mem_header *hdr;
|
||||
char *mem;
|
||||
|
||||
begin = p - strlen(MEM_BEGIN) - 1 - sizeof(int);
|
||||
if (strcmp(begin, MEM_BEGIN))
|
||||
die("checkmem: memory corrupted - invalid head sign");
|
||||
if (size < 0)
|
||||
die("mem_alloc: negative size (%d)", size);
|
||||
|
||||
if (*(int *)(begin + strlen(MEM_BEGIN) + 1) != size)
|
||||
die("checkmem: memory corrupted - invalid size");
|
||||
hdr = malloc(MEM_HEADER_SIZE + (size_t) size);
|
||||
if (!hdr)
|
||||
die("mem_alloc: no more memory (%d)", size);
|
||||
|
||||
end = begin + size + CONTROL_SIZE - strlen(MEM_END) - 1;
|
||||
if (strcmp(end, MEM_END))
|
||||
die("checkmem: memory corrupted - invalid end sign");
|
||||
hdr->h.size = size;
|
||||
mem = (char *)hdr + MEM_HEADER_SIZE;
|
||||
|
||||
return mem;
|
||||
}
|
||||
|
||||
void *getmem(int size)
|
||||
{
|
||||
char *mem;
|
||||
|
||||
if ((mem = mem_alloc(size)) == NULL)
|
||||
die("getmem: no more memory (%d)", size);
|
||||
|
||||
mem = mem_alloc(size);
|
||||
memset(mem, 0, size);
|
||||
// checkmem (mem, size);
|
||||
|
||||
return mem;
|
||||
}
|
||||
|
||||
void *mem_alloc(int size)
|
||||
{
|
||||
char *p;
|
||||
char *mem;
|
||||
|
||||
p = (char *)malloc(CONTROL_SIZE + size);
|
||||
if (!p)
|
||||
die("getmem: no more memory (%d)", size);
|
||||
|
||||
/* Write the MEM_BEGIN magic in the beginning of allocated memory. */
|
||||
strcpy(p, MEM_BEGIN);
|
||||
p += strlen(MEM_BEGIN) + 1;
|
||||
/* Write the size after the magic. */
|
||||
*(int *)p = size;
|
||||
p += sizeof(int);
|
||||
mem = p;
|
||||
p += size;
|
||||
strcpy(p, MEM_END);
|
||||
|
||||
return mem;
|
||||
}
|
||||
|
||||
void *expandmem(void *vp, int size, int by)
|
||||
{
|
||||
int allocated;
|
||||
char *mem, *p = vp;
|
||||
int expand_by = by;
|
||||
union mem_header *hdr;
|
||||
char *mem;
|
||||
size_t old_size = vp ? (size_t) mem_header_of(vp)->h.size : 0;
|
||||
size_t new_size = old_size + by;
|
||||
|
||||
if (p) {
|
||||
checkmem(p, size);
|
||||
allocated = CONTROL_SIZE + size;
|
||||
p -= (strlen(MEM_BEGIN) + 1 + sizeof(int));
|
||||
} else {
|
||||
allocated = 0;
|
||||
/* add control bytes to the new allocated area */
|
||||
expand_by += CONTROL_SIZE;
|
||||
}
|
||||
p = realloc(p, allocated + expand_by);
|
||||
if (!p)
|
||||
if (by < 0)
|
||||
die("expandmem: negative increment (%d)", by);
|
||||
|
||||
hdr = realloc(vp ? mem_header_of(vp) : NULL, MEM_HEADER_SIZE + new_size);
|
||||
if (!hdr)
|
||||
die("expandmem: no more memory (%d)", size);
|
||||
if (!vp) {
|
||||
strcpy(p, MEM_BEGIN);
|
||||
}
|
||||
mem = p + strlen(MEM_BEGIN) + 1 + sizeof(int);
|
||||
|
||||
*(int *)(p + strlen(MEM_BEGIN) + 1) = size + by;
|
||||
/* fill new allocated area by 0s */
|
||||
mem = (char *)hdr + MEM_HEADER_SIZE;
|
||||
hdr->h.size = new_size;
|
||||
|
||||
/* fill the newly allocated area with 0s */
|
||||
if (by > 0)
|
||||
memset(mem + size, 0, by);
|
||||
strcpy(mem + size + by, MEM_END);
|
||||
// checkmem (mem, size + by);
|
||||
memset(mem + old_size, 0, by);
|
||||
|
||||
return mem;
|
||||
}
|
||||
|
||||
void freemem(void *vp)
|
||||
{
|
||||
char *p = vp;
|
||||
int size;
|
||||
|
||||
if (!p)
|
||||
if (!vp)
|
||||
return;
|
||||
size = get_mem_size(vp);
|
||||
checkmem(p, size);
|
||||
|
||||
p -= (strlen(MEM_BEGIN) + 1 + sizeof(int));
|
||||
strcpy(p, MEM_FREED);
|
||||
strcpy(p + size + CONTROL_SIZE - strlen(MEM_END) - 1, MEM_FREED);
|
||||
free(p);
|
||||
free(mem_header_of(vp));
|
||||
}
|
||||
|
||||
typedef int (*func_t) (const char *);
|
||||
@@ -714,13 +707,18 @@ int prepare_dma_check(dma_info_t *dma_info)
|
||||
}
|
||||
|
||||
while ((dirent = readdir(dir)) != NULL) {
|
||||
int n;
|
||||
|
||||
if (strncmp(dirent->d_name, ".", 1) == 0
|
||||
|| strncmp(dirent->d_name, "..", 2) == 0)
|
||||
continue;
|
||||
memset(buf, 0, 256);
|
||||
strncat(buf, "/dev/", 5);
|
||||
strncat(buf, dirent->d_name,
|
||||
strlen(dirent->d_name));
|
||||
|
||||
/* "/dev/" + name; skip names that do not fit
|
||||
rather than truncating into a wrong path */
|
||||
n = snprintf(buf, sizeof(buf), "/dev/%s",
|
||||
dirent->d_name);
|
||||
if (n < 0 || (size_t) n >= sizeof(buf))
|
||||
continue;
|
||||
|
||||
if (stat(buf, &st))
|
||||
break;
|
||||
|
||||
Reference in new issue
Block a user