search

sshock / AFFLIBv3

AFF is an open and extensible file format to store disk images and associated metadata.
Other
80 stars 21 forks source link

aftest: bugs.aff: Invalid argument in HURD (Debian) #42

Closed eribertomota closed 4 years ago

eribertomota commented 4 years ago

Dear @sshock,

As I said in #41, I re-enabled the upstream tests for afflib in Debian. The package was built in several architectures but failed to build from source in hurd-i386. I attached a full build log but you can see the relevant part below:

running all tests with -a option (exception bigfile test)
foo.000=>foo.001
foo.100=>foo.101
bizmark.999=>bizmark.A00
nutter.A99=>nutter.A9A
bizmark.AZZ=>bizmark.B00
glutten.afj=>glutten.afk
aftest: bugs.aff: Invalid argument
FAIL aftest (exit status: 1)

============================================================================
Testsuite summary for AFFLIB 3.7.18
============================================================================
# TOTAL: 1
# PASS:  0
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0

I have SSH access to hurd-i386 machine. Please, let me know if you need more details or tests.

Thanks a lot in advance.

Regards,

Eriberto

afflib_3.7.18-8_hurd-i386.build.gz

sshock commented 4 years ago

@eribertomota This looks like probably the error is that is can't create the bugs.aff file, maybe due to permissions or something? Do you know what the CWD is when aftest gets run and is it somewhere that the test will have access to write to?

eribertomota commented 4 years ago

It is a bit odd for me because all Debian building environments are identical. I don't believe in a permission issue. The CWD is set by Makefile. But we have 22 architectures and aftest fails in hurd-i386 only.

eribertomota commented 4 years ago

I uploaded a workaround some minutes ago (disabling test_signing.sh) but you can see all current builds here[1].

[1] https://buildd.debian.org/status/package.php?p=afflib

sshock commented 4 years ago

Hmm, I wonder if it hurd doesn't like this combination of open flags O_RDWR|O_CREAT|O_TRUNC ?

eribertomota commented 4 years ago

I am creating a hurd environment to run gdb. But I can test any change from you now. Hurd is a slow machine here.

sshock commented 4 years ago

Let's start by verifying this line (line 733) in lib/aftest.cpp is the line that is failing:

AFFILE *af = af_open("bugs.aff",O_RDWR|O_CREAT|O_TRUNC,0666);

by e.g., adding a printf("HERE 1\n"); and a printf("HERE 2\n"); on the line before and after that line.

After confirming that line is the culprit, I think we need to play with those parameters to see what makes it work; e.g., try an absolute path for bugs.aff; e.g., try removing the O_TRUNC flag; e.g., try mode 0 instead of 0666.

eribertomota commented 4 years ago 
(sid_hurd-i386-dchroot)eriberto@exodar:~/afflib-3.7.18$ cat ../afflib_3.7.18-8_hurd-i386.build |grep HERE -C5
foo.100=>foo.101
bizmark.999=>bizmark.A00
nutter.A99=>nutter.A9A
bizmark.AZZ=>bizmark.B00
glutten.afj=>glutten.afk
HERE 1
HERE 2
aftest: bugs.aff: Invalid argument
FAIL aftest (exit status: 1)

============================================================================
Testsuite summary for AFFLIB 3.7.18
sshock commented 4 years ago

oops, I meant to say put the HERE 2 line after this line:

if(!af) err(1,"bugs.aff");
eribertomota commented 4 years ago

I already tried remove OTRUNC. Nothing. Going to s/0666/0.

(sid_hurd-i386-dchroot)eriberto@exodar:~/afflib-3.7.18$ cat ../afflib_3.7.18-8_hurd-i386.build |grep HERE -C5
foo.100=>foo.101
bizmark.999=>bizmark.A00
nutter.A99=>nutter.A9A
bizmark.AZZ=>bizmark.B00
glutten.afj=>glutten.afk
HERE 1
aftest: bugs.aff: Invalid argument
FAIL aftest (exit status: 1)

============================================================================
Testsuite summary for AFFLIB 3.7.18
sshock commented 4 years ago

cool, if changing the mode doesn't help, you can also try changing "bugs.aff" to something else like "/tmp/tmp.aff"

eribertomota commented 4 years ago

s/0666/0 not work

eribertomota commented 4 years ago

Not worked:

     printf("HERE 1\n");
-    AFFILE *af = af_open("bugs.aff",O_RDWR|O_CREAT|O_TRUNC,0666);
+    AFFILE *af = af_open("/tmp/tmp.aff",O_RDWR|O_CREAT|O_TRUNC,0666);
     if(!af) err(1,"bugs.aff");
     printf("HERE 2\n");
sshock commented 4 years ago

can you put the HERE 1 and HERE 2 lines around these two lines in lib/vnode_aff.cpp and try it?

printf("HERE 1\n"); // add this
line 553:    if(!af_is_filestream(af->fname) || (af->openflags & O_ACCMODE) == O_WRONLY)
line 554:        { errno = EINVAL; return -1; }
printf("HERE 2\n"); // add this
eribertomota commented 4 years ago

I added the new lines and I kept the old lines. 

(sid_hurd-i386-dchroot)eriberto@exodar:~/afflib-3.7.18$ cat ../afflib_3.7.18-8_hurd-i386.build |grep HERE -C 5
foo.100=>foo.101
bizmark.999=>bizmark.A00
nutter.A99=>nutter.A9A
bizmark.AZZ=>bizmark.B00
glutten.afj=>glutten.afk
HERE 1
aftest: bugs.aff: Invalid argument
FAIL aftest (exit status: 1)

============================================================================
Testsuite summary for AFFLIB 3.7.18
sshock commented 4 years ago

ok, looks like that check is where it's failing; now we just need to determine if it's failing on the !af_is_filestream(af->fname) or the (af->openflags & O_ACCMODE) == O_WRONLY

can you revert all the changes we've made so far and then replace lines 553 and 554 with this block of code:

if(!af_is_filestream(af->fname))
{
printf("BAD DAY 1\n");
errno = EINVAL;
return -1;
}

if((af->openflags & O_ACCMODE) == O_WRONLY)
{
printf("BAD DAY 2\n");
errno = EINVAL;
return -1;
}
eribertomota commented 4 years ago

Yes. In packaging is mandatory use patches. I will remove the patches and use this new.

eribertomota commented 4 years ago

No results for grep BAD

sshock commented 4 years ago

oh, I must have misinterpreted what your previous test was then; I suppose the HERE 1 in that was from an earlier HERE 1 and not the one in vnode_aff.cpp, so....

...back to square one, also, is it easier if I just attach patches?

eribertomota commented 4 years ago

...back to square one, also, is it easier if I just attach patches?

No. I just change the file and run 'dpkg-source --commit' and debhelper creates the patch for me.

sshock commented 4 years ago

ok, this next test is a little bigger, so I'll paste it as a patch just in case; please revert all previous changes and test this patch:

diff --git a/lib/vnode_aff.cpp b/lib/vnode_aff.cpp
index e07d2c7..b86ab84 100644
--- a/lib/vnode_aff.cpp
+++ b/lib/vnode_aff.cpp
@@ -549,37 +549,52 @@ static int err_close(AFFILE *af)

 static int aff_open(AFFILE *af)
 {
+    printf("aff_open: 1\n");
     // must be a file stream with read access
     if(!af_is_filestream(af->fname) || (af->openflags & O_ACCMODE) == O_WRONLY)
        { errno = EINVAL; return -1; }

+    printf("aff_open: 2\n");
+
     bool canWrite = (af->openflags & O_ACCMODE) == O_RDWR;

     /* Open the raw file */
     int fd = open(af->fname,af->openflags | O_BINARY,af->openmode);
+    printf("aff_open: 3\n");
     if(fd < 0)
        return -1;

+    printf("aff_open: 4\n");
+
     /* Lock the file if writing */
 #ifdef HAVE_FLOCK
     if(flock(fd, canWrite ? LOCK_EX : LOCK_SH) < 0)
        return err_close(fd);
 #endif

+    printf("aff_open: 5\n");
+
     /* Open the FILE for the AFFILE */
     af->aseg = fdopen(fd, canWrite ? "w+b" : "rb");
+    printf("aff_open: 6\n");
     if(!af->aseg)
        return err_close(fd);

+    printf("aff_open: 7\n");
+
     /* Get file size */
     struct stat sb;
     if(fstat(fd, &sb) < 0)
        return err_close(af);

+    printf("aff_open: 8\n");
+
     /* If file is empty, then put out an AFF header, badflag, and AFF version */
     if(canWrite && sb.st_size == 0)
        return aff_create(af);

+    printf("aff_open: 9\n");
+
     /* We are opening an existing file. Verify once more than it is an AFF file
      * and skip past the header...
      */
eribertomota commented 4 years ago

Sorry for delay. I made it manually because copy/paste changed tabs for spaces. Building.

sshock commented 4 years ago

ok, no worries; in the future you don't need to worry about that; the code will work regardless of tabs or spaces for indenting

eribertomota commented 4 years ago

Yes, but the patch command won't apply a patch if tabs changed by spaces.

sshock commented 4 years ago

oh, I see what you mean; sorry about that

eribertomota commented 4 years ago

No problem my friend. No results for grep aff_open.

sshock commented 4 years ago

what in the world? ok, let me take a closer look here...

eribertomota commented 4 years ago

Sorry but I need to send afflib to experimental and sleep. Now 04:12 AM in Brazil.

hurd is a non-official arch in Debian and can wait if needed. s390x is in main line. I will can work in some hours or other day if you busy.

I am thankful for the work you've done .

sshock commented 4 years ago

ok, let's try this

tmp.patch.txt

sshock commented 4 years ago

ok, no worries; it's getting late for me too; I can pick this back up on Saturday if not sooner

eribertomota commented 4 years ago

Thank you! I will send the package to experimental now and tomorrow I will post here the results of the new patch. Have a good night!

eribertomota commented 4 years ago

Sent. In some hours we will have a result here:

https://buildd.debian.org/status/package.php?p=afflib&suite=experimental

Goodbye!

eribertomota commented 4 years ago

Good evening from Brazil @sshock,

S390x, ppc64 and sparc64 has been built like a charm, thanks!

I applied tmp.patch.txt now in hurd-i386. The result of egrep is below.

(sid_hurd-i386-dchroot)eriberto@exodar:~/afflib-3.7.18+git20200701a.e386977$ cat ../afflib_3.7.18+git20200701a.e386977-1_hurd-i386.build | egrep af_open -C 5
foo.100=>foo.101
bizmark.999=>bizmark.A00
nutter.A99=>nutter.A9A
bizmark.AZZ=>bizmark.B00
glutten.afj=>glutten.afk
af_open 1
af_open 2
af_open 3
aftest: bugs.aff: Invalid argument
FAIL aftest (exit status: 1)

============================================================================ Testsuite summary for AFFLIB 3.7.18

eribertomota commented 4 years ago

Hi @sshock,

Now you can run Debian/Hurd-i386 in your own PC! I prepared a simple tutorial about it[1].

[1] https://eriberto.pro.br/wiki/index.php?title=Running_Debian/Hurd_in_KVM

After get your Hurd, you can do it as root:

# mkdir afflib
# cd afflib
# apt build-dep afflib
# apt source afflib
# cd afflib-3.7.18
# ./bootstrap.sh
# ./configure
# make
# make check

When fixed the tests, please, release a new version. If you need any help, please call me!

Thanks a lot in advance!

Cheers,

Eriberto

sshock commented 4 years ago

Cool, I'll give it a try soon.

sshock commented 4 years ago

I had gnu/hurd working then after a bunch of updates and rebooting it now no longer works, even when starting over. It hangs on start ext2fs: Hurd server bootstrap: ext2fs[device:hd0s2] exec.

But that's ok. I think I understand the problem already based on the output you pasted 4 days ago and a note from here:

On GNU/Hurd systems (and not on other systems), O_RDONLY and O_WRONLY are independent bits that can be bitwise-ORed together, and it is valid for either bit to be set or clear. This means that O_RDWR is the same as O_RDONLY|O_WRONLY

So yeah, this is going to be easy to fix.

sshock commented 4 years ago

@eribertomota I just made this commit that I believe will fix the issue: 7a8b5ca

Can you test that for me since hurd on kvm isn't working for me at the moment, and then I'll create a tag/release if it works. Thanks!

eribertomota commented 4 years ago

@sshock Thanks a lot for your efforts.

I think the problem with HURD Kvm machine can be caused by aleatory issues in environment. I think a new try can result in success.

The commit 7a8b5ca caused new fails. See below:


make  check-TESTS
make[3]: Entering directory '/root/afflib/afflib-3.7.18+git20200701a.e386977/tools'
make[4]: Entering directory '/root/afflib/afflib-3.7.18+git20200701a.e386977/tools'
FAIL: test_signing.sh
PASS: test_recovery.sh
FAIL: test_passphrase.sh
FAIL: test_afsegment.sh
FAIL: test_crypto.sh
=========================================
   AFFLIB 3.7.18: tools/test-suite.log
=========================================

# TOTAL: 5
# PASS:  1
# SKIP:  0
# XFAIL: 0
# FAIL:  4
# XPASS: 0
# ERROR: 0

.. contents:: :depth: 2

FAIL: test_signing.sh
=====================

TEST ./test_signing.sh
=== MAKING THE TEST FILES ===
Making the random ISO rawevidence.raw
-rw-r--r-- 1 root root 35786086 Jul  5 12:54 rawevidence.raw
MD5(rawevidence.raw)= b23fc66799175e9c0bc77ca11a509357
==== AFSIGN TEST ===
Making X.509 keys
Generating a RSA private key
........................+++++
..............................................................................................+++++
writing new private key to '/tmp/baseugBJy.agent.pem'
-----
Generating a RSA private key
................+++++
..................................................+++++
writing new private key to '/tmp/baseugBJy.analyst.pem'
-----
Generating a RSA private key
.............................................+++++
.......+++++
writing new private key to '/tmp/baseugBJy.archives.pem'
-----
Making an AFF file to sign
-rw-r--r-- 1 root root 35786086 Jul  5 12:54 rawevidence.raw
rawevidence.raw: Invalid argument
Initial AFF file made:
ls: cannot access '/tmp/baseugBJy.evidence.aff': No such file or directory
affinfo: /tmp/baseugBJy.evidence.aff: No such file or directory
FAIL test_signing.sh (exit status: 1)

FAIL: test_passphrase.sh
========================

=== testing /root/afflib/afflib-3.7.18+git20200701a.e386977/tools/.libs/affcrypto version 3.7.18 ===
=== MAKING THE TEST FILES ==
Making the random ISO /tmp/blankKtIbo.raw
-rw-r--r-- 1 root root 35786086 Jul  5 12:54 /tmp/blankKtIbo.raw
MD5(/tmp/blankKtIbo.raw)= 14e513511e325257b3a835fe9f0146e4
/tmp/blankKtIbo.raw: Invalid argument
FAIL test_passphrase.sh (exit status: 1)

FAIL: test_afsegment.sh
=======================

=== Putting a new metadata segment into blank.aff ===
/tmp/blankK5fOC.raw: Invalid argument
af_open(/tmp/blankK5fOC.aff) failed:: No such file or directory
af_open(/tmp/blankK5fOC.aff) failed:: No such file or directory
affsegment does not work properly
FAIL test_afsegment.sh (exit status: 1)

FAIL: test_crypto.sh
====================

Making encrypted AFF from stored value
Making encrypted AFD
Making plaintext ISO
TESTING WITH PASSPHRASE IN ENVIRONMENT VARIABLE
af_open(/tmp/encrypted61GnPv.raw): Invalid argument
/tmp/encrypted61GnPv.aff does not decrypt properly.
FAIL test_crypto.sh (exit status: 1)

============================================================================
Testsuite summary for AFFLIB 3.7.18
============================================================================
# TOTAL: 5
# PASS:  1
# SKIP:  0
# XFAIL: 0
# FAIL:  4
# XPASS: 0
# ERROR: 0
============================================================================
See tools/test-suite.log
Please report to https://github.com/sshock/AFFLIBv3
============================================================================

Cheers,

Eriberto

sshock commented 4 years ago

Looks like some more work to do. Ok, I'll try to hurd again later and hope it behaves so I can look into these...

sshock commented 4 years ago

Hi @eribertomota . I committed some more fixes, mostly related to open flags that were not being set properly. All the tests are passing for me now. Can you please try again with the latest code from master and make sure it works for you too?

eribertomota commented 4 years ago

Hi @sshock, the changes seem to work now. I uploaded a new package to Debian Experimental and in some hours we will know if the tests are working in all architectures[1]. I will notify you. Thanks a lot for your help!

[1] https://buildd.debian.org/status/package.php?p=afflib&suite=experimental

PS: worked fine your Hurd machine?

eribertomota commented 4 years ago

Hi @sshock! The afflib has been built in all architectures, except to Hurd, due to a temporary issue with a dependency of debhelper (the packaging system). However, I confirmed in my personal Hurd machine that the package is building fine. You can release a new version now.

There are some warnings when building, but it is not a problem for Debian. If you don't like it, please check the reports here, clicking over "Installed" links.

[1] https://buildd.debian.org/status/package.php?p=afflib&suite=experimental

Cheers,

Eriberto

sshock commented 4 years ago

PS: worked fine your Hurd machine?

Yeah, well, I had to switch to another computer and then I was able to get it working.

sshock commented 4 years ago

Hi @sshock! The afflib has been built in all architectures, except to Hurd, due to a temporary issue with a dependency of debhelper (the packaging system). However, I confirmed in my personal Hurd machine that the package is building fine. You can release a new version now.

Cool, I will take care of that later tonight.

There are some warnings when building, but it is not a problem for Debian. If you don't like it, please check the reports here, clicking over "Installed" links.

[1] https://buildd.debian.org/status/package.php?p=afflib&suite=experimental

Cool, I may take a look at those sometime.

eribertomota commented 4 years ago

Hi @sshock! The afflib has been built in all architectures, except to Hurd, due to a temporary issue with a dependency of debhelper (the packaging system). However, I confirmed in my personal Hurd machine that the package is building fine. You can release a new version now.

Cool, I will take care of that later tonight.

Don't worry! This is a specific problem of Debian infrastructure and it was already fixed. The Hurd version was built.

There are some warnings when building, but it is not a problem for Debian. If you don't like it, please check the reports here, clicking over "Installed" links. [1] https://buildd.debian.org/status/package.php?p=afflib&suite=experimental

Cool, I may take a look at those sometime.

Good! If you want, I can upload new changes to Experimental again before you release a new version.

sshock commented 4 years ago

I'm going to call this good for now. The new release has been made!

https://github.com/sshock/AFFLIBv3/releases/tag/v3.7.19

eribertomota commented 4 years ago

Thank you! I will send it to Debian.

Em ter., 7 de jul. de 2020 às 01:33, Phillip Hellewell < notifications@github.com> escreveu:

I'm going to call this good for now. The new release has been made!

https://github.com/sshock/AFFLIBv3/releases/tag/v3.7.19

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sshock/AFFLIBv3/issues/42#issuecomment-654592954, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABRDFLTOV7KAONVL3OWSG7DR2KQTJANCNFSM4OM6WEUQ .