Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v4: invalid legacy format produced #149

Closed
anatol opened this issue Nov 8, 2021 · 3 comments · Fixed by #150
Closed

v4: invalid legacy format produced #149

anatol opened this issue Nov 8, 2021 · 3 comments · Fixed by #150

Comments

@anatol
Copy link
Contributor

anatol commented Nov 8, 2021

I am trying to port https://github.com/anatol/booster from v2.6.1 to v4 API. And it turns that the new code fails to produce Linux-bootable images. Linux needs the lz4 legacy format. And it looks like the lz4 legacy output is not valid.

I checked the lz4 tests and see that the test golden files are not valid. Here is result of checking it with lz4 command line tool at Arch Linux:

$ lz4 -v
*** LZ4 command line interface 64-bits v1.9.3, by Yann Collet ***
$ lz4 --test vmlinux_LZ4_19377.lz4   
Stream followed by undecodable data at position 11398804 
vmlinux_LZ4_19377.lz : decoded 45037000 bytes                                  
$ lz4 --test bzImage_lz4_isolated.lz4 
Error 52 : Read error : cannot access compressed block !

And with booster tests I get following error:

$ lz4 --test booster.img
Error 53 : Decoding Failed ! Corrupted input detected !
@anatol
Copy link
Contributor Author

anatol commented Nov 8, 2021

When I modify the testcase to check output of the old and the new legacy implementations then it fails:

diff --git a/go.mod b/go.mod
index 42229b2..16acace 100644
--- a/go.mod
+++ b/go.mod
@@ -1,3 +1,8 @@
 module github.com/pierrec/lz4/v4
 
 go 1.14
+
+require (
+       github.com/frankban/quicktest v1.14.0 // indirect
+       github.com/pierrec/lz4 v2.6.1+incompatible
+)
diff --git a/go.sum b/go.sum
index e69de29..4a947fa 100644
--- a/go.sum
+++ b/go.sum
@@ -0,0 +1,20 @@
+github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
+github.com/frankban/quicktest v1.14.0 h1:+cqqvzZV87b4adx/5ayVOaYZ2CrvM4ejQvUdBzPPUss=
+github.com/frankban/quicktest v1.14.0/go.mod h1:NeW+ay9A/U67EYXNFA1nPE8e/tnQv/09mUdL/ijj8og=
+github.com/google/go-cmp v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ=
+github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
+github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
+github.com/kr/pretty v0.3.0 h1:WgNl7dwNpEZ6jJ9k1snq4pZsg7DOEN8hP9Xw0Tsjwk0=
+github.com/kr/pretty v0.3.0/go.mod h1:640gp4NfQd8pI5XOwp5fnNeVWj67G7CFk/SaSQn7NBk=
+github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
+github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
+github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
+github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
+github.com/pierrec/lz4 v2.6.1+incompatible h1:9UY3+iC23yxF0UfGaYrGplQ+79Rg+h/q9FV9ix19jjM=
+github.com/pierrec/lz4 v2.6.1+incompatible/go.mod h1:pdkljMzZIN41W+lC3N2tnIh5sFi+IEE17M5jbnwPHcY=
+github.com/rogpeppe/go-internal v1.6.1 h1:/FiVV8dS/e+YqF2JvO3yXRFbBLTIuSDkuC7aBOAvL+k=
+github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc=
+golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
+golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
+gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI=
diff --git a/writer_test.go b/writer_test.go
index d8c40c4..8ce366f 100644
--- a/writer_test.go
+++ b/writer_test.go
@@ -11,6 +11,7 @@ import (
        "strings"
        "testing"
 
+       oldlz4 "github.com/pierrec/lz4"
        "github.com/pierrec/lz4/v4"
        "github.com/pierrec/lz4/v4/internal/lz4block"
 )
@@ -247,12 +248,15 @@ func TestWriterLegacy(t *testing.T) {
                        }
 
                        out2 := new(bytes.Buffer)
-                       zr := lz4.NewReader(out)
-                       if _, err := io.Copy(out2, zr); err != nil {
+                       legacy := oldlz4.NewWriterLegacy(out2)
+                       if _, err := io.Copy(legacy, bytes.NewReader(src)); err != nil {
                                t.Fatal(err)
                        }
-                       if !bytes.Equal(out2.Bytes(), src) {
-                               t.Fatal("uncompressed compressed output different from source")
+                       if err := legacy.Close(); err != nil {
+                               t.Fatal(err)
+                       }
+                       if !bytes.Equal(out.Bytes(), out2.Bytes()) {
+                               t.Fatal("new legacy output does not match the old one")
                        }
                })
        }

@anatol
Copy link
Contributor Author

anatol commented Nov 10, 2021

Here is the legacy format expected by the kernel https://github.com/lz4/lz4/blob/dev/doc/lz4_Frame_format.md#legacy-frame

Main characteristics of the legacy format :

  • Fixed block size : 8 MB.
  • All blocks must be completely filled, except the last one.
  • All blocks are always compressed, even when compression is detrimental.
  • The last block is detected either because it is followed by the “EOF” (End of File) mark, or because it is followed by a known Frame Magic Number.
  • No checksum
  • Convention is Little endian
MagicNb B.CSize CData B.CSize CData (...) EndMark
4 bytes 4 bytes CSize 4 bytes CSize x times EOF

anatol added a commit to anatol/lz4 that referenced this issue Nov 11, 2021
Per https://github.com/lz4/lz4/blob/dev/doc/lz4_Frame_format.md#legacy-frame
legacy format uses 8MB fixed-size blocks.

Test it with generating lz4 initramfs image for booster and boot it with
Linux 5.14

Closes pierrec#149
@anatol
Copy link
Contributor Author

anatol commented Dec 3, 2021

And another related issue #156

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant