bugGNU Octave - Bugs: bug #36067, faulty mex/oct file with bad...

 
 

bug #36067: faulty mex/oct file with bad pointer assignments can crash Octave

Submitted by:  Kris Thielemans <krthie>
Submitted on:  Mon 02 Apr 2012 01:56:44 PM UTC  
 
Category: LibrariesSeverity: 3 - Normal
Priority: 3 - LowItem Group: Segfault, Bus Error, etc.
Status: ConfirmedAssigned to: None
Originator Name: Kris ThielemansOpen/Closed: Open
Release: devOperating System: GNU/Linux

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Fri 15 Jul 2016 11:44:41 PM UTC, comment #19:

Oh. I just saw JWE's comment at bug #36067 saying signal handlers shouldn't throw exceptions. Please ignore comment #18.

Lachlan Andrew <lachlan>
Project Member
Fri 15 Jul 2016 11:41:31 PM UTC, comment #18:

One possibility to reduce the severity of problems with segfaults in mex/oct files is to make the SIGSEGV handler throw and exception, and surround calls to the mex file by try/catch.

If an exception is caught, then a useful error message could be printed saying which mex file was to blame, and possibly giving the user the opportunity to debug a little. It should warn the user that the system has become unstable and should be restarted soon.

The same could be applied to known bugs with other libraries, like the graphics drivers.

Lachlan Andrew <lachlan>
Project Member
Fri 15 Jul 2016 10:28:59 PM UTC, comment #17:

I verified jwe's fix.

I've re-titled the bug report to reflect the remaining issue, which is indeed difficult to resolve.

Rik <rik5>
Project Administrator
Fri 15 Jul 2016 04:49:37 PM UTC, comment #16:

Oops, I hit the submit button a little early...

This changeset, now pushed:

http://hg.savannah.gnu.org/hgweb/octave/rev/5f91c1e89a98

John W. Eaton <jwe>
Project Administrator
Fri 15 Jul 2016 04:47:53 PM UTC, comment #15:

I pushed the following changeset:

I think this avoids the crash when calling exist, though of course actually executing a function like causessegfault.mex will crash Octave. I don't think that is something we will be fixing any time soon, but if someone has an idea about how to do that efficiently rand reliably, go ahead and propose a solution.

John W. Eaton <jwe>
Project Administrator
Fri 15 Jul 2016 03:38:23 PM UTC, comment #14:

I'm reopening this because I don't think it should be possible for Octave to crash just because you call exist, no matter what the argument.

If exist is somehow executing a .mex or .oct file, then I think that's a bug.

John W. Eaton <jwe>
Project Administrator
Sun 17 Jan 2016 03:52:05 PM UTC, comment #13:

@Lachlan: I agree that this is a very difficult bug to trigger and that it is better to document it, than to jump through hoops to fix it.

I checked in your documentation fix here (http://hg.savannah.gnu.org/hgweb/octave/rev/d6fa72047aeb). I changed the commit message slightly. We place only the affected function name in parentheses; everything else goes into the commit message after the ':'. So I changed "(docstring in Fexist)" in the following manner:

Closing report.

Rik <rik5>
Project Administrator
Sat 16 Jan 2016 02:00:20 AM UTC, comment #12:

I think that anyone writing C++ code with that causes a segfault should experience a crash. That is a developer's issue -- Octave isn't an operating system.

The only question is whether it is feasible to do "exist" without loading the oct/mex file.

Given Rik's partial fix, I recommend that the documentation of "which" be updated (draft attached), and this bug be marked "Won't fix" and closed.

(file #36054)

Lachlan Andrew <lachlan>
Project Member
Sun 29 Jun 2014 06:43:43 PM UTC, comment #11:

Further testing shows that things are improved, but the case of

still produces a segfault. It seems like we would need to go to great lengths to save/restore a sighandler routine before/after calling safe_symbol_lookup in order to get this to work.

Rik <rik5>
Project Administrator
Thu 26 Jun 2014 04:36:03 PM UTC, comment #10:

I checked in a changeset on the development branch that appears to fix the problem (http://hg.savannah.gnu.org/hgweb/octave/rev/a0fd65914811). Please test and close the report if this has been solved.

Rik <rik5>
Project Administrator
Sun 25 Aug 2013 01:05:51 PM UTC, comment #9:

Thanks for the new test case, segfault confirmed with this one on the dev branch. Yes, Octave is trying to load the named mex/oct file into the symbol table if the name matches.

Your suggestion about bypassing the symbol table lookup if an extension was given may be worth pursuing. I think the entire block that handles the symbol table search results will fall through if the given string looks like it has a file extension.

So to be clear, are you looking for a fix to avoid a crash when a string is given that happens to have the same base name as a buggy mex/oct function, exist ("causesegfault.bla"), or even exist ("causesegfault.oct"), but you don't have a problem with a crash on exist ("causesegfault")?

Mike Miller <mtmiller>
Project Administrator
Sat 10 Aug 2013 08:58:09 PM UTC, comment #8:

I have checked this with the current check-out, and indeed I see no problem with my previous test case.

However, I can create the same problem with a different mex file, see new attachment causesegfault.cpp. The difference here is that the (intentional) segmentation fault is caused by the initialisation of a static global variable. This code has to be run before any mex function, so it makes sense that it is executed when loading the mex file.

Clearly, this mex file is faulty, but it is used here as illustration of strange behaviour of exist().

With this mex file in the octave path, executing the following octave gives

octave:1> exist("causesegfault.bla","file")
will crash now
panic: Segmentation fault -- stopping myself...
Segmentation fault

I did compile octave with -g this time around, and can confirm that the stack-trace shows the crash occurs in the call to safe_symbol_lookup in symbol_exist (in libinterp/corefcn/variables.cc). This is called with argument "causesegfault" (because everything after the "." is stripped first as Rik said). (There's nothing wrong with safe_symbol_lookup in my opinion. Somewhere down the line it will call dlopen on the mex file, which will then cause the static variables to be initialised).

So... summary of all this. exist("arg.ext", "file") will load arg.mex or arg.oct, and hence initialise things inside the mex/oct file.

This is certainly surprising to me. It also causes a performance penalty as exist("arg.ext","file") really shouldn't be looking for anything with "arg" at all.

I'm not sure if you think this is worth fixing but I see 2 possible solutions:

- put the code testing for files first if the 2nd argument is "file". However, this is only a partial solution as it would still have the same problem when just using exist("causesegfault.bla")

- at the start of the symbol_exist function, before simply removing the extension, we could check if the extension is "m","mex" or "oct" (ignoring case?). If it isn't, we can skip everything involving symbol_name. I think this should be alright and be backwards compatible (at least, for intended use cases), but am not 100% sure.

(file #28806)

Kris Thielemans <krthie>
Sun 28 Jul 2013 04:31:38 PM UTC, comment #7:

I don't see any problem with the development version of Octave, unless I am missing something:

What exactly is the supposed bug with exist in this scenario? I'm ignoring anything about mixing versions of Octave because that's an incorrect use case to begin with.

Mike Miller <mtmiller>
Project Administrator
Sun 15 Apr 2012 02:16:14 AM UTC, comment #6:

I don't get any segfault, but I do get the reported error

when executing the sample code. For reference, the exist function in src/variables.cc calls symbol_exist () and that has the following:

This means the extension '.bla' is not used initially when Octave attempts to find a symbol match.

Rik <rik5>
Project Administrator
Tue 03 Apr 2012 08:37:32 AM UTC, comment #5:

strange. I'm using Ubuntu 11.10. Compiled Octave from source (both 3.4.3 and 3.6.1-rc0 (according to the mercurial tag but I checked that variables.cc hasn't changed since then).

Can you at least reproduce the fact that after compiling with 3.4.3, calling the mex function in 3.6.1 doesn't find the shared library? This isn't a bug of course, but if you do happen to have the shared library in your LD_LIBRARY_PATH or so, then exist() will never complain. Clutching at straws here...

Kris Thielemans <krthie>
Mon 02 Apr 2012 05:07:14 PM UTC, comment #4:

I'm unable to reproduce this problem.

John W. Eaton <jwe>
Project Administrator
Mon 02 Apr 2012 03:43:55 PM UTC, comment #3:

It's not usually important to figure out which precise nasal demon you're invoking. Suffice it to say that something crashed and it probably shouldn't have crashed.

Jordi GutiƩrrez Hermoso <jordigh>
Project Administrator
Mon 02 Apr 2012 03:31:49 PM UTC, comment #2:

ok. I can currently not cause a crash, but definitely cause strange behaviour.

Here's a simple (non-sensical) mex file called existtest.c
Now start octave 3.4.3, and type
Now start octave 3.6.1. Let's see what happens there:

This shows that exist() actually did try to do something with existtest, even though I'm asking for existtest.bla.

FYI, the error that was caught is a shared library problem caused by compiling a mex file with 3.4.3, but using it in 3.6.1.

Kris Thielemans <krthie>
Mon 02 Apr 2012 02:18:11 PM UTC, comment #1:

This needs more investigation. I found this bug when using a mex file compiled in octave 3.4.3, which was thus relying on a shared library which no longer exists. However, I now tried to reproduce this with a simple mex file as follows:
but exist has no problems with this file.

I am recompiling octave 3.4.3 now to see if I can reproduce my original problem.

Kris Thielemans <krthie>
Mon 02 Apr 2012 01:56:44 PM UTC, original submission:

I've just upgraded to 3.6.1 and discovered the following:

exits octave with "panic: Segmentation fault -- stopping myself..." IF myfile is actually a mex file (or presumably oct) file that causes a segmentation fault. Note that this happens independent of the extension used (i.e. you don't have to check for 'myfile.mex' or so).

This indicates that exist() actually tries to execute the function, which sounds really rather dangerous.

I've looked at the C++ source for exist() in variables.cc. This calls symbol_exist(). I don't know exactly where in this function it executes the mex file (sorry, I didn't compile a debug version), but I am puzzled by the fact that this function has special handling for structs in the string :

This is probably necessary for some use cases of symbol_exist (but which?) but definitely not when the type is 'file' (and many others).

Kris Thielemans <krthie>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #36054:  bug_36067_docfix.cset added by lachlan (1KiB - application/octet-stream)
file #28806:  causesegfault.cpp added by krthie (472B - text/plain)

 

Depends on the following items: None found

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -unavailable- added by lachlan (Updated the item)
  • -unavailable- added by mtmiller (Posted a comment)
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by jordigh (Posted a comment)
  • -unavailable- added by krthie (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 17 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 15 Jul 2016 10:28:59 PM UTCrik5StatusReady For Test=>Confirmed
      Summarybug in exist() when checking for a faulty mex/oct file=>faulty mex/oct file with bad pointer assignments can crash Octave
    Fri 15 Jul 2016 04:49:37 PM UTCjweStatusNone=>Ready For Test
    Fri 15 Jul 2016 03:38:23 PM UTCjweStatusWont Fix=>None
      Open/ClosedClosed=>Open
    Sun 17 Jan 2016 03:52:05 PM UTCrik5StatusConfirmed=>Wont Fix
      Open/ClosedOpen=>Closed
    Sat 16 Jan 2016 02:00:20 AM UTClachlanAttached File-=>Added bug_36067_docfix.cset, #36054
    Mon 02 Feb 2015 05:14:41 AM UTCrik5Priority5 - Normal=>3 - Low
    Sun 29 Jun 2014 06:43:43 PM UTCrik5StatusReady For Test=>Confirmed
    Thu 26 Jun 2014 04:36:03 PM UTCrik5StatusConfirmed=>Ready For Test
    Thu 26 Jun 2014 03:58:47 PM UTCrik5Dependencies-=>bugs #42614 is dependent
    Sun 25 Aug 2013 01:05:51 PM UTCmtmillerStatusWorks For Me=>Confirmed
      Release3.6.1=>dev
    Sat 10 Aug 2013 08:58:09 PM UTCkrthieAttached File-=>Added causesegfault.cpp, #28806
    Sun 28 Jul 2013 04:31:38 PM UTCmtmillerCategoryNone=>Libraries
      StatusNone=>Works For Me

    Back to the top


    Powered by Savane 3.1-cleanup1