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.

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

Switch from mach task_info to proc_pidinfo #42

Merged
merged 3 commits into from Aug 27, 2020

Conversation

fcheung
Copy link
Contributor

@fcheung fcheung commented Aug 27, 2020

The problem with the mach task_info routines is that on modern versions of
macos they require the task_for_pid-allow entitlement in order to get the
mach_task for another process (because there's all sorts of nefarious
things you can do with the mach port)

Since macos 10.5 the proc_* family of functions are a better way for getting
read-only information about processes.

This will still fail if the process is owned by another user. At the moment this raises Errno::EPERM, but should we perhaps rescue that somewhere and fall through to the next case (ps_memory)?

cc #32 #39 #41

The problem with the mach task_info routines is that on modern versions of
macos they require the task_for_pid-allow entitlement in order to get the
mach_task for another process (because there's all sorts of nefarious
things you can do with the mach port)

Since macos 10.5 the proc_* family of functions are a better way for getting
read-only information about processes.
@schneems
Copy link
Member

Thanks a ton for working on this and for the original PR! I have to ask, how

The failure is from

No checksum for downloaded archive, recording checksum in user configuration.
ruby-2.2.10 - #validate archive
ruby-2.2.10 - #extract
ruby-2.2.10 - #validate binary

Libraries missing for ruby-2.2.10: /usr/local/opt/gdbm/lib/libgdbm.5.dylib,/usr/local/opt/openssl/lib/libcrypto.1.0.0.dylib,/usr/local/opt/openssl/lib/libssl.1.0.0.dylib,/usr/local/opt/readline/lib/libreadline.7.dylib. Refer to your system manual for installing libraries

Mounting remote ruby failed with status 10, stopping installation.
Gemset '' does not exist, 'rvm ruby-2.2.10 do rvm gemset create ' first, or append '--create'.
The command "rvm use 2.2 --install --binary --fuzzy" failed and exited with 2 during .
Your build has been stopped.

I think we can ignore that failure. Having tests run on at least one mac distro seems good enough even if it's only one Ruby version. I started moving over to circleCI but then found that they don't support macOS on the open-source/free tier #43

This will still fail if the process is owned by another user. At the moment this raises Errno::EPERM, but should we perhaps rescue that somewhere and fall through to the next case (ps_memory)?

That would be pretty easy to do here:

  def darwin_memory
    Darwin.resident_size
  rescue Errno::EPERM
    return nil
  end

Then there's an auto fallback to shelling out to ps which would be invoked.

The api we call to get task info will raise this if you don't have
permission (eg trying to get info about another user's processes
when not root). By returning nil we'll fall back to the base case
of shelling out to `ps` (which doesn't have this issue because `ps` is
installed setuid & with the correct entitlements for this)
@fcheung
Copy link
Contributor Author

fcheung commented Aug 27, 2020

Thanks a ton for working on this and for the original PR! I have to ask, how

Missing sentence?

That would be pretty easy to do here:

  def darwin_memory
    Darwin.resident_size
  rescue Errno::EPERM
    return nil
  end

Then there's an auto fallback to shelling out to ps which would be invoked.

Cool, i have implemented that

@schneems schneems mentioned this pull request Aug 27, 2020
@schneems schneems merged commit 42af938 into zombocom:main Aug 27, 2020
@schneems
Copy link
Member

Thanks for all your help (yet again) I've merged this in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants