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

Reply via email to