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

Seg. fault - Geeqie with 0.28.0 and certain .nef files #2638

Closed
caclark opened this issue May 28, 2023 · 22 comments · Fixed by #2665
Closed

Seg. fault - Geeqie with 0.28.0 and certain .nef files #2638

caclark opened this issue May 28, 2023 · 22 comments · Fixed by #2665
Assignees
Labels

Comments

@caclark
Copy link

caclark commented May 28, 2023

Describe the bug

When Geeqie is compiled with exiv2 0.28.0, viewing the exif data of certain files cause a crash. This occurs with .nef files from my camera, but not with downloaded test .nef files.
The fault does not occur with earlier versions of exiv2.

To Reproduce

Compile Geeqie with exiv2 0.28.0. Run and load the attached .nef file. Open the Exif Window.

Attached is a sample image from my camera (copyright free)
The stack trace after the seg. fault which includes some debug output prior to the crash
The source code of the two Geeqie files involved.

Desktop (please complete the following information):

Ubuntu 23.04
Exiv2 0.28.0 via meson wrap install exiv2
gcc version 12.2.0 (Ubuntu 12.2.0-17ubuntu1)
meson warning level 3

2019-12-23-11:21:44-DSC_3328.NEF.gz
exiv2.cc.gz
advanced-exif.cc.gz
exiv2-028-1-log.txt.gz

@caclark caclark added the bug label May 28, 2023
@postscript-dev
Copy link
Collaborator

@caclark
I downloaded your 2019-12-23-11:21:44-DSC_3328.NEF.gz file and also the official exiv2-0.28.0-Linux64.tar.gz binaries. I tested using Linux Mint 21.1 .

I examined your image using the exiv2 app and didn't find any problems. As the app is a basic front end for the Exiv2 library, it is a good way to test if we can correctly process a file's metadata. For example,

$ exiv2 --Print xkcyvt 2019-12-23-11:21:44-DSC_3328.NEF
0x00fe Exif.Image.NewSubfileType                    Long        1  1  Thumbnail/Preview image
0x0100 Exif.Image.ImageWidth                        Long        1  160  160
0x0101 Exif.Image.ImageLength                       Long        1  120  120
0x0102 Exif.Image.BitsPerSample                     Short       3  8 8 8  8 8 8
0x0103 Exif.Image.Compression                       Short       1  1  Uncompressed
0x0106 Exif.Image.PhotometricInterpretation         Short       1  2  RGB
0x010f Exif.Image.Make                              Ascii      18  NIKON CORPORATION  NIKON CORPORATION
0x0110 Exif.Image.Model                             Ascii      12  NIKON D3300  NIKON D3300
...

(see --Print flgs).

I don't know the Geeqie code but perhaps the recent conversion to Exiv2 0.28.0 still has problems?

I am sorry but I don't have more time to help with this issue.

@caclark
Copy link
Author

caclark commented May 29, 2023

Thanks for taking a look at this.

The crash is at line 3394 of nikonmn_int.cpp with tag Exif.NikonFl6.ExternalFlashData1.
This is new code introduced in 0.28.0.

I will do what I can to try to find the problem.

@postscript-dev
Copy link
Collaborator

@caclark

I will do what I can to try to find the problem.

Just out of interest, does Geeqie work when using the same binaries that I used?

@caclark
Copy link
Author

caclark commented May 30, 2023

Using the command line exiv2 <test image> (version: exiv2 0.28.0.9) shows the image exif data as expected.

I compiled Geeqie using meson wrap install exiv2 which shows Installed exiv2 version 0.28.0 revision 1

The two Geeqie source files involved have not had any changes other than type-casts (the project went from c to c++) and the changes necessary due to Exiv2 0.28.0.

@caclark
Copy link
Author

caclark commented Jun 3, 2023

The crash occurs also with file RAW_SONY_R1.SR2 from rawsamples.ch.
A stack dump is attached.

The crash occurs at line 702 of sonymn_int.cpp. That line is:
const auto pos = metadata->findKey(ExifKey("Exif.SonySInfo1.MetaVersion"));

And the line 3394 of nikonmn_int.cpp is
auto pos = metadata->findKey(ExifKey("Exif.NikonFl6.ExternalFlashData1"));

My assumption is that either exifMetadata_.begin() or exifMetadata_.end() is out of bounds.

I have no understand of c++, so if anyone has a suggestion of how I can further debug this, that would be good.

sony-log.txt.gz

@postscript-dev
Copy link
Collaborator

@caclark

The crash occurs also with file RAW_SONY_R1.SR2 from rawsamples.ch.

I downloaded the raw Sony file and tested it using the exiv2 app (with exiv2-0.28.0-Linux64.tar.gz). On Linux Mint 21.1 , I could read the file without issue:

$ exiv2 --Print xkycvt RAW_SONY_R1.SR2
0x00fe Exif.Image.NewSubfileType                    Long        1  1  Thumbnail/Preview image
0x0103 Exif.Image.Compression                       Short       1  6  JPEG (old-style)
0x010e Exif.Image.ImageDescription                  Ascii      32                                                                  
0x010f Exif.Image.Make                              Ascii       5  SONY  SONY
0x0110 Exif.Image.Model                             Ascii       7  DSC-R1  DSC-R1
0x0112 Exif.Image.Orientation                       Short       1  1  top, left
0x011a Exif.Image.XResolution                       Rational    1  72/1  72
0x011b Exif.Image.YResolution                       Rational    1  72/1  72
0x0128 Exif.Image.ResolutionUnit                    Short       1  2  inch
0x0132 Exif.Image.DateTime                          Ascii      20  2007:03:24 11:17:27  2007:03:24 11:17:27
...

At the moment, I'm not sure where the problem lies. To narrow things down, it would be helpful to know on Ubuntu 23.04:

  1. If the meson version of the exiv2 app (0.28.0) works with your test files.
  2. If the exiv2-0.28.0-Linux64.tar.gz version of the exiv2 app works with your test files.

As I said before, I'm sorry but I don't have the time to fully take on this issue.

@kevinbackhouse kevinbackhouse self-assigned this Jun 6, 2023
@caclark
Copy link
Author

caclark commented Jun 6, 2023

The exiv2 command line program, version 0.28.0, works with my test files.

I'm sorry, but the only way I can get exiv2 to compile is via meson, which shows this data:
meson wrap install exiv2
Installed exiv2 version 0.28.0 revision 1
meson setup -Ddevel=enabled --wrap-mode=forcefallback build
Downloading exiv2 source from https://github.com/Exiv2/exiv2/releases/download/v0.28.0/exiv2-0.28.0-Source.tar.gz

I made a primitive hack in exiv2/src/exif.cpp:
456 ExifData::const_iterator ExifData::findKey(const ExifKey& key) const {

by including:
printf("%p\n",(const void *)&exifMetadata_.end());
printf("%p\n",(const void *)&exifMetadata_.begin());
printf("0x%08x\n",exifMetadata_.end());
printf("0x%08x\n",exifMetadata_.begin());

When a problem tag is encountered, exifMetadata_.end() contains 0000000 and access to exifMetadata_.begin() causes a seg. fault.

I will continue to try to find out why.

@neheb
Copy link
Collaborator

neheb commented Jun 7, 2023

@caclark the meson version is unfortunately not as mature as the CMake one. When I tried adding tests, I couldn’t get them to pass.

As far as the wrap is concerned, try without it. Compile the CMake build and install it with sudo make install. meson should be able to pick it up then. If not, PKG_CONFIG_PATH needs to be set.

@caclark
Copy link
Author

caclark commented Jun 8, 2023

Thanks. After installing the cmake version I get:
Run-time dependency exiv2 found: YES 1.00.0.9
but the bug is still there.

@neheb
Copy link
Collaborator

neheb commented Jun 8, 2023

So the error is not meson. Great!

@caclark
Copy link
Author

caclark commented Jun 13, 2023

The Geeqie bug report BestImageViewer/geeqie#1105
also shows that Pentax DNG RAW files are a problem.

@caclark
Copy link
Author

caclark commented Jun 14, 2023

@kevinbackhouse
If I comment out lines 179 to 194 inclusive of exif.cpp, my test images run OK.

Naturally, I have no understanding of this piece of code - I just hope this information may help a little.

@neheb
Copy link
Collaborator

neheb commented Jun 14, 2023

That is…interesting. Guess extra debug code is needed to find out what fct is.

@kevinbackhouse
Copy link
Collaborator

I've figured out how to build geeqie so that I can debug this. Here's the relevant part of the stack trace:

  • 0x00007ffff6a7205f in Exiv2::Internal::Nikon3MakerNote::printFlashMasterDataFl6(std::ostream&, Exiv2::Value const&, Exiv2::ExifData const*) (os=..., value=..., metadata=0x0) at nikonmn_int.cpp:3394
  • 0x00007ffff696ce53 in Exiv2::Exifdatum::write(std::ostream&, Exiv2::ExifData const*) const (this=0x55555656ebd0, os=..., pMetadata=0x0) at exif.cpp:187
  • 0x00007ffff69ae0e4 in Exiv2::Metadatum::print[abi:cxx11](Exiv2::ExifData const*) const (this=0x55555656ebd0, pMetadata=0x0) at metadatum.cpp:12
  • 0x000055555561edb9 in exif_item_get_data_as_text(ExifItem*) (item=item@entry=0x55555656ebd0) at exiv2.cc:787

The problem is that exif_item_get_data_as_text (which is part of geeqie, not exiv2) calls metadatum->print() without an argument, so it defaults to nullptr. It ends up calling printFlashMasterDataFl6() which doesn't expect it's metadata argument to be null, leading to the crash.

I believe printFlashMasterDataFl6 is a new function in 0.28.0 so that's why this crash doesn't happen with earlier versions.

The thing that I'm not sure about is where the bug is. Are functions like printFlashMasterDataFl6 expected to be able to handle this NULL metadata case, or should a function higher up the call stack, such as Exifdatum::write, error out if the metadata is null?

@kevinbackhouse
Copy link
Collaborator

@caclark: I have created a PR which I think fixes this: BestImageViewer/geeqie#1119

@Piezoid
Copy link

Piezoid commented Jun 21, 2023

Gwenview on Arch is also affected. (gwenview 23.04.2-1, exiv2 0.28.0-2)

stack trace
#0  Exiv2::ExifData::findKey (this=0x0, key=...) at /usr/src/debug/exiv2/exiv2-0.28.0/src/exif.cpp:457
#1  0x00007ffff704225a in Exiv2::Internal::getModel(Exiv2::ExifData const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) [clone .constprop.0] (metadata=metadata@entry=0x0, 
    val="") at /usr/src/debug/exiv2/exiv2-0.28.0/src/sonymn_int.cpp:641
#2  0x00007ffff6f66735 in Exiv2::Internal::SonyMakerNote::printSonyMisc3cShotNumberSincePowerUp (os=..., value=..., metadata=0x0) at /usr/src/debug/exiv2/exiv2-0.28.0/src/sonymn_int.cpp:2059
#3  0x00007ffff6fc2c44 in Exiv2::Exifdatum::write (this=0x7fffc00ce840, os=..., pMetadata=0x0) at /usr/src/debug/exiv2/exiv2-0.28.0/src/exif.cpp:187
#4  0x00007ffff7dec1f4 in Exiv2::operator<< (md=..., os=...) at /usr/include/exiv2/metadatum.hpp:277
#5  Gwenview::ImageMetaInfoModelPrivate::fillExivGroup<Exiv2::ExifData, std::_List_const_iterator<Exiv2::Exifdatum> > (container=..., group=0x55555691cd70, parent=..., this=0x555556a17400)
    at /usr/src/debug/gwenview/gwenview-23.04.2/lib/imagemetainfomodel.cpp:282
#6  Gwenview::ImageMetaInfoModel::setExiv2Image (this=0x5555566aabe0, image=0x7fffc0001410) at /usr/src/debug/gwenview/gwenview-23.04.2/lib/imagemetainfomodel.cpp:441
#7  0x00007ffff7db0c21 in Gwenview::Document::setExiv2Image (this=0x555556560400, image=...) at /usr/src/debug/gwenview/gwenview-23.04.2/lib/document/document.cpp:388
#8  0x00007ffff7dbbd8e in Gwenview::AbstractDocumentImpl::setDocumentExiv2Image (this=0x555556825860, image=std::unique_ptr<Exiv2::Image> = {...})
    at /usr/src/debug/gwenview/gwenview-23.04.2/lib/document/abstractdocumentimpl.cpp:81
#9  Gwenview::LoadingDocumentImpl::slotMetaInfoLoaded (this=0x555556825860) at /usr/src/debug/gwenview/gwenview-23.04.2/lib/document/loadingdocumentimpl.cpp:497

The null dereference happens while reading .jpg files exported by Darktable from Sony ARWs. The function that fails to handle metadata=0x0 is printSonyMisc3cShotNumberSincePowerUp.
The workaround that worked for me was running exiv2 rm *.jpg in the export folder. (I don't care about metadata.)

@kevinbackhouse
Copy link
Collaborator

@postscript-dev: I think you added some of these functions like Nikon3MakerNote::printFlashMasterDataFl6. They're designed with the assumption that the metadata parameter is not NULL, which seems to be a valid assumption in exiv2 itself and all of our tests, but not in other applications in like geeqie. Do think it would be possible to make those functions more robust so that they don't crash like this?

@postscript-dev
Copy link
Collaborator

@postscript-dev: I think you added some of these functions like Nikon3MakerNote::printFlashMasterDataFl6. They're designed with the assumption that the metadata parameter is not NULL, which seems to be a valid assumption in exiv2 itself and all of our tests, but not in other applications in like geeqie. Do think it would be possible to make those functions more robust so that they don't crash like this?

@kevinbackhouse
Sorry for the slow reply. I don't have a lot of time but I will try and take a look at this over the weekend.

@neheb
Copy link
Collaborator

neheb commented Jun 24, 2023

@kevinbackhouse an alternative is to mark such functions with the nonnull attribute to add warnings to third party applications.

@Piezoid
Copy link

Piezoid commented Jun 24, 2023

The common ancestor in Geeqie and Gwenview stack traces is std::ostream& Exifdatum::write(std::ostream& os, const ExifData* pMetadata = nullptr). This signature strongly suggest that the pMetadata argument is optional, in agreement with this documentation block.
In fact, Gwenview uses the operator<< and can't provide the ExifData pointer that way.

@neheb
Copy link
Collaborator

neheb commented Jun 24, 2023

Hrm never mind then.

@rouelle
Copy link

rouelle commented Jul 17, 2023

Last update of geeqie in Arch Linux repo -- geeqie 2.1-1 -- runs now perfectly without crashing.

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

Successfully merging a pull request may close this issue.

6 participants