Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
paulgevers opened this issue Mar 7, 2024 · 15 comments
Closed

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

paulgevers opened this issue Mar 7, 2024 · 15 comments
Assignees
@lwindolf
Milestone
1.15.7

Comments

@paulgevers
Copy link
Contributor

paulgevers commented Mar 7, 2024 •
edited
Loading

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
Copy link
Owner

lwindolf commented Mar 8, 2024

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

@paulgevers
Copy link
Contributor Author

As you can see here my build would fail on most architectures. The architectures where it already failed in the past are referenced in #1223 (comment) 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
Copy link
Owner

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
Copy link
Contributor Author

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 time_t (64 bits) work ongoing on armel and armhf). See our wiki

@lwindolf
Copy link
Owner

lwindolf commented Mar 13, 2024 •
edited
Loading

@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
Copy link
Contributor Author

@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
Copy link
Owner

lwindolf commented Mar 13, 2024 •
edited
Loading

@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
Copy link
Owner

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

@paulgevers
Copy link
Contributor Author

paulgevers commented Mar 14, 2024 via email

@lwindolf
Copy link
Owner

lwindolf commented Mar 14, 2024 •
edited
Loading

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

@paulgevers
Copy link
Contributor Author

paulgevers commented Mar 15, 2024 via email

@lwindolf
Copy link
Owner

@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.

@lwindolf lwindolf added this to the 1.15.7 milestone Mar 15, 2024
@lwindolf lwindolf self-assigned this Mar 15, 2024
@paulgevers
Copy link
Contributor Author

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

@lwindolf
Copy link
Owner

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

@paulgevers
Copy link
Contributor Author

paulgevers commented Apr 6, 2024 •
edited
Loading

The latest log suggests it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lwindolf lwindolf

Labels
None yet
Projects
None yet
Milestone
1.15.7
Development

No branches or pull requests
2 participants
@lwindolf @paulgevers