bugGNU Octave - Bugs: bug #65944, octave-dicom: crash in test suite...

 
 

bug #65944: octave-dicom: crash in test suite with use-after-free on ubuntu 24.10

Submitter:  None
Submitted:  Wed 03 Jul 2024 07:04:29 PM UTC
   
 
Category:  Octave Package Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Segfault, Bus Error, etc.
Status:  Ready For Test Assigned to:  lostbard
Originator Name:  Daniel Bungert Originator Email:  -email is unavailable-
Open/Closed:  * Open Release:  * other
Operating System:  * GNU/Linux Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Tue 09 Jul 2024 01:27:32 PM UTC, comment #3: 

I retested the original, failing, build environment with change 185 and it is of course fine.  Thanks for the incorporating the change.

Daniel Bungert <dbungert>
Sat 06 Jul 2024 02:18:11 PM UTC, comment #2: 

I pushed a change up to the repo that includes some additional variables in that file being static as well.


It will appear in the next release of octave  dicom

John Donoghue <lostbard>
Group Member
Thu 04 Jul 2024 07:15:42 AM UTC, comment #1: 

CC'ing package maintainers listed on https://gnu-octave.github.io/packages/dicom/

Markus M├╝tzel <mmuetzel>
Group administrator
Wed 03 Jul 2024 07:04:29 PM UTC, original submission:  

Other bug reference URLs:

https://bugs.launchpad.net/debian/+source/octave-dicom/+bug/2069660
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1075721

Greetings!

Recent attempts to build octave-dicom on Ubuntu 24.10 resulted in test suite failures, with a use-after-free crash.

Tracing with valgrind shows that the std::map destructors are performing an overwriting clear operation during cleanup, but I don't think this is a std::map bug.

dicomdict uses std::map and contains them globally in non-static storage.  Also, dicomdict.o is built into multiple .oct files provided by octave-dicom and I believe this is what causes the problem - at cleanup time, the duplicated RTLD_GLOBAL symbols may not be being run in the correct fashion.

For Ubuntu I have modified octave-dicom as such (also attached):

--- a/src/dicomdict.cpp
+++ b/src/dicomdict.cpp
@@ -51,9 +51,9 @@
 const char * factory_dicom_dict_filename="octavedicom.dic";
 static std::string dic_filename(factory_dicom_dict_filename);

-std::map<gdcm::Tag, std::string> tagmap ;
-std::map<std::string, gdcm::Tag> keymap ;
-std::map<std::string, gdcm::DictEntry> dict ;
+static std::map<gdcm::Tag, std::string> tagmap ;
+static std::map<std::string, gdcm::Tag> keymap ;
+static std::map<std::string, gdcm::DictEntry> dict ;

 void insert(const char *k, const gdcm::Tag t, const gdcm::DictEntry e)
 {


With this modification the test suite passes.  I also played with dicomdict/other plugins to see if they behave correctly and everyone seems fine with the modification in my scenarios (load dicomdict before or after other plugins that use dicomdict).

Another option I explored was to no longer build dicomdict.o into the relevant *.oct files, but I don't know much about Octave and it's not clear to me if some sort of plugin ordering can be enforced to ensure that dicomdict is loaded before the others that require it.

I expect that this is not limited to a test suite problem and may similarly cause programs that use octave-dicom to crash on exit.

Anonymous

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #56221:  dicomdict-use-after-free.patch added by None (970B - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by dbungert (Posted a comment)
  • -email is unavailable- added by lostbard (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by mmuetzel
  • -email is unavailable- added by mmuetzel
  • -email is unavailable- added by None (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 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-07-06 lostbard StatusNone Ready For Test
        Assigned toNone lostbard
    2024-07-04 mmuetzel Carbon-Copy- Added lostbard
        Carbon-Copy- Added blondandy
    2024-07-03 None Attached File- Added dicomdict-use-after-free.patch, #56221

    Back to the top

    Powered by Savane 3.13-b921.
    Corresponding source code