Page MenuHomeFreeBSD

Remove long broken procfs-based process tracing.
Needs ReviewPublic

Authored by mjg on Fri, Feb 14, 1:59 PM.

Details

Reviewers
kib
jeff
des
jhb
Summary

truss was converted to use ptrace in 2007 with 6.4 being the last release to use procfs to do it.

I untarred 6.4 base, mounted devfs and procfs and chrooted inside. "truss /bin/echo" hangs in the kernel with "pseudofs" as wait channel.

I downloaded a 10.3 image and performed the same test on said kernel with the same result. In other words while I don't know when the feature got broken, it was already broken in a release which is no longer supported. As such I don't believe there are any users and the feature should be eliminated.

truss on patched kernel:

$ truss /bin/echo
truss: PIOCBIS: Operation not supported
truss: cannot open1 /proc/622/mem: No such file or directory

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 29375

Event Timeline

mjg created this revision.Fri, Feb 14, 1:59 PM
mjg edited the summary of this revision. (Show Details)Fri, Feb 14, 1:59 PM
mjg edited the summary of this revision. (Show Details)
mjg edited the summary of this revision. (Show Details)
mjg edited the summary of this revision. (Show Details)
mjg edited the summary of this revision. (Show Details)Fri, Feb 14, 2:44 PM
jeff added a comment.Sun, Feb 16, 9:42 AM

It may be minorly useful to keep the ioctl definitions so that they are not-reused but I'm not sure what value we get necessarily from the switch statements. If there is some I'd prefer to group them and fallthrough to ENOTSUPP.

Do we want things to continue to compile? If not then some of sys/pioctl.h could go.

I am happy to see anything that adds complexity to signal handling go.

sys/fs/procfs/procfs_ioctl.c
129

Why keep status when the rest return ENOTSUPP?

kib added a comment.Sun, Feb 16, 10:26 AM

If one day I feel offended enough by this breakage, it seems that ptracestop() can be used to restore the functionality.

sys/sys/proc.h
646

There is no point to leave this. Yes, you have to recalculate offsets in kern_thread.c.

kib added a reviewer: jhb.Sun, Feb 16, 10:26 AM
mjg added a comment.Sun, Feb 16, 12:52 PM

The patch is intended be least disruptive: namely anyone who just references any of these is unaffected, while anyone using these is not regressed (and perhaps is improved because it exits with an error instead of hanging in the kernel).

sys/fs/procfs/procfs_ioctl.c
129

This retains the part which still worked.

sys/sys/proc.h
646

I just did not want to change the layout to not affect whatever (false) sharing might be there, but I'm happy to remove these if you insist.

Note the current layout is pretty arbitrary.