On 31/10/2023 22:36, Chris Lamb wrote:
tags 1055039 + pending
thanks
Hey Arnaud!
Long story below.
A huge thanks for tracking this down! I've gone ahead and removed
ProcSubset=pid from the systemd unit files, and am uploading a version
to unstable and experimental right now. However, do you think this
warrants an update to stable as well…? Thanks again.
I would say yes, upload to stable as well. At least for our use-case, it
makes Redis unusable, that's pretty bad. I wish I could tell you more
precisely what goes wrong in our case though...
I checked a bit more in the redis source code, to find reference to
/proc (and not /proc/self):
$ find -name '*.[ch]' | xargs grep -ho '[ "]/proc/[^ "]*' \
| grep -v /self/ | tr -d ' ",.' | sort | uniq -c
1 /proc/cpuinfo
1 /proc/curproc/map
1 /proc/%d/maps
1 /proc/%d/task/%d/maps
1 /proc/%ld/psinfo
1 /proc/%ld/smaps
1 /proc/<pid>/stat
3 /proc/sys/net/core/somaxconn
1 /proc/sys/net/ipv4/tcp_tw_reuse'
8 /proc/sys/vm/overcommit_memory
To me, it suggests that yes, Redis needs full access to /proc to be
fully functional.
Moreover, I see that ProcSusbset=pid caused some trouble already, that
you fixed in 80470e3dc0ae56db9c9512c38a1757844443bcfc ;)
Looking at the part of the code that was touched by this patch, I see
that Redis checks whether overcommit memory is enabled (function
checkOvercommit), and it displays a big fat warning if ever it's not. I
suppose that, with ProcSubset=pid, Redis won't be able to perform this
check and won't be able to display this warning.
So, yep, I still think that it's better to backport this change to
stable. If ever someone needs to re-enable ProcSubset=pid, maybe it will
need more care. Better reach out to upstream to ask whether it's a good idea
Thank you very much for maintaining this package!
--
Arnaud Rebillout / OffSec / Kali Linux Developer