lwindolf / liferea

Liferea (Linux Feed Reader), a news reader for GTK/GNOME
https://lzone.de/liferea
GNU General Public License v2.0
804 stars 131 forks source link

[1.15.5]: ./memcheck.sh reports problems #1342

Closed paulgevers closed 3 months ago

paulgevers commented 3 months ago

I'm getting this during my build of liferea 1.15.5:

./memcheck.sh parse_html favicon parse_date social
ERROR: memcheck reports problems for 'parse_html'!
==368794== Invalid write of size 8
ERROR: memcheck reports problems for 'favicon'!
==368803== Invalid write of size 8
ERROR: memcheck reports problems for 'parse_date'!
==368810== Invalid write of size 8
ERROR: memcheck reports problems for 'social'!
==368817== Invalid write of size 8

(One note, I do replace js/purify.min.js with a version provided in a Debian package instead of the one vendored in liferea.)

lwindolf commented 3 months ago

@paulgevers These errors are new to 1.15.5 and did not happen with 1.15.4?

paulgevers commented 3 months ago

As you can see here my build would fail on most architectures. The architectures where it already failed in the past are referenced in https://github.com/lwindolf/liferea/issues/1223#issuecomment-1686928225 so it's not totally new, but before it didn't happen on amd64 and the error message is slightly different. As you can see on reproducible builds, it's not because of changes in the toolchain and I missed it because no rebuild happened. It really seems new (in 1.15.5) on amd64.

lwindolf commented 3 months ago

Thanks then it must be a commit in between 1.15.4 and 1.15.5. I'll have a look to find the culprit

paulgevers commented 3 months ago

Thanks then it must be a commit in between 1.15.4 and 1.15.5. I'll have a look to find the culprit

It seems I was wrong. I just received bug 1066763 showing the same problem with 1.15.4, so this looks like a toolchain issue.

Note: Debian recently enabled a new build profile (because of timet (64 bits) work ongoing on armel and armhf). See [our wiki](https://wiki.debian.org/qa.debian.org/FTBFS#A2024-03-13-Werror.3Dimplicit-function-declaration)

lwindolf commented 3 months ago

@paulgevers Thanks for letting me know. By using valgrind I now that the code base has a lot of memory errors. Sadly they are too many also in several underlying libraries, so valgrind is useless as it stop reporting after 1000 errors. This prevents me from actually debugging it with valgrind.

One possible way I could go is to whitelist a lot of stuff to at least catch new regressions. But this still wouldn't solve memcheck failing...

I'm a bit at a loss here.

paulgevers commented 3 months ago

@lwindolf I reported this bug in the hope it is/was useful. I'm fine with ignoring the memcheck failure for the time being. Unless you believe it points at severe problems. I must admit I have little experience (and less success) with debugging c code, so I can't really help at that level.

lwindolf commented 3 months ago

@paulgevers I'm not the C expert, but I tried hard to reproduce it on Ubuntu and failed so far. At the moment I guess it is a library dependency causing it.

Do you have a Debian system where you can reproduce the issue and could run

cd src/tests
valgrind -q --leak-check=full --suppressions=<(cat <<-EOT
{
        selinuxfs_exists
        Memcheck:Leak
        match-leak-kinds: definite
        fun:malloc
        fun:initialise_tags
        fun:_Z25semmle_read_configurationv
        fun:semmle_init
        fun:fopen
        fun:selinuxfs_exists
        obj:/lib/x86_64-linux-gnu/libselinux.so.1
        fun:call_init
        fun:_dl_init
        obj:/lib/x86_64-linux-gnu/ld-2.27.so
}
EOT
) ./parse_html

This would give a log of the errors. Alternatively you could patch memcheck.sh to remove this condition

# When in github provide extra details
if [ "$GITHUB_ACTION" != "" ]; then

so error details are always shown.

With those details we could decide wether the problem lies in Liferea or an underlying library.

lwindolf commented 3 months ago

Thinking over it once more I think it is easier to always print the detail output. I changed the behaviour in 14993227

paulgevers commented 3 months ago

Hi,

On 13-03-2024 10:53 p.m., Lars Windolf wrote:

Do you have a Debian system where you can reproduce the issue and could run

|cd src/tests valgrind -q --leak-check=full --suppressions=<(cat <<-EOT { selinuxfs_exists Memcheck:Leak match-leak-kinds: definite fun:malloc fun:initialise_tags fun:_Z25semmle_read_configurationv fun:semmle_init fun:fopen fun:selinuxfs_exists obj:/lib/x86_64-linux-gnu/libselinux.so.1 fun:call_init fun:_dl_init obj:/lib/x86_64-linux-gnu/ld-2.27.so } EOT ) ./parse_html |

This would give a log of the leaks. Alternatively you could patch |memcheck.sh| to remove this condition

|# When in github provide extra details if [ "$GITHUB_ACTION" != "" ]; then |

so error details are always shown.

With those details we could decide wether the problem lies in Liferea or an underlying library.

http://debomatic-amd64.debian.net/distribution#testing/liferea/1.15.5-1~debo1/buildlog

ERROR: memcheck reports problems for 'parse_html'! ==725869== Invalid write of size 8 ::group:: parse_html details ==725869== begin ==725869== Invalid write of size 8 ==725869== at 0xC0E7D45: ??? (in /usr/lib/x86_64-linux-gnu/liblzma.so.5.6.0) ==725869== by 0xC0CA81B: ??? (in /usr/lib/x86_64-linux-gnu/liblzma.so.5.6.0) ==725869== by 0x6A00000006: ??? ==725869== by 0xBBE053F: ??? ==725869== by 0xBB1F52626F721DFF: ??? ==725869== by 0x1FFEFFF92F: ??? ==725869== by 0x400EFAA: elf_machine_rela (dl-machine.h:300) ==725869== by 0x400EFAA: elf_dynamic_do_Rela (do-rel.h:147) ==725869== by 0x400EFAA: _dl_relocate_object (dl-reloc.c:301) ==725869== by 0x11E90D7F: ??? ==725869== by 0x11EA8B6F: ??? ==725869== by 0x1FFEFFF8BF: ??? ==725869== Address 0x1ffeffe9d8 is on thread 1's stack ==725869== 136 bytes below stack pointer ==725869== ==725869== end

Paul

lwindolf commented 3 months ago

As suspected a base library is causing it. So no fault in Liferea. I added a valgrind supression rule with cec20384. If I defined it right your build should now pass.

paulgevers commented 3 months ago

Hi,

On 14-03-2024 8:00 p.m., Lars Windolf wrote:

As suspected a base library is causing it. So no fault in Liferea. I added a valgrind supression rule with cec2038 https://github.com/lwindolf/liferea/commit/cec2038486a59db4cf399108e58930fe5b404934. If I defined it right you build should now pass.

It seems you missed the following block: lzmaDebian#1342_value Memcheck:Addr8 obj:/usr/lib//liblzma.so.

(I made the obj check architecture independent). With this fix it worked. I'll upload the new version to Debian shortly. Do you think this warrants filing a bug with the lzma package (not that that's very active in Debian)?

Paul

lwindolf commented 3 months ago

@paulgevers I've added the additional suppression and made the others architecture independant too. I'd guess that the liblzma maintainer would have a hard time to reproduce the effect. So I'd predict it would not help much.

Closing this as solved.

paulgevers commented 3 months ago

I'm suspecting this is related: https://www.openwall.com/lists/oss-security/2024/03/29/4

lwindolf commented 3 months ago

@paulgevers Interesting. Let's see if it is gone with the patches to liblzma!

paulgevers commented 2 months ago

The latest log suggests it is.