Re: svn commit: r278479 - in head: etc sys/kern
On Mar 22, 2015, at 18:08, Mateusz Guzik mjgu...@gmail.com wrote: On Mon, Feb 09, 2015 at 11:13:51PM +, Rui Paulo wrote: Author: rpaulo Date: Mon Feb 9 23:13:50 2015 New Revision: 278479 URL: https://svnweb.freebsd.org/changeset/base/278479 Log: Notify devd(8) when a process crashed. This change implements a notification (via devctl) to userland when the kernel produces coredumps after a process has crashed. devd can then run a specific command to produce a human readable crash report. The command is most usually a helper that runs gdb/lldb commands on the file/coredump pair. It's possible to use this functionality for implementing automatic generation of crash reports. devd(8) will be notified of the full path of the binary that crashed and the full path of the coredump file. The more I look at this the more I'm convinced this is quite insecure. At a minimum this should also grow a flag to decide whether notification about jailed process crashes are allowed. Off by default. As it is you pass a path leading to a jail, but that's inherently untrusted and will lead to trouble. We got sidetracked by the devd-bloat discussion, but I can turn this off until a better approach is programmed. -- Rui Paulo ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On Mon, Feb 09, 2015 at 11:13:51PM +, Rui Paulo wrote: Author: rpaulo Date: Mon Feb 9 23:13:50 2015 New Revision: 278479 URL: https://svnweb.freebsd.org/changeset/base/278479 Log: Notify devd(8) when a process crashed. This change implements a notification (via devctl) to userland when the kernel produces coredumps after a process has crashed. devd can then run a specific command to produce a human readable crash report. The command is most usually a helper that runs gdb/lldb commands on the file/coredump pair. It's possible to use this functionality for implementing automatic generation of crash reports. devd(8) will be notified of the full path of the binary that crashed and the full path of the coredump file. The more I look at this the more I'm convinced this is quite insecure. At a minimum this should also grow a flag to decide whether notification about jailed process crashes are allowed. Off by default. As it is you pass a path leading to a jail, but that's inherently untrusted and will lead to trouble. -- Mateusz Guzik mjguzik gmail.com ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
[[ I know I’m a little behind… ]] On Feb 10, 2015, at 7:16 AM, John Baldwin j...@freebsd.org wrote: On Monday, February 09, 2015 11:13:51 PM Rui Paulo wrote: Author: rpaulo Date: Mon Feb 9 23:13:50 2015 New Revision: 278479 URL: https://svnweb.freebsd.org/changeset/base/278479 Log: Notify devd(8) when a process crashed. This change implements a notification (via devctl) to userland when the kernel produces coredumps after a process has crashed. devd can then run a specific command to produce a human readable crash report. The command is most usually a helper that runs gdb/lldb commands on the file/coredump pair. It's possible to use this functionality for implementing automatic generation of crash reports. devd(8) will be notified of the full path of the binary that crashed and the full path of the coredump file. I think this is a very useful feature and I think this is fine to be in the tree as-is for now. My only note is that this is a bit of feature creep for devd (this isn't a device notification, this is a system event notification). devd was always envisioned as being a system event notification thing. It just started out life doing newbus events. It’s been growing these sorts of notifications for quite some time. As such, I think it might be worth thinking if we (collectively) want to think about having a separate framework at all for system event notification. You could possibly publish other interesting events this way. For example, Isilon currently has a patch to log(9) Witness LORs. I personally think it's a bit hackish and potentially unreliable. A much nicer interface if you want to capture such things would be to publish an event for each logged LOR instead. Machine checks are another example of something that might be nice to publish (though you could possibly make the case that those would not be inappropriate to publish via devd since actual hardware is involved). Disk and PCI errors are another class of thing that it would be nice to publish in an easier to programmaticaly parse manner. I’d love to see this. Though it might mean we’d need to implement some kind of filtering for the redistribution port that devd has. Warner signature.asc Description: Message signed with OpenPGP using GPGMail
Re: svn commit: r278479 - in head: etc sys/kern
On Tuesday, February 10, 2015 07:06:03 AM Adrian Chadd wrote: On 10 February 2015 at 06:16, John Baldwin j...@freebsd.org wrote: On Monday, February 09, 2015 11:13:51 PM Rui Paulo wrote: Author: rpaulo Date: Mon Feb 9 23:13:50 2015 New Revision: 278479 URL: https://svnweb.freebsd.org/changeset/base/278479 Log: Notify devd(8) when a process crashed. This change implements a notification (via devctl) to userland when the kernel produces coredumps after a process has crashed. devd can then run a specific command to produce a human readable crash report. The command is most usually a helper that runs gdb/lldb commands on the file/coredump pair. It's possible to use this functionality for implementing automatic generation of crash reports. devd(8) will be notified of the full path of the binary that crashed and the full path of the coredump file. I think this is a very useful feature and I think this is fine to be in the tree as-is for now. My only note is that this is a bit of feature creep for devd (this isn't a device notification, this is a system event notification). As such, I think it might be worth thinking if we (collectively) want to think about having a separate framework at all for system event notification. You could possibly publish other interesting events this way. For example, Isilon currently has a patch to log(9) Witness LORs. I personally think it's a bit hackish and potentially unreliable. A much nicer interface if you want to capture such things would be to publish an event for each logged LOR instead. Machine checks are another example of something that might be nice to publish (though you could possibly make the case that those would not be inappropriate to publish via devd since actual hardware is involved). Disk and PCI errors are another class of thing that it would be nice to publish in an easier to programmaticaly parse manner. Cool, so someone's going to add multi-subscriber support to /dev/devctl ? Eh, devd publishes /var/run/devd.pipe already which supports multiple subscribers. I think that was one of the intentional design decisions was to handle multiple subscribers in userland rather than the kernel. I think devd grows these things because it's easier than teaching the devctl interface to support multiple listeners. That wasn't really my question. My question was if we want distinct streams or if we want want unified stream. Having a unified stream might very well make sense (and if so we could rename devd to make that more obvious). -- John Baldwin ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On Tue, Feb 10, 2015 at 07:06:03AM -0800, Adrian Chadd wrote: On 10 February 2015 at 06:16, John Baldwin j...@freebsd.org wrote: On Monday, February 09, 2015 11:13:51 PM Rui Paulo wrote: Author: rpaulo Date: Mon Feb 9 23:13:50 2015 New Revision: 278479 URL: https://svnweb.freebsd.org/changeset/base/278479 Log: Notify devd(8) when a process crashed. This change implements a notification (via devctl) to userland when the kernel produces coredumps after a process has crashed. devd can then run a specific command to produce a human readable crash report. The command is most usually a helper that runs gdb/lldb commands on the file/coredump pair. It's possible to use this functionality for implementing automatic generation of crash reports. devd(8) will be notified of the full path of the binary that crashed and the full path of the coredump file. I think this is a very useful feature and I think this is fine to be in the tree as-is for now. My only note is that this is a bit of feature creep for devd (this isn't a device notification, this is a system event notification). As such, I think it might be worth thinking if we (collectively) want to think about having a separate framework at all for system event notification. You could possibly publish other interesting events this way. For example, Isilon currently has a patch to log(9) Witness LORs. I personally think it's a bit hackish and potentially unreliable. A much nicer interface if you want to capture such things would be to publish an event for each logged LOR instead. Machine checks are another example of something that might be nice to publish (though you could possibly make the case that those would not be inappropriate to publish via devd since actual hardware is involved). Disk and PCI errors are another class of thing that it would be nice to publish in an easier to programmaticaly parse manner. Cool, so someone's going to add multi-subscriber support to /dev/devctl ? I think devd grows these things because it's easier than teaching the devctl interface to support multiple listeners. /dev/eventctl and eventd? And, may be, rename /dev/devctl2 to /dev/eventctl too. ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On Tue, 2015-02-10 at 07:06 -0800, Adrian Chadd wrote: On 10 February 2015 at 06:16, John Baldwin j...@freebsd.org wrote: On Monday, February 09, 2015 11:13:51 PM Rui Paulo wrote: Author: rpaulo Date: Mon Feb 9 23:13:50 2015 New Revision: 278479 URL: https://svnweb.freebsd.org/changeset/base/278479 Log: Notify devd(8) when a process crashed. This change implements a notification (via devctl) to userland when the kernel produces coredumps after a process has crashed. devd can then run a specific command to produce a human readable crash report. The command is most usually a helper that runs gdb/lldb commands on the file/coredump pair. It's possible to use this functionality for implementing automatic generation of crash reports. devd(8) will be notified of the full path of the binary that crashed and the full path of the coredump file. I think this is a very useful feature and I think this is fine to be in the tree as-is for now. My only note is that this is a bit of feature creep for devd (this isn't a device notification, this is a system event notification). As such, I think it might be worth thinking if we (collectively) want to think about having a separate framework at all for system event notification. You could possibly publish other interesting events this way. For example, Isilon currently has a patch to log(9) Witness LORs. I personally think it's a bit hackish and potentially unreliable. A much nicer interface if you want to capture such things would be to publish an event for each logged LOR instead. Machine checks are another example of something that might be nice to publish (though you could possibly make the case that those would not be inappropriate to publish via devd since actual hardware is involved). Disk and PCI errors are another class of thing that it would be nice to publish in an easier to programmaticaly parse manner. Cool, so someone's going to add multi-subscriber support to /dev/devctl ? I don't see how you get that from the foregoing at all. Doing fan-out in the kernel is hard, and doing it in userland is easier, so the existing devd model could be an appropriate one to follow for a general kernel events which are not hardware events system. I think devd grows these things because it's easier than teaching the devctl interface to support multiple listeners. Until recently, devd only grew additional support for things which are device-related, modulo the zfs stuff that I don't know much about. I was under the impression that that also was device-related in some way, such as zfs reporting on device errors or attach/remove and things related to that (like health of the pool(s) affected by the device change). Maybe the zfs stuff was the beginning of the mission creep and still had enough of a device-related fig leaf to not look like creep. But there's no disguising the fact that post-processing core dumps has absolutely nothing to do with devices. -- Ian ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On Mon, Feb 09, 2015 at 06:35:55PM -0800, Rui Paulo wrote: On Feb 9, 2015, at 15:28, Konstantin Belousov kostik...@gmail.com wrote: Arguably, there should be a knob, probably sysctl, to turn the functionality off. I definitely do not want this on crash boxes used for userspace debugging. Even despite the example handler is inactive. OK, I can provide a sysctl knob. Seen that, thanks. + len = MAXPATHLEN * 2 + 5 /* comm= */ + 5 /* core= */ + 1; It is much cleaner to use static const char arrays for the names, and use sizeof() - 1 instead of hard-coding commented constants. OK. I was trying to avoid allocating 2k on the stack. Probably I was not clear enough. I do not suggest to change data allocation from malloc to automatic. I mean static const char comm_name[] = comm=; and then use sizeof(comm_name) - 1 and comm_name instead of string literal. + data = malloc(len, M_TEMP, M_NOWAIT); Why is this allocation M_NOWAIT ? That should be M_WAITOK. + freepath = NULL; + } + if (vn_fullpath_global(td, vp, fullpath, freepath) != 0) + goto out; + snprintf(data, len, %s core=%s, data, fullpath); This is weird, and highly depends on the implementation details, supplying the same string as target and source. IMO strcat(9) is enough there. OK, I'll change it to strcat. -- Rui Paulo ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On Mon, Feb 09, 2015 at 06:57:36PM -0800, Rui Paulo wrote:
On Feb 9, 2015, at 18:43, Mateusz Guzik mjgu...@gmail.com wrote:
On Mon, Feb 09, 2015 at 11:13:51PM +, Rui Paulo wrote:
+notify 10 {
+ match system kernel;
+ match subsystem signal;
+ match typecoredump;
+ action logger $comm $core;
+};
+
*/
[..]
+ if (vn_fullpath_global(td, p-p_textvp, fullpath, freepath) != 0)
+ goto out;
+ snprintf(data, len, comm=%s, fullpath);
I cannot test it right now, but it looks like immediate privilege
escalation.
Path is not sanitized in any way and devd passes it to 'sh -c'.
So a file named a.out; /bin/id; meh or so should result in execution
of aforementioned /bin/id.
Well, you can't have a file name with / but you're right.
I mean the whole path. You are resolving the name along with all dirs,
so it's just a matter of some mkdirs.
Another note is that currently devctl is record oriented, but this may
change at some point and free form userspace text could be used to forge
new events.
As such is trongly suggest we sanitize this somehow. Maybe a base64 or
something.
I was trying hard to avoid this issue in unpublished my crash helper, but I
forgot that devd runs execl(sh -c, ); :-(
It might just be easier to inspect the path names and allow only
[a-z][A-Z][0-9] and '/' before sending the devctl message.
I'm pretty sure sooner or later people will want something with a space,
so I would prefer a reasonably complete solution.
A hack like the one yu mention should suffice fr now though (with the
addition of a dot).
--
Mateusz Guzik mjguzik gmail.com
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On Tue, Feb 10, 2015 at 06:30:27PM +, Rui Paulo wrote: On Feb 10, 2015, at 07:37 AM, John Baldwin j...@freebsd.org wrote: That wasn't really my question. My question was if we want distinct streams or if we want want unified stream. Having a unified stream might very well make sense (and if so we could rename devd to make that more obvious). I'm fine with renaming devd to eventd or something else, but Ian was saying that he's worried about the number of notifications that devd has to process. I'm not sure that's a real problem at this point, though. On freefall, devd used 0.07 seconds of CPU time and has been running for a 1 day and a half. On my BeagleBone, devd used 0.61 seconds of CPU time and it has been up for 5 days and a half. On my VM that has been up for 5 days and a half, it used 4 seconds of CPU time. Renaming sounds like a good idea and it looks like we could leave the optimisations to a later time. For common case (I am not talk about current devd implementation -- I am don't have any inforamtion/metrics/etc) routing and processing events may be sensitive to delay and ordering: may be exist requirement 'delay not more then 700ns', may be exist requirement 'next event process only after complete process previos event'. And some event handling may be very CPU/disk/etc consumption. Need to good think over and design API and architecure. ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On Feb 10, 2015, at 10:52 AM, Slawa Olhovchenkov s...@zxy.spb.ru wrote: For common case (I am not talk about current devd implementation -- I am don't have any inforamtion/metrics/etc) routing and processing events may be sensitive to delay and ordering: may be exist requirement 'delay not more then 700ns', may be exist requirement 'next event process only after complete process previos event'. And some event handling may be very CPU/disk/etc consumption. Need to good think over and design API and architecure. routing events do not come over devd (or any other /dev device): there's a routing socket specifically tailored for that purpose. For that type of latency, you probably don't want to be crossing the kernel/userland barrier too often (or never). ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On Feb 10, 2015, at 07:37 AM, John Baldwin j...@freebsd.org wrote: That wasn't really my question. My question was if we want distinct streams or if we want want unified stream. Having a unified stream might very well make sense (and if so we could rename devd to make that more obvious). I'm fine with renaming devd to eventd or something else, but Ian was saying that he's worried about the number of notifications that devd has to process. I'm not sure that's a real problem at this point, though. On freefall, devd used 0.07 seconds of CPU time and has been running for a 1 day and a half. On my BeagleBone, devd used 0.61 seconds of CPU time and it has been up for 5 days and a half. On my VM that has been up for 5 days and a half, it used 4 seconds of CPU time. Renaming sounds like a good idea and it looks like we could leave the optimisations to a later time. Another thing I had in mind (which is more work) was to abstract the devctl kernel code in an API which could make it easy to fan out the notifications to multiple /dev devices. However, that may be overkill. ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On 10 Feb 2015, at 18:30, Rui Paulo rpa...@me.com wrote: Another thing I had in mind (which is more work) was to abstract the devctl kernel code in an API which could make it easy to fan out the notifications to multiple /dev devices. However, that may be overkill. This kind of notification is something that kdbus is increasingly being used for on Linux. The primitive allows events to originate either in the kernel or in userspace and to be sent either point-to-point or to a bloom filter set of recipients (so you occasionally get some messages you're not expecting, but hopefully don't get too many spurious wakeups). David ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On Tue, Feb 10, 2015 at 06:55:28PM +, Rui Paulo wrote:
On Feb 10, 2015, at 10:52 AM, Slawa Olhovchenkov s...@zxy.spb.ru wrote:
For common case (I am not talk about current devd implementation -- I
am don't have any inforamtion/metrics/etc) routing and processing
events may be sensitive to delay and ordering: may be exist
requirement 'delay not more then 700ns', may be exist requirement
'next event process only after complete process previos event'. And
some event handling may be very CPU/disk/etc consumption. Need to good
think over and design API and architecure.
routing events do not come over devd (or any other /dev device):
there's a routing socket specifically tailored for that purpose.
I am talk abot this:
nomatch 32 {
match bus uhub[0-9]+;
match mode host;
match vendor 0x03eb;
match product 0x2109;
action kldload -n uftdi;
};
For that type of latency, you probably don't want to be crossing
the kernel/userland barrier too often (or never).
We have action for coredump. And have action for automount (when
inserting USB disk).
Must be coredump wait for complete mounting USB disk?
For flapping USB disk need to start multiple mount script?
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
John Baldwin wrote this message on Tue, Feb 10, 2015 at 10:36 -0500: I think devd grows these things because it's easier than teaching the devctl interface to support multiple listeners. That wasn't really my question. My question was if we want distinct streams or if we want want unified stream. Having a unified stream might very well make sense (and if so we could rename devd to make that more obvious). The biggested issue that I see w/ using devd as the gateway is that as we add more events, devd will now have to be woken up for ALL events, even when the even has no listeners for it... If we keep adding events, this will be very bad for laptop battery life... This is becoming a standard pub/sub type problem, and there is plenty of research/programs out there that already does this... As someone mentioned earlier, it seems like using kqueue is starting to make more sense for this type of thing where various programs can select what they get woken up for... -- John-Mark Gurney Voice: +1 415 225 5579 All that I will do, has been done, All that I have, has not. ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On Tue, 2015-02-10 at 12:15 -0800, John-Mark Gurney wrote: John Baldwin wrote this message on Tue, Feb 10, 2015 at 10:36 -0500: I think devd grows these things because it's easier than teaching the devctl interface to support multiple listeners. That wasn't really my question. My question was if we want distinct streams or if we want want unified stream. Having a unified stream might very well make sense (and if so we could rename devd to make that more obvious). The biggested issue that I see w/ using devd as the gateway is that as we add more events, devd will now have to be woken up for ALL events, even when the even has no listeners for it... If we keep adding events, this will be very bad for laptop battery life... That's similar to my worry but for our apps at $work the problem extends to them as well... the kernel will wake up devd for lots of events it doesn't need to process, then it fans them out to our app which also gets woken for events that it's not interested in. Big iron in datacenters isn't going to have any problems with this, but embedded systems at the low end are still somewhat cycle-sensitive. The worrisome part is the creep. Right now it's just crashdumps, and that isn't going to add any overhead for $work, because our apps never crash (::grin::). But the floodgates are open and in a couple years it's going to be a lot of new kinds of events and a lot of new cycles consumed processing them in userland. This is becoming a standard pub/sub type problem, and there is plenty of research/programs out there that already does this... As someone mentioned earlier, it seems like using kqueue is starting to make more sense for this type of thing where various programs can select what they get woken up for... Filtering at the source (or nearer to it) does seem to be one way to get everything that's good about a generic event notification daemon without all of the bad that will accumulate if we just stumble into it without a long term vision. I'm not as familiar with kevent specifics as I'd like to be, so I can't say it's a ready to use off the shelf solution, but it may be part of one. -- Ian ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On 10 February 2015 at 06:16, John Baldwin j...@freebsd.org wrote: On Monday, February 09, 2015 11:13:51 PM Rui Paulo wrote: Author: rpaulo Date: Mon Feb 9 23:13:50 2015 New Revision: 278479 URL: https://svnweb.freebsd.org/changeset/base/278479 Log: Notify devd(8) when a process crashed. This change implements a notification (via devctl) to userland when the kernel produces coredumps after a process has crashed. devd can then run a specific command to produce a human readable crash report. The command is most usually a helper that runs gdb/lldb commands on the file/coredump pair. It's possible to use this functionality for implementing automatic generation of crash reports. devd(8) will be notified of the full path of the binary that crashed and the full path of the coredump file. I think this is a very useful feature and I think this is fine to be in the tree as-is for now. My only note is that this is a bit of feature creep for devd (this isn't a device notification, this is a system event notification). As such, I think it might be worth thinking if we (collectively) want to think about having a separate framework at all for system event notification. You could possibly publish other interesting events this way. For example, Isilon currently has a patch to log(9) Witness LORs. I personally think it's a bit hackish and potentially unreliable. A much nicer interface if you want to capture such things would be to publish an event for each logged LOR instead. Machine checks are another example of something that might be nice to publish (though you could possibly make the case that those would not be inappropriate to publish via devd since actual hardware is involved). Disk and PCI errors are another class of thing that it would be nice to publish in an easier to programmaticaly parse manner. Cool, so someone's going to add multi-subscriber support to /dev/devctl ? I think devd grows these things because it's easier than teaching the devctl interface to support multiple listeners. -adrian ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On Monday, February 09, 2015 11:13:51 PM Rui Paulo wrote: Author: rpaulo Date: Mon Feb 9 23:13:50 2015 New Revision: 278479 URL: https://svnweb.freebsd.org/changeset/base/278479 Log: Notify devd(8) when a process crashed. This change implements a notification (via devctl) to userland when the kernel produces coredumps after a process has crashed. devd can then run a specific command to produce a human readable crash report. The command is most usually a helper that runs gdb/lldb commands on the file/coredump pair. It's possible to use this functionality for implementing automatic generation of crash reports. devd(8) will be notified of the full path of the binary that crashed and the full path of the coredump file. I think this is a very useful feature and I think this is fine to be in the tree as-is for now. My only note is that this is a bit of feature creep for devd (this isn't a device notification, this is a system event notification). As such, I think it might be worth thinking if we (collectively) want to think about having a separate framework at all for system event notification. You could possibly publish other interesting events this way. For example, Isilon currently has a patch to log(9) Witness LORs. I personally think it's a bit hackish and potentially unreliable. A much nicer interface if you want to capture such things would be to publish an event for each logged LOR instead. Machine checks are another example of something that might be nice to publish (though you could possibly make the case that those would not be inappropriate to publish via devd since actual hardware is involved). Disk and PCI errors are another class of thing that it would be nice to publish in an easier to programmaticaly parse manner. -- John Baldwin ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On Feb 9, 2015, at 15:28, Konstantin Belousov kostik...@gmail.com wrote: Arguably, there should be a knob, probably sysctl, to turn the functionality off. I definitely do not want this on crash boxes used for userspace debugging. Even despite the example handler is inactive. OK, I can provide a sysctl knob. +len = MAXPATHLEN * 2 + 5 /* comm= */ + 5 /* core= */ + 1; It is much cleaner to use static const char arrays for the names, and use sizeof() - 1 instead of hard-coding commented constants. OK. I was trying to avoid allocating 2k on the stack. +data = malloc(len, M_TEMP, M_NOWAIT); Why is this allocation M_NOWAIT ? That should be M_WAITOK. +freepath = NULL; +} +if (vn_fullpath_global(td, vp, fullpath, freepath) != 0) +goto out; +snprintf(data, len, %s core=%s, data, fullpath); This is weird, and highly depends on the implementation details, supplying the same string as target and source. IMO strcat(9) is enough there. OK, I'll change it to strcat. -- Rui Paulo ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On 9 Feb, Don Lewis wrote:
On 10 Feb, Mateusz Guzik wrote:
On Mon, Feb 09, 2015 at 11:13:51PM +, Rui Paulo wrote:
+notify 10 {
+ match system kernel;
+ match subsystem signal;
+ match typecoredump;
+ action logger $comm $core;
+};
+
*/
[..]
+ if (vn_fullpath_global(td, p-p_textvp, fullpath, freepath) != 0)
+ goto out;
+ snprintf(data, len, comm=%s, fullpath);
I cannot test it right now, but it looks like immediate privilege
escalation.
Path is not sanitized in any way and devd passes it to 'sh -c'.
So a file named a.out; /bin/id; meh or so should result in execution
of aforementioned /bin/id.
Then there is the issue of a user-generated core file being fed into the
crash analyzer, possibly exploiting bugs in the latter.
Or worse, the contents of the executable, in particular the debug info,
could also be an attack vector.
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On Feb 9, 2015, at 18:43, Mateusz Guzik mjgu...@gmail.com wrote:
On Mon, Feb 09, 2015 at 11:13:51PM +, Rui Paulo wrote:
+notify 10 {
+match system kernel;
+match subsystem signal;
+match typecoredump;
+action logger $comm $core;
+};
+
*/
[..]
+if (vn_fullpath_global(td, p-p_textvp, fullpath, freepath) != 0)
+goto out;
+snprintf(data, len, comm=%s, fullpath);
I cannot test it right now, but it looks like immediate privilege
escalation.
Path is not sanitized in any way and devd passes it to 'sh -c'.
So a file named a.out; /bin/id; meh or so should result in execution
of aforementioned /bin/id.
Well, you can't have a file name with / but you're right.
Another note is that currently devctl is record oriented, but this may
change at some point and free form userspace text could be used to forge
new events.
As such is trongly suggest we sanitize this somehow. Maybe a base64 or
something.
I was trying hard to avoid this issue in unpublished my crash helper, but I
forgot that devd runs execl(sh -c, ); :-(
It might just be easier to inspect the path names and allow only
[a-z][A-Z][0-9] and '/' before sending the devctl message.
--
Rui Paulo
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On 10 Feb, Mateusz Guzik wrote:
On Mon, Feb 09, 2015 at 11:13:51PM +, Rui Paulo wrote:
+notify 10 {
+match system kernel;
+match subsystem signal;
+match typecoredump;
+action logger $comm $core;
+};
+
*/
[..]
+if (vn_fullpath_global(td, p-p_textvp, fullpath, freepath) != 0)
+goto out;
+snprintf(data, len, comm=%s, fullpath);
I cannot test it right now, but it looks like immediate privilege
escalation.
Path is not sanitized in any way and devd passes it to 'sh -c'.
So a file named a.out; /bin/id; meh or so should result in execution
of aforementioned /bin/id.
Then there is the issue of a user-generated core file being fed into the
crash analyzer, possibly exploiting bugs in the latter.
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On Feb 9, 2015, at 15:29, Benjamin Kaduk bjkf...@gmail.com wrote: The question boils down to: is the time saved by implementing it this way worth the tradeoff of architectural purity. Yes, that was a tradeoff. -- Rui Paulo ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On Mon, 2015-02-09 at 18:29 -0500, Benjamin Kaduk wrote: On Mon, Feb 9, 2015 at 6:22 PM, Rui Paulo rpa...@me.com wrote: On Feb 09, 2015, at 03:16 PM, Benjamin Kaduk bjkf...@gmail.com wrote: What advantage does putting this in devd have over a standalone daemon for crash reporting? Is it just the ease of implementation to leverage the existing infrastructure? Well, I want to automatically inspect all the programs that crashed in a given system. I don't see how you can do that with a standalone daemon. Or maybe I didn't understand what you meant. I think you have misunderstood what I was trying to ask. We could in principle write a new daemon, call it crash-reporterd for now, and have the kernel notify that daemon whenever any program on the system crashes. But writing the infrastructure to support that would be a bunch of work, and we already have devd set up to get notifications from the kernel, so it is much faster to implement crash reporting in devd, even though crashes in software have nothing to do with device changes. The question boils down to: is the time saved by implementing it this way worth the tradeoff of architectural purity. I don't have an opinion myself, I just want to make sure the question is considered. -Ben Truth be told, it kind of bugs me. I think adding this to devctl and devd is inappropriate without also renaming those components to reflect their new role, and rewriting the manpages to reflect what they actually do now. If you ponder for a moment what the new role seems to be (generic notification to userland of events happening in the kernel), you end up with names like keventd and that makes you wonder whether a new type of knote for kevent, listened for by a new crashd app, would be better. -- Ian ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On Mon, Feb 9, 2015 at 6:13 PM, Rui Paulo rpa...@freebsd.org wrote: Author: rpaulo Date: Mon Feb 9 23:13:50 2015 New Revision: 278479 URL: https://svnweb.freebsd.org/changeset/base/278479 Log: Notify devd(8) when a process crashed. This change implements a notification (via devctl) to userland when the kernel produces coredumps after a process has crashed. devd can then run a specific command to produce a human readable crash report. The command is most usually a helper that runs gdb/lldb commands on the file/coredump pair. It's possible to use this functionality for implementing automatic generation of crash reports. devd(8) will be notified of the full path of the binary that crashed and the full path of the coredump file. What advantage does putting this in devd have over a standalone daemon for crash reporting? Is it just the ease of implementation to leverage the existing infrastructure? -Ben ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On Feb 09, 2015, at 03:16 PM, Benjamin Kaduk bjkf...@gmail.com wrote: On Mon, Feb 9, 2015 at 6:13 PM, Rui Paulo rpa...@freebsd.org wrote: Author: rpaulo Date: Mon Feb 9 23:13:50 2015 New Revision: 278479 URL: https://svnweb.freebsd.org/changeset/base/278479 Log: Notify devd(8) when a process crashed. This change implements a notification (via devctl) to userland when the kernel produces coredumps after a process has crashed. devd can then run a specific command to produce a human readable crash report. The command is most usually a helper that runs gdb/lldb commands on the file/coredump pair. It's possible to use this functionality for implementing automatic generation of crash reports. devd(8) will be notified of the full path of the binary that crashed and the full path of the coredump file. What advantage does putting this in devd have over a standalone daemon for crash reporting? Is it just the ease of implementation to leverage the existing infrastructure? Well, I want to automatically inspect all the programs that crashed in a given system. I don't see how you can do that with a standalone daemon. Or maybe I didn't understand what you meant. ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On Mon, Feb 09, 2015 at 11:13:51PM +, Rui Paulo wrote:
Author: rpaulo
Date: Mon Feb 9 23:13:50 2015
New Revision: 278479
URL: https://svnweb.freebsd.org/changeset/base/278479
Log:
Notify devd(8) when a process crashed.
This change implements a notification (via devctl) to userland when
the kernel produces coredumps after a process has crashed.
devd can then run a specific command to produce a human readable crash
report. The command is most usually a helper that runs gdb/lldb
commands on the file/coredump pair. It's possible to use this
functionality for implementing automatic generation of crash reports.
devd(8) will be notified of the full path of the binary that crashed and
the full path of the coredump file.
Arguably, there should be a knob, probably sysctl, to turn the
functionality off. I definitely do not want this on crash boxes used for
userspace debugging. Even despite the example handler is inactive.
Modified:
head/etc/devd.conf
head/sys/kern/kern_sig.c
Modified: head/etc/devd.conf
==
--- head/etc/devd.confMon Feb 9 23:04:30 2015(r278478)
+++ head/etc/devd.confMon Feb 9 23:13:50 2015(r278479)
@@ -325,4 +325,16 @@ notify 100 {
action /usr/sbin/automount -c;
};
+# Handle userland coredumps.
+# This commented out handler makes it possible to run an
+# automated debugging session after the core dump is generated.
+# Replace action with a proper coredump handler, but be aware that
+# it will run with elevated privileges.
+notify 10 {
+ match system kernel;
+ match subsystem signal;
+ match typecoredump;
+ action logger $comm $core;
+};
+
*/
Modified: head/sys/kern/kern_sig.c
==
--- head/sys/kern/kern_sig.c Mon Feb 9 23:04:30 2015(r278478)
+++ head/sys/kern/kern_sig.c Mon Feb 9 23:13:50 2015(r278479)
@@ -46,6 +46,7 @@ __FBSDID($FreeBSD$);
#include sys/signalvar.h
#include sys/vnode.h
#include sys/acct.h
+#include sys/bus.h
#include sys/capsicum.h
#include sys/condvar.h
#include sys/event.h
@@ -3237,6 +3238,9 @@ coredump(struct thread *td)
void *rl_cookie;
off_t limit;
int compress;
+ char *data = NULL;
+ size_t len;
+ char *fullpath, *freepath = NULL;
#ifdef COMPRESS_USER_CORES
compress = compress_user_cores;
@@ -3322,9 +3326,36 @@ close:
error1 = vn_close(vp, FWRITE, cred, td);
if (error == 0)
error = error1;
+ else
+ goto out;
+ /*
+ * Notify the userland helper that a process triggered a core dump.
+ * This allows the helper to run an automated debugging session.
+ */
+ len = MAXPATHLEN * 2 + 5 /* comm= */ + 5 /* core= */ + 1;
It is much cleaner to use static const char arrays for the names,
and use sizeof() - 1 instead of hard-coding commented constants.
+ data = malloc(len, M_TEMP, M_NOWAIT);
Why is this allocation M_NOWAIT ?
+ if (data == NULL)
+ goto out;
+ if (vn_fullpath_global(td, p-p_textvp, fullpath, freepath) != 0)
+ goto out;
+ snprintf(data, len, comm=%s, fullpath);
+ if (freepath != NULL) {
+ free(freepath, M_TEMP);
Checks for NULL pointer before free(9) are redundant.
+ freepath = NULL;
+ }
+ if (vn_fullpath_global(td, vp, fullpath, freepath) != 0)
+ goto out;
+ snprintf(data, len, %s core=%s, data, fullpath);
This is weird, and highly depends on the implementation details, supplying
the same string as target and source. IMO strcat(9) is enough there.
+ devctl_notify(kernel, signal, coredump, data);
+ free(name, M_TEMP);
+out:
#ifdef AUDIT
audit_proc_coredump(td, name, error);
#endif
+ if (freepath != NULL)
+ free(freepath, M_TEMP);
+ if (data != NULL)
+ free(data, M_TEMP);
free(name, M_TEMP);
return (error);
}
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On Mon, Feb 9, 2015 at 6:22 PM, Rui Paulo rpa...@me.com wrote: On Feb 09, 2015, at 03:16 PM, Benjamin Kaduk bjkf...@gmail.com wrote: What advantage does putting this in devd have over a standalone daemon for crash reporting? Is it just the ease of implementation to leverage the existing infrastructure? Well, I want to automatically inspect all the programs that crashed in a given system. I don't see how you can do that with a standalone daemon. Or maybe I didn't understand what you meant. I think you have misunderstood what I was trying to ask. We could in principle write a new daemon, call it crash-reporterd for now, and have the kernel notify that daemon whenever any program on the system crashes. But writing the infrastructure to support that would be a bunch of work, and we already have devd set up to get notifications from the kernel, so it is much faster to implement crash reporting in devd, even though crashes in software have nothing to do with device changes. The question boils down to: is the time saved by implementing it this way worth the tradeoff of architectural purity. I don't have an opinion myself, I just want to make sure the question is considered. -Ben ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On Mon, Feb 09, 2015 at 11:13:51PM +, Rui Paulo wrote:
+notify 10 {
+ match system kernel;
+ match subsystem signal;
+ match typecoredump;
+ action logger $comm $core;
+};
+
*/
[..]
+ if (vn_fullpath_global(td, p-p_textvp, fullpath, freepath) != 0)
+ goto out;
+ snprintf(data, len, comm=%s, fullpath);
I cannot test it right now, but it looks like immediate privilege
escalation.
Path is not sanitized in any way and devd passes it to 'sh -c'.
So a file named a.out; /bin/id; meh or so should result in execution
of aforementioned /bin/id.
Another note is that currently devctl is record oriented, but this may
change at some point and free form userspace text could be used to forge
new events.
As such is trongly suggest we sanitize this somehow. Maybe a base64 or
something.
--
Mateusz Guzik mjguzik gmail.com
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r278479 - in head: etc sys/kern
On Feb 9, 2015, at 19:11, Don Lewis truck...@freebsd.org wrote:
On 10 Feb, Mateusz Guzik wrote:
On Mon, Feb 09, 2015 at 11:13:51PM +, Rui Paulo wrote:
+notify 10 {
+ match system kernel;
+ match subsystem signal;
+ match typecoredump;
+ action logger $comm $core;
+};
+
*/
[..]
+ if (vn_fullpath_global(td, p-p_textvp, fullpath, freepath) != 0)
+ goto out;
+ snprintf(data, len, comm=%s, fullpath);
I cannot test it right now, but it looks like immediate privilege
escalation.
Path is not sanitized in any way and devd passes it to 'sh -c'.
So a file named a.out; /bin/id; meh or so should result in execution
of aforementioned /bin/id.
Then there is the issue of a user-generated core file being fed into the
crash analyzer, possibly exploiting bugs in the latter.
That's why there's a warning in devd.conf: devd will run the helper as root, so
a proper written helper has to drop the privileges very early or be invoked by
devd with lower privileges. My helper just drops privileges to match the
UID/GID of the generated core file before doing anything else.
--
Rui Paulo
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
