bugGNU Octave - Bugs: bug #59094, Possible integer overflow in...

 
 

bug #59094: Possible integer overflow in __ode15__.cc

Submitted by:  Markus Mützel <mmuetzel>
Submitted on:  Thu 10 Sep 2020 03:40:06 PM UTC  
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Segfault, Bus Error, etc.
Status:  Ready For Test Assigned to:  None
Originator Name:  Open/Closed:  Open
Release:  dev Operating System:  Any

Add a New Comment (Rich Markup)
   

You are not logged in

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

 

Thu 01 Oct 2020 06:31:21 AM UTC, comment #3: 

I pushed the change here:
https://hg.savannah.gnu.org/hgweb/octave/rev/8b0675c2cfe0

We could add additional overloads for char, unsigned char and signed char as we need them.
The same for the other operations (see comment #2).

Marking as ready for test.

Markus Mützel <mmuetzel>
Project Member
Sat 19 Sep 2020 11:24:30 AM UTC, comment #2: 

The attached patch adds functions that check whether multiplication of two "similar" integers would overflow. These functions wrap around macros provided by gnulib's `intprops` module.

It also adds the necessary check to `__ode15__.cc`.

The rules for integer multiplication between "dis-similar" integers and for function dispatching in C++ might differ (I haven't checked).
But imho it would make code easier to understand (and possibly avoid unintended behavior) if we explicitly casted to a common integer type before such kinds of operation anyway. So I don't think it is necessary to add functions that check integer overflow on operations between "dis-similar" types (like "int type" times "unsigned long int type").

If needed, we could add similar check functions for other potentially critical operations.
Checks are provided by gnulib for:

  • addition
  • subtraction
  • negation
  • multiplication
  • division
  • remainder
  • left shift

See also:
https://www.gnu.org/software/gnulib/manual/html_node/Integer-Type-Overflow.html#Integer-Type-Overflow

Adding jwe to CC because he made the original proposition.

(file #49819)

Markus Mützel <mmuetzel>
Project Member
Mon 14 Sep 2020 04:36:15 PM UTC, comment #1: 

With the change for bug #55905, the possible integer overflow is mitigated on systems with a version of SUNDIALS that provides "SUNSparseMatrix_Reallocate".

Markus Mützel <mmuetzel>
Project Member
Thu 10 Sep 2020 03:40:06 PM UTC, original submission:  

It is possible that the multiplication m_num*m_num in the following line in _ode15_.cc can overflow:

        m_sunJacMatrix = SUNSparseMatrix (m_num, m_num, m_num*m_num, CSC_MAT);

https://hg.savannah.gnu.org/hgweb/octave/file/0c9a5eae6c27/libinterp/dldfcn/__ode15__.cc#l404

jwe suggested in bug #58795:

> Gnulib has macros for checking whether integer operations overflow, or we could (conditionally) use things like __builtin_mul_overflow to multiply dimensions and check for overflow.
> I recently noticed that one of the Octave dependencies (maybe it was Sundials?) was using the portable snippets library for this job.  Perhaps we could use that or at least borrow ideas from it.  See, for example,  https://github.com/nemequ/portable-snippets/tree/master/safe-math
> I hope we can refactor so that we limit the number of places where checks like this are needed.


The gnulib macros are probably the ones in "intprops.h":
https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/intprops.h;h=df66a3877abbe9e563e2b285297e2702c8106adc;hb=HEAD

IIUC, we try to avoid including gnulib headers directly. Instead we have wrappers for the functions we want to use.
I don't know how to do something similar with these macros.
Is it ok to include that header directly?

Markus Mützel <mmuetzel>
Project Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #49819:  bug59094_ode15_integer_overflow.patch added by mmuetzel (10KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mmuetzel
  • -email is unavailable- added by mmuetzel (Submitted the item)
  •  

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

    Only project members can vote.

     

     

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    06:31 mmuetzel StatusPatch Submitted => Ready For Test
    2020-09-19 mmuetzel Attached File- => Added bug59094_ode15_integer_overflow.patch, #49819
        StatusNone => Patch Submitted
        Carbon-Copy- => Added jwe

    Back to the top


    Powered by Savane 3.5