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

 
 

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

Submitter:  Kris Thielemans <krthie>
Submitted:  Mon 02 Apr 2012 01:56:44 PM UTC
   
 
Category:  Libraries Severity:  3 - Normal
Priority:  3 - Low Item Group:  Segfault, Bus Error, etc.
Status:  Confirmed Assigned to:  None
Originator Name:  Kris Thielemans Open/Closed:  * Open
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 12 Jan 2024 10:21:20 PM UTC, comment #21: 

@nrjank: On Linux the command "which causesegfault" still definitely crashes Octave.

This bug is a pretty low priority since programmers writing bad mex/oct code are working outside of the interpreter and may crash the interpreter, but it is their fault.

Rik <rik5>
Group administrator
Tue 18 Apr 2023 05:11:33 PM UTC, comment #20: 

it's been a few years, this could use a reminder from those familiar with the issue what the remaining trigger case is.

using the attached test code, here's the current output i get. Can't trigger a crash. But can trigger an error with which:


>> mkoctfile --mex causesegfault.cpp
>> exist('causesegfault.mex','file')
ans = 3
>> exist('causesegfault.mex')
ans = 3
>> exist('causesegfault')
ans = 3
>> which causesegfault
will crash now
error: opening the library 'C:\temp\octave\causesegfault.mex' failed (error 1114): A dynamic link library (DLL) initialization routine failed.
>> causesegfault
will crash now
error: opening the library 'C:\Users\nicholas.jankowski\Desktop\temp\octave\causesegfault.mex' failed (error 1114): A dynamic link library (DLL) initialization routine failed.


so, even the intended segfault produces an error instead of crashing octave, but in any case I can't trigger it with exist. is there a better test case to use now?

Nicholas Jankowski <nrjank>
Group Member
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>
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>
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>
Group 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>
Group 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>
Group 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>
Group 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:


Update documentation of "exist" to warn of buggy .oct files (bug #36067).

* variables.cc (Fexist): Add warning to docstring about calling function
with a buggy .oct file.


Closing report.

Rik <rik5>
Group 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>
Sun 29 Jun 2014 06:43:43 PM UTC, comment #11: 

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


exist ("causesegfault")


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>
Group 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>
Group 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>
Group Member
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:


octave:28> mex existtest.c
octave:29> exist("existtest.bla", "file")
ans = 0
octave:30> exist("existtest.mex", "file")
ans =  2
octave:31> exist("existtest", "file")
ans =  3
octave:32> rename ("existtest.mex", "existtest.bla");
octave:33> exist("existtest.bla", "file")
ans =  2
octave:34> exist("existtest", "file")
ans = 0


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>
Group Member
Sun 15 Apr 2012 02:16:14 AM UTC, comment #6: 

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


error: caught execution error in library function


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


size_t pos = name.find ('.');

if (pos != std::string::npos && pos > 0)
  {
    struct_elts = name.substr (pos+1);
    symbol_name = name.substr (0, pos);
  }


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

Rik <rik5>
Group 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>
Group 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>
Group Member
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

#include <mex.h>
void mexFunction(int nlhs, mxArray *plhs[],int nrhs, const mxArray *prhs[])
{
    int dims[3]; /* I didn't bother to fill this in */
    plhs[0] = mxCreateNumericArray(3,dims,mxSINGLE_CLASS, mxREAL);
}

Now start octave 3.4.3, and type

mex existtest.c

Now start octave 3.6.1. Let's see what happens there:

octave:1> exist('existtest.bla','file')
error: caught execution error in library function


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.

octave:1> existtest
error: /home/kris/windows/Documents/devel/octave/test/existtest.mex: failed to load: liboctinterp.so.0: cannot open shared object file: No such file or directory




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:

#include "mex.h"

void mexFunction(int nlhs, mxArray *plhs[],int nrhs, const mxArray *prhs[])
{
  int * p = NULL;
  *p=2; /* assign data to null-pointer, causing seg fault */

}

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:


exist('myfile.bla','file')


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  :


  size_t pos = name.find ('.');
  if (pos != std::string::npos && pos > 0)
    {
      struct_elts = name.substr (pos+1);
      symbol_name = name.substr (0, pos);
    }
 octave_value val = safe_symbol_lookup (symbol_name);


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 Files:
   
   
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
  • -email is unavailable- added by nrjank (Posted a comment)
  • -email is unavailable- added by lachlan (Updated the item)
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by jordigh (Posted a comment)
  • -email is unavailable- added by krthie (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 20 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-01-12 rik5 StatusNeed Info Confirmed
    2023-04-18 nrjank Operating SystemGNU/Linux Any
    2023-04-18 nrjank StatusConfirmed Need Info
    2016-07-15 rik5 StatusReady 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
    2016-07-15 jwe StatusNone Ready For Test
    2016-07-15 jwe StatusWont Fix None
        Open/ClosedClosed Open
    2016-01-17 rik5 StatusConfirmed Wont Fix
        Open/ClosedOpen Closed
    2016-01-16 lachlan Attached File- Added bug_36067_docfix.cset, #36054
    2015-02-02 rik5 Priority5 - Normal 3 - Low
    2014-06-29 rik5 StatusReady For Test Confirmed
    2014-06-26 rik5 StatusConfirmed Ready For Test
    2014-06-26 rik5 Dependencies- bugs #42614 is dependent
    2013-08-25 mtmiller StatusWorks For Me Confirmed
        Release3.6.1 dev
    2013-08-10 krthie Attached File- Added causesegfault.cpp, #28806
    2013-07-28 mtmiller CategoryNone Libraries
        StatusNone Works For Me

    Back to the top

    Powered by Savane 3.13-cf05.
    Corresponding source code