search

archlinux / contrib

Arch contrib scripts
GNU General Public License v2.0
63 stars 18 forks source link

Checkservices: False positive on firewalld.service #76

Closed Antiz96 closed 1 month ago

Antiz96 commented 1 month ago

firewalld.service will always be reported as having broken maps files and needed to be restarted by checkservices, regardless if it was already restarted earlier or not (or even upgraded at all). By looking at the script, it seems like checkervices does some grepping/parsing to /proc/"pid of the service"/maps to determine the list of services with deleted maps files. For some reason, firewalld.service will always report one as deleted, causing a false positive:

image

Unfortunately, I don't have enough knowledge on this to understand if the issue is on the firewalld side, the libffi one or if it's the grepping/parsing part of checkservices that should be made more "robust" somehow. Maybe someone we'll be able to understand what's wrong here?

lahwaacz commented 1 month ago

I've noticed this too, seems to be happening only since the last upgrade (of something :angel:)

Antiz96 commented 1 month ago

I've noticed this too, seems to be happening only since the last upgrade (of something 👼)

Oh, well... I upgraded both archlinux-contrib and firewalld recently so... :angel: If you're positive it only started since the last upgrade of something, downgrading them might worth a shot!

I'll try that in a few.

Antiz96 commented 1 month ago

Actually, checkservices was last modified 2 years ago so it's most probably not the cause. I'll try downgrading firewalld in a minute to see if it helps.

Antiz96 commented 1 month ago

@lahwaacz Nice catch! Downgrading to firewalld 2.1.2-3 fixes the issue. The upgrade from 2.1.2 to 2.2.0 on the package side was a trivial one. So it's most probably due to an upstream change.

Not sure how to report that properly though (as I don't really understand what/where the problem might be) :/

lahwaacz commented 1 month ago

It seems to be related to this systemd service hardening: https://github.com/firewalld/firewalld/commit/1632f93544be8a26c278ac0012993c27f865bfa3

Antiz96 commented 1 month ago

Thanks, I'm including that in my upstream report.

lahwaacz commented 1 month ago

I'm thinking we should actually skip the /memfd: lines from the maps in the checkservices handling... But asking should not hurt

Antiz96 commented 1 month ago

After investigation, it turns out that it's the MemoryDenyWriteExecute=yes line added to the service file in https://github.com/firewalld/firewalld/commit/1632f93544be8a26c278ac0012993c27f865bfa3 that creates this behavior.

As @lahwaacz said, it seems like there's not much reason for checkservices to look at /memfd: maps (as checkservices is looking in the proc maps to find processes using files that were changed on the disk, but /memfd: is never on disk).

I'll open a MR to ignore /memfd: lines in checkservices.