kspalaiologos / modern-rzip

A backup suite. Supports FLZMA2, bzip3, LZ4, Zstandard, LSH i-node ordering deduplicating archiver, long range deduplication, encryption and recovery records
GNU General Public License v2.0
15 stars 1 forks source link

src/mrzip.c:128:71: error: expected expression before ‘,’ token #3

Closed asarubbo closed 1 year ago

asarubbo commented 1 year ago
make V=1 
make -C common all
make[1]: Entering directory '/var/tmp/portage/app-arch/modern-rzip-0.9.5/work/modern-rzip-0.9.5/common'
   CC blake2b.c
make[1]: Leaving directory '/var/tmp/portage/app-arch/modern-rzip-0.9.5/work/modern-rzip-0.9.5/common'
make mrzip
make[1]: Entering directory '/var/tmp/portage/app-arch/modern-rzip-0.9.5/work/modern-rzip-0.9.5'
   CC src/main.c
   CC src/mrzip.c
src/mrzip.c: In function ‘write_magic’:
src/mrzip.c:128:71: error: expected expression before ‘,’ token
  128 |     unsigned char magic[MAGIC_LEN] = { 'M', 'R', 'Z', 'I', MRZIP_MAJOR, MRZIP_MINOR };
      |                                                                       ^
make[1]: *** [common.mk:30: src/mrzip.o] Error 1
make[1]: Leaving directory '/var/tmp/portage/app-arch/modern-rzip-0.9.5/work/modern-rzip-0.9.5'
make: *** [Makefile:23: all] Error 2
$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-pc-linux-gnu/12/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /var/tmp/portage/sys-devel/gcc-12.2.1_p20230121-r1/work/gcc-12-20230121/configure --host=x86_64-pc-linux-gnu --build=x86_64-pc-linux-gnu --prefix=/usr --bindir=/usr/x86_64-pc-linux-gnu/gcc-bin/12 --includedir=/usr/lib/gcc/x86_64-pc-linux-gnu/12/include --datadir=/usr/share/gcc-data/x86_64-pc-linux-gnu/12 --mandir=/usr/share/gcc-data/x86_64-pc-linux-gnu/12/man --infodir=/usr/share/gcc-data/x86_64-pc-linux-gnu/12/info --with-gxx-include-dir=/usr/lib/gcc/x86_64-pc-linux-gnu/12/include/g++-v12 --with-python-dir=/share/gcc-data/x86_64-pc-linux-gnu/12/python --enable-languages=c,c++,fortran --enable-obsolete --enable-secureplt --disable-werror --with-system-zlib --enable-nls --without-included-gettext --disable-libunwind-exceptions --enable-checking=release --with-bugurl=https://bugs.gentoo.org/ --with-pkgversion='Gentoo 12.2.1_p20230121-r1 p10' --with-gcc-major-version-only --disable-esp --enable-libstdcxx-time --disable-libstdcxx-pch --enable-shared --enable-threads=posix --enable-__cxa_atexit --enable-clocale=gnu --enable-multilib --with-multilib-list=m32,m64 --disable-fixed-point --enable-targets=all --enable-libgomp --disable-libssp --disable-libada --disable-cet --disable-systemtap --disable-valgrind-annotations --disable-vtable-verify --disable-libvtv --without-zstd --enable-lto --without-isl --disable-default-pie --disable-default-ssp
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 12.2.1 20230121 (Gentoo 12.2.1_p20230121-r1 p10)

Did I miss something?

kspalaiologos commented 1 year ago

Hello! Mind posting the output of ./configure?

./configure should define the following variables:

------------------------------------------------------------------------
MODERN-RZIP 0.9.5 (0.9.5) CONFIGURATION SUMMARY
------------------------------------------------------------------------

Compilation............: make (or gmake)
  CFLAGS...............: -g -O2 -msse2
  CXXFLAGS.............: -g -O2 -msse2
  CC...................: gcc
  CXX..................: g++ -std=gnu++17
  STRIP................: strip
  WINDOWS..............: 0
  VERSION..............: -DMRZIP_MAJOR=0 -DMRZIP_MINOR=9 -DMRZIP_PATCH=5
kspalaiologos commented 1 year ago

Okay, I have reviewed it and it appears that a recent git upgrade that fixes a vulnerability is being hypervigilant:

git (1:2.20.1-2+deb10u5) buster-security; urgency=high

  * Non-maintainer upload by the LTS Security Team.
  * CVE-2022-24765: Git not checking the ownership of directories in a
    local multi-user system when running commands specified in the local
    repository configuration.  This allows the owner of the repository to
    cause arbitrary commands to be executed by other users who access the
    repository.
  * The above introduces new 'safe.directory' checks which may cause
    regressions: allow opt-out of such checks with 'safe.directory=*'
  * CVE-2022-29187: an unsuspecting user could still be affected by the
    issue reported in CVE-2022-24765, for example when navigating as root
    into a shared tmp directory that is owned by them, but where an
    attacker could create a git repository.
  * CVE-2022-39253: exposure of sensitive information to a malicious
    actor. When performing a local clone (where the source and target of
    the clone are on the same volume), Git copies the contents of the
    source's `$GIT_DIR/objects` directory into the destination by either
    creating hardlinks to the source contents, or copying them (if
    hardlinks are disabled via `--no-hardlinks`). A malicious actor could
    convince a victim to clone a repository with a symbolic link pointing
    at sensitive information on the victim's machine.
  * CVE-2022-39260: `git shell` improperly uses an `int` to represent the
    number of entries in the array, allowing a malicious actor to
    intentionally overflow the return value, leading to arbitrary heap
    writes. Because the resulting array is then passed to `execv()`, it is
    possible to leverage this attack to gain remote code execution on a
    victim machine.

 -- Sylvain Beucler <beuc@debian.org>  Tue, 13 Dec 2022 15:14:23 +0100

Seems like WikiMedia was also hit by this issue: https://phabricator.wikimedia.org/T325128. A suggested fix is:

$ git config --global --add safe.directory /path/to/modern-rzip
asarubbo commented 1 year ago

Hi, I filed the issue without digging a bit to understand the cause, but now it is clear.

I'm using the 0.9.5 tarball (https://github.com/modern-rzip/modern-rzip/archive/refs/tags/0.9.5.tar.gz), so the following commands into configure do not work because it is not a git repo:

MRZIP_MAJOR=`git describe --tags --abbrev=0 | sed 's/v//' | cut -d. -f1`
MRZIP_MINOR=`git describe --tags --abbrev=0 | sed 's/v//' | cut -d. -f2`
MRZIP_PATCH=`git describe --tags --abbrev=0 | sed 's/v//' | cut -d. -f3`
FULLVER=`git describe --tags --abbrev=0 | sed 's/v//'

Infact the configure outputs the following:

  VERSION..............: -DMRZIP_MAJOR= -DMRZIP_MINOR= -DMRZIP_PATCH=

The fast fix I can suggest is manually put the version number into configure.ac that need to be incremented when you do version bump.

kspalaiologos commented 1 year ago

I have not implemented source tarball releases (yet), but I will get around to doing this soon-ish.

asarubbo commented 1 year ago

I have bypassed the issue for now, by manually adding the magic numbers to the configure.

Now I have another issue during make phase:

   CC src/stream.c
src/stream.c:37:10: fatal error: ../vendor/bzip3/include/libbz3.h: No such file or directory
   37 | #include "../vendor/bzip3/include/libbz3.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

There is a bit of confusion about the paths, my suggestion is: let the configure understand if there is a missing library in the system (bzip3/zstd/lz4) and do not bundle libraries in vendor, or if you want to bundle for some reasons, just declare the #include as usual and add -Ivendor/$dir where needed. Hope it helps.

kspalaiologos commented 1 year ago

Hello. The tarball you had downloaded does not include the vendored files. Could you try the following: https://github.com/modern-rzip/modern-rzip/releases/download/0.9.6/mrzip-src.tar.gz

asarubbo commented 1 year ago

Hello. The tarball you had downloaded does not include the vendored files. Could you try the following: https://github.com/modern-rzip/modern-rzip/releases/download/0.9.6/mrzip-src.tar.gz

it works for me now, however from the configure I get:

  VERSION..............: -DMRZIP_MAJOR=0 -DMRZIP_MINOR=9 -DMRZIP_PATCH=MRZIP_PATCH

because .version here is:

0.9.6
0
9
MRZIP_PATCH

Another problem I get (please tell me if you want a new ticket) is that in the install phase, binaries want to be installed as /usr/bin that is a directory, maybe there is a missing / at the end

asarubbo commented 1 year ago

it works for me now, however from the configure I get:

  VERSION..............: -DMRZIP_MAJOR=0 -DMRZIP_MINOR=9 -DMRZIP_PATCH=MRZIP_PATCH

because .version here is:

0.9.6
0
9
MRZIP_PATCH

There is a missin $ here: https://github.com/modern-rzip/modern-rzip/blob/627de2bf8fbe6c3af3500811be587586ce084fca/.github/workflows/release.yml#L23

kspalaiologos commented 1 year ago

Oh, good catch. I will fix this now.

asarubbo commented 1 year ago

it partially works for me because it uses the install defined in the build system instead of the install program defined in the system.

Is there a chance to get it using the default install program? Ftr bzip3 works for me from this point.