Thank you so much for that confirmation. After finding this issue I tested with exactly that fix and that seems to avoid the crash. I'll create a PR.
/D > On Jan 10, 2025, at 14:55, Berthold Stoeger via subsurface > <subsurface@subsurface-divelog.org> wrote: > > PS: Same bug seems to be in > mobile-widgets/statsmanager.cpp > stats/chartlistmodel.cpp > stats/statsstate.cpp > > On Samstag, 11. Jänner 2025 05:52:11 WIB Berthold Stoeger via subsurface > wrote: > > The +1 is OK, since the resize() calls also have a +1: > > > > out.stats_by_depth.resize((STATS_MAX_DEPTH / STATS_DEPTH_BUCKET) + 1); > > out.stats_by_temp.resize((STATS_MAX_TEMP / STATS_TEMP_BUCKET) + 1); > > > > but the std::clamp() are not OK, because the interval is inclusive. They > > should read > > > > d_idx = std::clamp(d_idx, 0, STATS_MAX_DEPTH / STATS_DEPTH_BUCKET - 1); > > t_idx = std::clamp(t_idx, 0, STATS_MAX_TEMP / STATS_TEMP_BUCKET - 1); > > > > Berthold > > > > On Samstag, 11. Jänner 2025 00:53:06 WIB Dirk Hohndel via subsurface wrote: > > > Hmm. That's weird code: > > > int d_idx = dp->maxdepth.mm / (STATS_DEPTH_BUCKET * 1000); > > > d_idx = std::clamp(d_idx, 0, STATS_MAX_DEPTH / > > > STATS_DEPTH_BUCKET); > > > process_dive(*dp, out.stats_by_depth[d_idx + 1]); > > > > > > So we make sure that d_idx is an allowable index value, and then add 1 to > > > it... I could see how that might access beyond the end of the array. > > > > > > /D > > > > > > > On Jan 10, 2025, at 09:46, Christof Arnosti via subsurface > > > > <subsurface@subsurface-divelog.org> wrote: > > > > > > > > Check the passed stats_t reference, this is the structure that is read > > > > there. I guess the pointer passed there is outside of the > > > > out.stat_by_depth array. The field "total_average_depth_time" is quite > > > > at > > > > the beginning of this structure, thus the 8 byte offset makes sense. > > > > > > > > Best > > > > Christof > > > > > > > > On 10.01.25 18:33, Dirk Hohndel via subsurface wrote: > > > >> I ended up using the ASAN build we already have configured in cmake: > > > >> > > > >> ================================================================= > > > >> ==960805==ERROR: AddressSanitizer: heap-buffer-overflow on address > > > >> 0x520000010e18 at pc 0x56f170bc8a05 bp 0x7ffcd0552140 sp 0x7ffcd0552130 > > > >> READ of size 4 at 0x520000010e18 thread T0 > > > >> > > > >> #0 0x56f170bc8a04 in process_dive > > > >> /home/hohndel/src/subsurface/core/statistics.cpp:50 #1 > > > >> 0x56f170bca05e in calculate_stats_summary(bool) > > > >> /home/hohndel/src/subsurface/core/statistics.cpp:162 #2 > > > >> 0x56f170a2dbef in exportHTMLstatistics > > > >> /home/hohndel/src/subsurface/core/divelogexportlogic.cpp:81 #3 > > > >> 0x56f170a307fd in exportHtmlInitLogic(QString const&, > > > >> htmlExportSetting&) > > > >> /home/hohndel/src/subsurface/core/divelogexportlogic.cpp:142 #4 > > > >> 0x56f1709f89a3 in main > > > >> /home/hohndel/src/subsurface/export-html.cpp:67 #5 0x7522d4c2a1c9 > > > >> in > > > >> __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 #6 > > > >> 0x7522d4c2a28a in __libc_start_main_impl ../csu/libc-start.c:360 #7 > > > >> 0x56f1709f7aa4 in _start > > > >> (/srv/hohndel/src/subsurface/build/export-html+0x236aa4) (BuildId: > > > >> 3185437f967b115b8c1c9b6c8b5b8fe3d46e49bb)>> > > > >> > > > >> 0x520000010e18 is located 8 bytes after 3472-byte region > > > >> [0x520000010080,0x520000010e10)>> > > > >> > > > >> allocated by thread T0 here: > > > >> #0 0x7522d88fe548 in operator new(unsigned long) > > > >> ../../../../src/libsanitizer/asan/asan_new_delete.cpp:95 #1 > > > >> 0x56f170bd0eef in std::__new_allocator<stats_t>::allocate(unsigned > > > >> long, void const*) /usr/include/c++/13/bits/new_allocator.h:151 #2 > > > >> 0x56f170bd051e in std::allocator_traits<std::allocator<stats_t> > > > >> > > > >> >::allocate(std::allocator<stats_t>&, unsigned long) > > > >> > > > >> /usr/include/c++/13/bits/alloc_traits.h:482 #3 0x56f170bd051e in > > > >> std::_Vector_base<stats_t, std::allocator<stats_t> > > > >> > > > >> >::_M_allocate(unsigned long) > > > >> > > > >> /usr/include/c++/13/bits/stl_vector.h:381 #4 0x56f170bcf11d in > > > >> std::vector<stats_t, std::allocator<stats_t> > > > >> > > > >> >::_M_default_append(unsigned long) > > > >> > > > >> /usr/include/c++/13/bits/vector.tcc:663 #5 0x56f170bcd6ba in > > > >> std::vector<stats_t, std::allocator<stats_t> >::resize(unsigned > > > >> long) /usr/include/c++/13/bits/stl_vector.h:1016 #6 0x56f170bc96bb > > > >> in calculate_stats_summary(bool) > > > >> /home/hohndel/src/subsurface/core/statistics.cpp:119 #7 > > > >> 0x56f170a2dbef in exportHTMLstatistics > > > >> /home/hohndel/src/subsurface/core/divelogexportlogic.cpp:81 #8 > > > >> 0x56f170a307fd in exportHtmlInitLogic(QString const&, > > > >> htmlExportSetting&) > > > >> /home/hohndel/src/subsurface/core/divelogexportlogic.cpp:142 #9 > > > >> 0x56f1709f89a3 in main > > > >> /home/hohndel/src/subsurface/export-html.cpp:67 #10 0x7522d4c2a1c9 > > > >> in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 > > > >> #11 0x7522d4c2a28a in __libc_start_main_impl > > > >> ../csu/libc-start.c:360 > > > >> #12 0x56f1709f7aa4 in _start > > > >> (/srv/hohndel/src/subsurface/build/export-html+0x236aa4) (BuildId: > > > >> 3185437f967b115b8c1c9b6c8b5b8fe3d46e49bb)>> > > > >> > > > >> SUMMARY: AddressSanitizer: heap-buffer-overflow > > > >> /home/hohndel/src/subsurface/core/statistics.cpp:50 in process_dive > > > >> > > > >> So clearly we are addressing data outside the structure. I haven't > > > >> figured out how that could happen on line 50, though... > > > >> > > > >> /D > > > >> > > > >>> On Jan 10, 2025, at 08:42, Berthold Stoeger > > > >>> <bstoe...@mail.tuwien.ac.at> > > > >>> <mailto:bstoe...@mail.tuwien.ac.at> wrote: > > > >>> > > > >>> Hi Dirk, > > > >>> > > > >>> please run a debug version under valgrind and send me the result. A > > > >>> minimal reproducing example would also be nice (repeatedly delete one > > > >>> half of the dives and check if the issue is still there). > > > >>> > > > >>> N.B.: I'm in Indonesia until end of February and therefore will not be > > > >>> very responsive. I'll try to check my e-mails once per day. > > > >>> > > > >>> Berthold > > > _______________________________________________ > subsurface mailing list -- subsurface@subsurface-divelog.org > To unsubscribe send an email to subsurface-le...@subsurface-divelog.org _______________________________________________ subsurface mailing list -- subsurface@subsurface-divelog.org To unsubscribe send an email to subsurface-le...@subsurface-divelog.org
