BTRFS from basic master #3065

simsong · 2024-10-26T20:36:08Z

This is #413 but brought up to develop.

Add test case for btrfs
Make sure any other code changed is also tested.

simsong · 2024-10-27T22:11:51Z

Still waiting for AppVeyor, but this now works on windows, linux and MacOS. It would be great for @bcarrier and @jayaramcs to take a look, as this is a new file system for TSK. That's pretty significant! @joachimmetz and @uckelman-sf are also invited to comment.

simsong · 2024-10-28T14:30:46Z

The AppVeyor fails were because:

PyYAML didn't build
AppVeyor doesn't have libewf installed.

joachimmetz · 2024-10-28T18:07:06Z

Nice, would recommend adding a fuzz target as well https://github.com/sleuthkit/sleuthkit/tree/develop/ossfuzz

simsong · 2024-10-28T18:23:48Z

I don't know how to add the fuzz targets, but if you tell me how, I'll do it!

joachimmetz · 2024-10-28T19:23:47Z

I don't know how to add the fuzz targets, but if you tell me how, I'll do it!

sure, should not be too involved, let me ask a couple of questions first to get some more context

is there a small test file publicly available that can be used as seed? for https://github.com/sleuthkit/sleuthkit/blob/develop/ossfuzz/buildcorpus.sh
does the btrfs back end require the pool layer like apfs or is it more comparable to the ext backend?

simsong · 2024-10-28T23:31:34Z

I don't know how to add the fuzz targets, but if you tell me how, I'll do it!

sure, should not be too involved, let me ask a couple of questions first to get some more context

is there a small test file publicly available that can be used as seed? for https://github.com/sleuthkit/sleuthkit/blob/develop/ossfuzz/buildcorpus.sh

does the btrfs back end require the pool layer like apfs or is it more comparable to the ext backend?

How about https://corp.digitalcorpora.org/corpora/drives/2015-basicmaster/btrfs_testimage_50MB.E01?

btrfs sits on top of raw storage, like ext.

joachimmetz · 2024-10-29T05:07:49Z

How about https://corp.digitalcorpora.org/corpora/drives/2015-basicmaster/btrfs_testimage_50MB.E01?

to isolate just fuzzing the btrfs backend a smaller only btrfs raw file system (let me see if I can create a script and add it to https://github.com/dfirlabs/btrfs-specimens). also note that the ossfuzz build does not build with E01 support

btrfs sits on top of raw storage, like ext.

the fls fuzz target likely can be reused then https://github.com/sleuthkit/sleuthkit/blob/develop/ossfuzz/fls_fuzzer.cc
and only an update to the Makefile is needed https://github.com/sleuthkit/sleuthkit/blob/develop/Makefile.am#L825 comparable to

fls_btrfs_fuzzer_SOURCES = \
	ossfuzz/fls_fuzzer.cc \
	ossfuzz/mem_img.h

fls_btrfs_fuzzer_CPPFLAGS = \
	-DFSTYPE=TSK_FS_TYPE_BTRFS_DETECT \
	$(AM_CPPFLAGS)

fls_btrfs_fuzzer_LDADD = \
	@LIB_FUZZING_ENGINE@ \
	$(TSK_LIB)

joachimmetz · 2024-10-29T05:21:35Z

let me see if I can create a script and add it to https://github.com/dfirlabs/btrfs-specimens

Looks like the minimum required size is 114294784, that might be too large. Note that the seed file is not required but helps with more targeted fuzzing.

simsong · 2024-10-29T08:43:11Z

I’m unclear what the seed file does if it is not needed. Instead of making the seed files once and downloading it, why not make it as part of the fuzzing operation?

…

On Tue, Oct 29, 2024 at 1:22 AM Joachim Metz ***@***.***> wrote: let me see if I can create a script and add it to https://github.com/dfirlabs/btrfs-specimens Looks like the minimum required size is 114294784, that might be too large. Note that the seed file is not required but helps with more targeted fuzzing. — Reply to this email directly, view it on GitHub <#3065 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMFHLDZ425S3QVKHFXOKW3Z54LPLAVCNFSM6AAAAABQVDU2J6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINBTGIZTSOJXGI> . You are receiving this because you authored the thread.Message ID: ***@***.***>

joachimmetz · 2024-10-29T08:56:50Z

I’m unclear what the seed file does if it is not needed.

tldr it helps with improving test coverage, maybe useful context https://google.github.io/oss-fuzz/getting-started/new-project-guide/#seed-corpus

why not make it as part of the fuzzing operation?

not sure what you specifically are asking with this, but what the fuzzer does is invoke the fuzz targets with mutated input based on the seed corpus

simsong · 2024-10-29T09:08:17Z

Since the fuzzer is running on a Linux system, instead of downloading the seed, perhaps it could just run the script to create the seed.

…

On Oct 29, 2024, at 4:57 AM, Joachim Metz ***@***.***> wrote: I’m unclear what the seed file does if it is not needed. tldr it helps with improving test coverage, maybe useful context https://google.github.io/oss-fuzz/getting-started/new-project-guide/#seed-corpus why not make it as part of the fuzzing operation? not sure what you specifically are asking with this, but what the fuzzer does is invoke the fuzz targets with mutated input based on the seed corpus — Reply to this email directly, view it on GitHub <#3065 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMFHLEPCIMGC4DXVOENIUDZ55EWTAVCNFSM6AAAAABQVDU2J6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINBTGYYTKMRWGY>. You are receiving this because you authored the thread.

joachimmetz · 2024-10-29T09:21:46Z

The seed file is not required, adding the fuzz target should be sufficient for now. Seed file can always be added later as well.

simsong · 2024-10-29T10:54:46Z

I clearly don’t understand how the fuzzer works. Doesn’t it need an executable that it runs? So doesn’t it need a data file to run with the executable? Can you point me at academic paper that describes how it works? Thanks..

…

On Tue, Oct 29, 2024 at 5:22 AM Joachim Metz ***@***.***> wrote: The seed file is not required, adding the fuzz target should be sufficient for now. Seed file can always be added later as well. — Reply to this email directly, view it on GitHub <#3065 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMFHLEIMWD2FLLTZWW7KWLZ55HUFAVCNFSM6AAAAABQVDU2J6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINBTGY3TCNBVHA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

joachimmetz · 2024-10-29T12:13:34Z

I'm unaware of academic paper specifically but the project documentation can be found here https://google.github.io/oss-fuzz/

The proposed changes to Makefile.am (in #3065 (comment)) and adding fls_btrfs_fuzzer to the corresponding noinst_PROGRAMS in should be sufficient to get a btrfs fuzz target binary to build and picked up by OSSFuzz.

simsong · 2024-10-29T13:26:07Z

I'm unaware of academic paper specifically but the project documentation can be found here https://google.github.io/oss-fuzz/

Thanks. I wonder how hard it would be to get it to work with bulk_extractor.

The proposed changes to Makefile.am (in #3065 (comment)) and adding fls_btrfs_fuzzer to the corresponding noinst_PROGRAMS in should be sufficient to get a btrfs fuzz target binary to build and picked up by OSSFuzz.

I'll look into it and see if I can add it to this PR. Meanwhile, I'm during this BT back to a draft until I hear from @jayaramcs.

simsong · 2024-10-29T13:33:08Z

fls_btrfs_fuzzer_SOURCES = \
	ossfuzz/fls_fuzzer.cc \
	ossfuzz/mem_img.h

fls_btrfs_fuzzer_CPPFLAGS = \
	-DFSTYPE=TSK_FS_TYPE_BTRFS_DETECT \
	$(AM_CPPFLAGS)

fls_btrfs_fuzzer_LDADD = \
	@LIB_FUZZING_ENGINE@ \
	$(TSK_LIB)

Oh, I get it. This compiles a fls_btrfs_fuzzer executable with the fuzzing engine linked in and LLVMFuzzerTestOneInput as the test harness. But the fuzzing doesn't happen in the GitHub action, it happens over at Google in GCP. That's why the size is an issue.

I'm still unclear on where the code analysis is happening. Is the fuzz happening with source code inspection or object code inspection? This is presumably data mutation to cause more execution graph exploration, right?

joachimmetz · 2024-10-29T15:41:41Z

I'm still unclear on where the code analysis is happening. Is the fuzz happening with source code inspection or object code inspection?

It is configured to run on clusterfuzz. Also see https://google.github.io/oss-fuzz/further-reading/clusterfuzz/ but you can also run it locally.

Issues are available through https://issues.oss-fuzz.com/ Brian currently has access, given he is configure as project owner in https://github.com/google/oss-fuzz/tree/master/projects/sleuthkit

joachimmetz · 2024-10-29T15:43:54Z

Regarding 1f84b56#diff-0462e381b2fb3286568215681c8983490a37ac9ae0f0c5ee304df7fa6426d4afR807

you also need to add fls_btrfs_fuzzer to noinst_PROGRAMS on line 805

uckelman · 2024-10-29T17:53:03Z

There should be a test image for validating how Sleuthkit reads BTRFS, to go with the other test images in #3044. Has there been any progress on that?

simsong · 2024-10-29T18:49:15Z

There should be a test image for validating how Sleuthkit reads BTRFS, to go with the other test images in #3044. Has there been any progress on that?

Yes. We have one here:
https://corp.digitalcorpora.org/corpora/drives/2015-basicmaster/btrfs_testimage_50MB.E01

My understanding is that @joachimmetz is concerned about the size.

simsong · 2024-10-29T20:43:06Z

How can I get access to the issues?

…

On Oct 29, 2024, at 11:42 AM, Joachim Metz ***@***.***> wrote: I'm still unclear on where the code analysis is happening. Is the fuzz happening with source code inspection or object code inspection? It is configured to run on clusterfuzz. Also see https://google.github.io/oss-fuzz/further-reading/clusterfuzz/ but you can also run it locally. Issues are available through https://issues.oss-fuzz.com/ Brian currently has access, given he is configure as project owner in https://github.com/google/oss-fuzz/tree/master/projects/sleuthkit — Reply to this email directly, view it on GitHub <#3065 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMFHLCHUSZ7DPGUSAPRCM3Z56UEXAVCNFSM6AAAAABQVDU2J6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINBUGY2TIOJVGQ>. You are receiving this because you authored the thread.

uckelman · 2024-10-29T21:00:15Z

tsk/fs/Makefile.am

@@ -0,0 +1,27 @@
+AM_CPPFLAGS = -I../.. -I$(srcdir)/../.. -Wall 


New makefiles shouldn't be created in subdirectories. This should go into the main (and only) Makefile.am.

Legacy cruft from the original PR. It's not being used. I'll delete it. Thanks for catching it.

simsong · 2024-11-02T10:51:49Z

It's unclear to me why the AppVeryor failed, so I'm doing an update branch and we will see if it fails again.

uckelman · 2024-11-02T12:23:26Z

Makefile.am

@@ -701,9 +704,6 @@ endif
 tsk_jar = $(top_builddir)/bindings/java/dist/sleuthkit-$(PACKAGE_VERSION).jar
 jardir = $(prefix)/share/java

-$(tsk_jar):
-	cd bindings/java ; ant dist $(ant_args)
-


Why is this being removed?

user error. I'll add it back.

Makefile.am

…nused-parameter\" to CXXFLAGS=-Werror"

simsong · 2024-11-26T17:29:40Z

fixed in d98b278

…

On Nov 26, 2024, at 5:44 PM, Joel Uckelman ***@***.***> wrote: @uckelman-sf commented on this pull request. In .github/workflows/build-unix.yml <#3065 (comment)>: > @@ -36,7 +36,7 @@ jobs: linkage: "shared" compiler: "gcc" runner: "ubuntu-22.04" - configure_opts: "--with-libewf --with-libqcow --with-libvhdi --with-libvmdk --disable-java CFLAGS=\"-Wno-unused-command-line-argument -Werror\" CXXFLAGS=-Werror" + configure_opts: "--with-libewf --with-libqcow --with-libvhdi --with-libvmdk --disable-java CFLAGS=\"-Wno-unused-command-line-argument -Werror -Wno-error=unused-parameter\" CXXFLAGS=\"-Werror -Wno-error=unused-parameter\"" This is still not fixed. — Reply to this email directly, view it on GitHub <#3065 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMFHLBQFC4GZPOGILXBBSD2CSQQRAVCNFSM6AAAAABQVDU2J6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDINRSGEZDINJWGY>. You are receiving this because you authored the thread.

uckelman-sf · 2024-11-26T17:30:37Z

.github/workflows/build-unix.yml

@@ -37,7 +37,7 @@ jobs:
            linkage: "shared"
            compiler: "gcc"
            runner: "ubuntu-22.04"
-            configure_opts: "--with-libewf --with-libqcow --with-libvhdi --with-libvmdk --disable-java CFLAGS=\"-Wno-unused-command-line-argument -Werror\" CXXFLAGS=-Werror"
+            configure_opts: "--with-libewf --with-libqcow --with-libvhdi --with-libvmdk --disable-java CXXFLAGS=-Werror"


CFLAGS=-Werror should not be removed.

uckelman-sf · 2024-11-26T17:31:13Z

Makefile.am

@@ -2,7 +2,7 @@ ACLOCAL_AMFLAGS = -I m4

 AM_CPPFLAGS = -I$(srcdir)/tsk
 AM_CFLAGS = -Wall -Wextra
-AM_CXXFLAGS = -Wall -Wextra -Woverloaded-virtual
+AM_CXXFLAGS = -Wall -Wextra  -Woverloaded-virtual


Extra space.

simsong · 2024-11-26T17:48:52Z

got it.

…

On Nov 26, 2024, at 6:30 PM, Joel Uckelman ***@***.***> wrote: @uckelman-sf commented on this pull request. In .github/workflows/build-unix.yml <#3065 (comment)>: > @@ -37,7 +37,7 @@ jobs: linkage: "shared" compiler: "gcc" runner: "ubuntu-22.04" - configure_opts: "--with-libewf --with-libqcow --with-libvhdi --with-libvmdk --disable-java CFLAGS=\"-Wno-unused-command-line-argument -Werror\" CXXFLAGS=-Werror" + configure_opts: "--with-libewf --with-libqcow --with-libvhdi --with-libvmdk --disable-java CXXFLAGS=-Werror" CFLAGS=-Werror should not be removed. — Reply to this email directly, view it on GitHub <#3065 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMFHLBNVGH4PK3WSLX6RYD2CSV5HAVCNFSM6AAAAABQVDU2J6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDINRSGI3DIMRRGA>. You are receiving this because you authored the thread.

uckelman-sf · 2024-11-26T17:53:47Z

.github/workflows/build-unix.yml

@@ -49,7 +49,7 @@ jobs:
            linkage: "shared"
            compiler: "clang"
            runner: "ubuntu-22.04"
-            configure_opts: "--with-libewf --with-libqcow --with-libvhdi --with-libvmdk --disable-java CC=clang CXX=clang++ CFLAGS=-Werror CXXFLAGS=-Werror"
+            configure_opts: "--with-libewf --with-libqcow --with-libvhdi --with-libvmdk --disable-java CC=clang CXX=clang++ CFLAGS=-Werror CXXFLAGS=-Werror


This is missing the trailing double quote.

codecov · 2024-11-27T05:45:25Z

Codecov Report

Attention: Patch coverage is 80.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 19.18%. Comparing base (db89b4e) to head (69d8604).
Report is 58 commits behind head on develop.

Files with missing lines	Patch %	Lines
tsk/fs/fs_open.c	0.00%	2 Missing ⚠️
tsk/fs/dcat_lib.cpp	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3065      +/-   ##
===========================================
+ Coverage    16.50%   19.18%   +2.68%     
===========================================
  Files          145      148       +3     
  Lines        34107    36233    +2126     
===========================================
+ Hits          5627     6948    +1321     
- Misses       28480    29285     +805

Files with missing lines	Coverage Δ
tsk/fs/btrfs.cpp	`58.43% <ø> (ø)`
tsk/fs/btrfs_csum.cpp	`100.00% <100.00%> (ø)`
tsk/fs/fs_types.c	`14.81% <ø> (ø)`
tsk/fs/tsk_btrfs.h	`100.00% <100.00%> (ø)`
tsk/fs/tsk_fs.h	`0.00% <ø> (ø)`
tsk/fs/dcat_lib.cpp	`0.00% <0.00%> (ø)`
tsk/fs/fs_open.c	`30.22% <0.00%> (-0.44%)`	⬇️

... and 5 files with indirect coverage changes

simsong · 2024-11-27T07:22:47Z

Thanks to @uckelman-sf for all of your help!

uckelman-sf · 2024-11-27T11:18:24Z

Wait, this should not have been merged yet.

I didn't review the substantive parts of the code---I was waiting for all the build problems to be fixed before doing that.
How are we validating that any of this works? How was the test data produced?

simsong · 2024-11-27T11:20:10Z

Wait, this should not have been merged yet.

I didn't review the substantive parts of the code---I was waiting for all the build problems to be fixed before doing that.

How are we validating that any of this works? How was the test data produced?

Whoops.
I validated it on the disk image btrfs_testimage_50MB.E01

simsong · 2024-11-27T11:20:36Z

Wait, this should not have been merged yet.

I didn't review the substantive parts of the code---I was waiting for all the build problems to be fixed before doing that.

How are we validating that any of this works? How was the test data produced?

Whoops. I validated it on the disk image btrfs_testimage_50MB.E01

How do you want to proceed?

uckelman-sf · 2024-11-27T11:26:41Z

Whoops. I validated it on the disk image btrfs_testimage_50MB.E01

How? By hand? Against some expected results?

simsong · 2024-11-27T12:06:54Z

Whoops. I validated it on the disk image btrfs_testimage_50MB.E01

How? By hand? Against some expected results?

By hand. I created the test image and ran fiwalk on it and I got results that I expected.
Separately, I've been working with NIST to see if we can create a new forensic image generator that generates a disk image and creates a DFXML file at the same time, so that we can automate the process of creating novel complex images and then testing them.

uckelman-sf · 2024-11-27T16:35:41Z

Ok. I'd like to have a look through tsk/fs/btrfs.cpp and tsk/fs/tsk_btrfs.h when I'm back on Tuesday.

simsong · 2024-11-27T17:44:54Z

And we are excited to have you go through the code! We really appreciate the attention.

…

On Wed, Nov 27, 2024 at 5:36 PM Joel Uckelman ***@***.***> wrote: Ok. I'd like to have a look through tsk/fs/btrfs.cpp and tsk/fs/tsk_btrfs.h when I'm back on Tuesday. — Reply to this email directly, view it on GitHub <#3065 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMFHLACAFTAEVDSHQRFPG32CXYHJAVCNFSM6AAAAABQVDU2J6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMBUGMYTSOBZGI> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>

simsong mentioned this pull request Oct 26, 2024

Btrfs support #413

Closed

simsong marked this pull request as ready for review October 28, 2024 14:30

simsong requested review from bcarrier and jayaramcs October 28, 2024 14:31

simsong marked this pull request as draft October 29, 2024 13:26

simsong added a commit that referenced this pull request Oct 29, 2024

added btrfs_fuzzer targets per #3065 (comment)

1f84b56

uckelman reviewed Oct 29, 2024

View reviewed changes

uckelman reviewed Nov 2, 2024

View reviewed changes

simsong added 3 commits November 26, 2024 18:05

removed -Wno-error=overloaded-virtual

7ae7303

removed -Wno-error=overloaded-virtual

6818589

removed -Wno-error=overloaded-virtual

12978ca

uckelman-sf reviewed Nov 26, 2024

View reviewed changes

Makefile.am Outdated Show resolved Hide resolved

uckelman-sf reviewed Nov 26, 2024

View reviewed changes

Makefile.am Outdated Show resolved Hide resolved

simsong added 4 commits November 26, 2024 18:12

moved test inside #ifdef HAVE_LIBEWF

8650ee3

removed setting of CXXFLAGS to default value.

2584e0e

restored -Woverloaded-virtual to AM_CXXFLAGS

a66999c

changed -Wno-error=unused-parameter\" CXXFLAGS=\"-Werror -Wno-error=u…

d98b278

…nused-parameter\" to CXXFLAGS=-Werror"

uckelman-sf reviewed Nov 26, 2024

View reviewed changes

simsong added 2 commits November 26, 2024 18:46

removed extra spaces

afd5c8c

removed -Wno-error=unused-parameter

5f017e6

uckelman-sf reviewed Nov 26, 2024

View reviewed changes

simsong added 2 commits November 26, 2024 19:15

declared ignored parameters

4de8f2d

fixed typo

69d8604

simsong merged commit 9608dae into develop Nov 27, 2024
24 checks passed

simsong deleted the develop-btrfs branch November 27, 2024 07:22

uckelman-sf mentioned this pull request Nov 27, 2024

Reinstate accidentally removed -Werror #3114

Merged

		@@ -0,0 +1,27 @@
		AM_CPPFLAGS = -I../.. -I$(srcdir)/../.. -Wall

BTRFS from basic master #3065

BTRFS from basic master #3065

Conversation

simsong commented Oct 26, 2024 • edited Loading

simsong commented Oct 27, 2024

simsong commented Oct 28, 2024

joachimmetz commented Oct 28, 2024

simsong commented Oct 28, 2024

joachimmetz commented Oct 28, 2024

simsong commented Oct 28, 2024

joachimmetz commented Oct 29, 2024 • edited Loading

joachimmetz commented Oct 29, 2024

simsong commented Oct 29, 2024 via email

joachimmetz commented Oct 29, 2024

simsong commented Oct 29, 2024 via email

joachimmetz commented Oct 29, 2024

simsong commented Oct 29, 2024 via email

joachimmetz commented Oct 29, 2024

simsong commented Oct 29, 2024

simsong commented Oct 29, 2024

joachimmetz commented Oct 29, 2024

joachimmetz commented Oct 29, 2024

uckelman commented Oct 29, 2024

simsong commented Oct 29, 2024

simsong commented Oct 29, 2024 via email

uckelman Oct 29, 2024

Choose a reason for hiding this comment

simsong Oct 29, 2024

Choose a reason for hiding this comment

simsong commented Nov 2, 2024

uckelman Nov 2, 2024

Choose a reason for hiding this comment

simsong Nov 2, 2024

Choose a reason for hiding this comment

simsong commented Nov 26, 2024 via email

uckelman-sf Nov 26, 2024

Choose a reason for hiding this comment

uckelman-sf Nov 26, 2024

Choose a reason for hiding this comment

simsong commented Nov 26, 2024 via email

uckelman-sf Nov 26, 2024

Choose a reason for hiding this comment

codecov bot commented Nov 27, 2024 • edited Loading

Codecov Report

simsong commented Nov 27, 2024

uckelman-sf commented Nov 27, 2024

simsong commented Nov 27, 2024

simsong commented Nov 27, 2024

uckelman-sf commented Nov 27, 2024

simsong commented Nov 27, 2024

uckelman-sf commented Nov 27, 2024

simsong commented Nov 27, 2024 via email

simsong commented Oct 26, 2024 •

edited

Loading

joachimmetz commented Oct 29, 2024 •

edited

Loading

codecov bot commented Nov 27, 2024 •

edited

Loading