Bug #1164

exiv2 tool crash with bad canon raw file

Added by Nicolas THOMASSON about 1 year ago. Updated about 1 year ago.

Status:ClosedStart date:02 Mar 2016
Priority:NormalDue date:
Assignee:Robin Mills% Done:

100%

Category:metadataEstimated time:2.00 hours
Target version:0.26

Description

Running exiv2 tool with the attached bad file crashes. This file was corrupted by an interrupted copy.

exiv2 version : 0.25 001900 (64 bit build)

$ exiv2 -v crw_3888.crw
File 1/1: crw_3888.crw
Segmentation fault (core dumped)

Originally I had this problem in dikikam. Someone at Digikam was able to generate this backtrace and therefore we think it comes from the exiv2 library :

Thread 1 (Thread 0x7f54e39bda40 (LWP 10870)):
[KCrash Handler]
#6 0x00007f55098f1718 in Exiv2::getUShort(unsigned char const*, Exiv2::ByteOrder) (buf=buf@entry=0x7f55d4019019 <error: Cannot access memory at address 0x7f55d4019019>, byteOrder=byteOrder@entry=Exiv2::littleEndian) at types.cpp:236
#7 0x00007f5509855fc4 in Exiv2::Internal::CiffDirectory::readDirectory(unsigned char const*, unsigned int, Exiv2::ByteOrder) (this=0x1d89e90, pData=0x7f54d401901a "", size=1277926, byteOrder=Exiv2::littleEndian) at crwimage.cpp:452
#8 0x00007f5509856327 in Exiv2::Internal::CiffHeader::read(unsigned char const*, unsigned int) (this=this@entry=0x1d89e60, pData=pData@entry=0x7f54d4019000 "II\032", size=size@entry=1277952) at crwimage.cpp:391
#9 0x00007f5509856481 in Exiv2::CrwParser::decode(Exiv2::CrwImage*, unsigned char const*, unsigned int) (pCrwImage=pCrwImage@entry=0x1d89d90, pData=0x7f54d4019000 "II\032", size=size@entry=1277952) at crwimage.cpp:177
#10 0x00007f55098566cd in Exiv2::CrwImage::readMetadata() (this=0x1d89d90) at crwimage.cpp:134
#11 0x00007f550cf59e75 in Digikam::MetaEngine::load(QString const&) const (this=0x7ffc08993850, filePath=...) at /home/gilles/Devel/5.x/core/libs/dmetadata/metaengine.cpp:283

Please ask if you need the original uncorrupted file.

crw_3888.crw (1.22 MB) Nicolas THOMASSON, 02 Mar 2016 08:56

exiv2.cpp Magnifier - Added a signal handler (50 KB) Nicolas THOMASSON, 02 Mar 2016 13:35

CMakeLists.txt Magnifier - Quick and dirty modification of the flags (10.2 KB) Nicolas THOMASSON, 02 Mar 2016 13:35

Associated revisions

Revision 4210
Added by Robin Mills about 1 year ago

#1164. Fix submitted. Thank you Nicolas and Gilles for reporting this. More comments in the issue report.

History

#1 Updated by Robin Mills about 1 year ago

  • Category set to not-a-bug
  • Status changed from New to Closed
  • Assignee set to Robin Mills
  • Target version set to 0.26
  • % Done changed from 0 to 100
  • Estimated time set to 1.00

Nothing can be done about this. DigiKam has to use an interrupt handler to catch this situation.

#2 Updated by Nicolas THOMASSON about 1 year ago

Hi,

I still think there is something wrong somewhere because the exiv2 tool crash as well (and it is not related to digikam). Please correct me if I am wrong but there is likely something to correct somewhere, either in exiv2 executable or either in the exiv2 library.

Nicolas

#3 Updated by Robin Mills about 1 year ago

If you want something done about this, please provide a patch for me to review.

#4 Updated by Gilles Caulier about 1 year ago

  • Category changed from not-a-bug to metadata

Of course it's a bug. Try Exiv2 CLI tool with corrupted CR2 file, and lib crash.

digiKam already has the C++ exception handler everywhere to wrap Exiv2 dysfunctions. This is not a DK bug, but an Exiv2 one...

[gilles@localhost BugDigikamRaw]$ exiv2 --version
exiv2 0.25 001900 (64 bit build)
Copyright (C) 2004-2015 Andreas Huggel.

This program is free software; you can redistribute it and/or
modify it under the terms of the GNU General Public License
as published by the Free Software Foundation; either version 2
of the License, or (at your option) any later version.

This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.

You should have received a copy of the GNU General Public
License along with this program; if not, write to the Free
Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
Boston, MA 02110-1301 USA
[gilles@localhost BugDigikamRaw]$ exiv2 -v crw_3888.crw
File 1/1: crw_3888.crw
Segmentation fault (core dumped)
[gilles@localhost BugDigikamRaw]$

#5 Updated by Robin Mills about 1 year ago

What does your exception handler not catch it?

#6 Updated by Nicolas THOMASSON about 1 year ago

I'm neither involved in digikam or exiv and I'm not an expert of dealing with exception but from my point of view this is a memory fault signal and not a standard c++ exception.

Natively it seems not possible to catch that signal by a try/catch mechanism unless maybe some compilation flags. For instance it is possible to compile with -fnon-exception-call with gcc which allows to throw exception inside a signal handler.

I read this and try to use it on exiv2 cli : http://angasule.blogspot.fr/2011/09/catching-null-pointer-exceptions-with.html

If I modify exiv2.cpp this way

#include <signal.h>

static void handler(int sig, siginfo_t *dont_care, void *dont_care_either) {
throw(Exiv2::BasicError<char>(-1));
}

int main(int argc, char* const argv[]) {

struct sigaction sa;

memset(&sa, 0, sizeof(sa));
sigemptyset(&sa.sa_mask);
sa.sa_flags     = SA_NODEFER;
sa.sa_sigaction = handler;
sigaction(SIGSEGV, &sa, NULL); /*  ignore whether it works or not */ 

}

I get this result :

$ ../bin/exiv2 ~/Dropbox/BugDigikamRaw/crw_3888.crw
Exiv2 exception in print action for file /home/thm/Dropbox/BugDigikamRaw/crw_3888.crw:
Error -1: arg2=%2, arg3=%3, arg1=%1.

This time the signal is caught and threw back to the application.

I don't know the impact of this solution and if it is the right one (for portability reason).

Nicolas

#7 Updated by Robin Mills about 1 year ago

  • Status changed from Closed to Assigned
  • % Done changed from 100 to 50
  • Estimated time changed from 1.00 to 2.00

Ah, your determination to make me fix this has prevailed. I'll fix this tonight.

#8 Updated by Robin Mills about 1 year ago

Nicolas and Gilles

Thanks for pushing me into this. Apologies to Nicolas and Gilles for rejecting this today. You are right, the library should never crash.

I've added a fix in the code - r4210

The blog found by Nicolas is interesting. I was aware that some exceptions can escape try/catch and now I've learned something about why. We had already had a trap in the code for this, however it required a little hardening to avoid an arithmetic overflow. Software is tricky stuff.

I'm hesitant to add a Linux signal handler to our code. This is really a process mechanism and should go into our sample applications. Putting a signal handler into the exiv2 library doesn't feel good. We could obliterate/defeat signal handlers installed by applications. Comments Welcome.

#9 Updated by Nicolas THOMASSON about 1 year ago

Hello Robin,

You are right, signal management is application responsibility (digikam, exiv2 cli,...) and should not be part of libraries.

I sent you this link because I thought there was no way to prevent the segmentation fault other than handling the signal.

Since there is a classical way to prevent the fault, I strongly prefer the kind of correction you made in this patch. Using signal handler should only be used in last resort or inside the final application for safety/specific reasons.

Thanks for the patch,

Nicolas

#10 Updated by Robin Mills about 1 year ago

  • Status changed from Assigned to Closed
  • % Done changed from 50 to 100

I think we're done with this. The article here is interesting http://angasule.blogspot.co.uk/2011/09/catching-null-pointer-exceptions-with.html Perhaps Gilles/DigiKam should consider adding a null pointer exception handler.

Also available in: Atom PDF

Redmine Appliance - Powered by TurnKey Linux