bugGNU Octave - Bugs: bug #50974, SEGFAULT with constructed...

 
 

bug #50974: SEGFAULT with constructed cmdline_options

Submitter:  Steven Bergner <steb>
Submitted:  Mon 08 May 2017 09:12:17 PM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Segfault, Bus Error, etc.
Status:  Confirmed Assigned to:  None
Originator Name:  Steven Open/Closed:  * Open
Release:  * 4.2.1 Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Tue 09 May 2017 04:08:18 AM UTC, comment #5: 

jwe - I guess I would expect a user program that processes some command line options to also adjust argc and argv before passing them in to the application object.

Let's say I write a program that takes its own arguments, followed by a "--", followed by Octave interpreter options.

I would probably use getopt_long, finish handling arguments at the "--", and then create an instance of cli_application, passing in argc-idx and &argv[idx]. This way the value of optind doesn't have to be a precious value that is preserved between my call to getopt_long and Octave's call to getopt_long. And Octave should be able assume that it will start processing at the beginning of the argv that it was given, so I think it should ensure that optind is exactly 1 in its own getopt_long loop.

Mike Miller <mtmiller>
Group Member
Tue 09 May 2017 02:54:35 AM UTC, comment #4: 

Sorry, I was wrong about the no-argument constructor cmdline_options() that one is safe and not affected by optind.

A more valid point would have been that any documentation on how to pass in arguments should not just go with cmdline_options, but also with the application class, because it consumes the input arguments that are then processed as discussed.

Steven Bergner <steb>
Tue 09 May 2017 01:59:46 AM UTC, comment #3: 

Documenting this behavior on doxyhtml and in the embedded interpreter example code, would certainly help and people, who want to use octave as a library, would probably find the hint there.

Note, even people who create an application without arguments,  invoking the default constructor

application::application (const cmdline_options& opts = cmdline_options ());

e.g., when instantiating a derived application class, may have to reset optind, before they do so.

I'm not sure what to recommend for resolving this, as I've only just learned about optind, when investigating this.

Steven Bergner <steb>
Tue 09 May 2017 12:07:09 AM UTC, comment #2: 

My idea for cmdline_options was that it would be constructed either directly from the argv that is passed to a program, or by creating a default cmdline_options object and then setting the options using the member functions like, like this:


  cmdline_options opts;
  opts.read_history_file (true);
  ...


but I can also see how it could make sense to pass your own generated argv array to the "cmdline_options (int, char**)" constructor.  Since that is processed with getopt, how can we ensure that it works properly other than to expect the user to do what Mike suggests?  I don't think we can unconditionally set optind inside the cmdline_options constructor because the user may have already processed some options before calling the cmdline_options constructor.

Maybe we should just document this requirement instead.

John W. Eaton <jwe>
Group administrator
Mon 08 May 2017 10:32:30 PM UTC, comment #1: 

This seems accurate to me. I have not tried to reproduce but seems easy to demonstrate by writing a main function that calls getopt and then instantiating an Octave application.

The GNU C library says

> The variable optind is the index of the next element to be
> processed in argv. The system initializes this value to 1. The
> caller can reset it to 1 to restart scanning of the same argv,
> or when scanning a new argument vector.


So this can be worked around on the caller side by resetting the global optind variable to 1 after they are done with their own calls to getopt or getopt_long from the real argv.

But ideally we will reset optind to 1 in Octave's command line parser because it has no idea where argv came from or whether getopt had been called already.

Mike Miller <mtmiller>
Group Member
Mon 08 May 2017 09:12:17 PM UTC, original submission:  

There is a potential for SEGFAULT when using Octave as library via embedded interpreter inside an application that has its own command line arguments, independent of the arguments that are passed to some internal octave::cli_application instance.

The bug reaches back a number of versions and is still present in v4.3.0+.

In file libinterp/octave.cc
the function
cmdline_options::cmdline_options (int argc, char **argv)
constructs an object like this
m_remaining_args = string_vector (argv+octave_optind_wrapper (),
argc-octave_optind_wrapper ())

Here, argv may be a user-constructed char**, whereas octave_optind_wrapper() returns the optind integer from getopt.h, which reports the number of command line arguments of the actual application.

When using Octave as a library, this constructed address may point to invalid memory and a SEGFAULT is triggered.

Observation and mitigation of this bug is documented here:
https://github.com/renozao/RcppOctave/pull/17/commits/7cdb1d4f0b6253920869ac74709554fc822cac96
related commit msg: "
- octave.cc (v4.2, 4.3+) cmdline_options::cmdline_options() uses
  argv + octave_optind_wrapper(), where argv is the char** produced
  in rcpp_octave.cpp octave_session and optind is a global int
  giving the number of arguments getopt.h reports for the executable,
  having nothing to do with the args produced here. If the executable
  was called with more than narg arguments, this gives a SEGFAULT.
  resolved: octave_session now ensures that char** is long enough
"
with corresponding code changes in function octave_session in src/rcpp_octave.cpp of https://github.com/git-steb/RcppOctave/tree/develop

Thanks for keeping up this great project!

Steven
https://github.com/git-steb

Steven Bergner <steb>

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by steb (Submitted the item)
  • -email is unavailable- added by steb
  •  

    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 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-05-08 mtmiller StatusNone Confirmed
    2017-05-08 steb Carbon-Copy- Added steb

    Back to the top

    Powered by Savane 3.13-cf05.
    Corresponding source code