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-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r278474 - head/sys/sys
On Feb 09, 2015, at 03:17 PM, Jung-uk Kim j...@freebsd.org wrote: I am fine with #if defined(__clang__) || !define(_KERNEL) case. Works for me. Note, however, that I'm fixing the original problem in the xz repository first. CPU_COUNT() will only be used by xz after we import 5.2.1. ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r278474 - head/sys/sys
On Monday, February 09, 2015 05:27:45 PM Jung-uk Kim wrote: On 02/09/2015 17:12, John Baldwin wrote: On Monday, February 09, 2015 04:55:52 PM Jung-uk Kim wrote: On 02/09/2015 16:08, John Baldwin wrote: On Monday, February 09, 2015 09:03:24 PM John Baldwin wrote: Author: jhb Date: Mon Feb 9 21:03:23 2015 New Revision: 278474 URL: https://svnweb.freebsd.org/changeset/base/278474 Log: Use __builtin_popcnt() to implement a BIT_COUNT() operation for bitsets and use this to implement CPU_COUNT() to count the number of CPUs in a cpuset. MFC after: 2 weeks Yes, __builtin_popcnt() works with GCC 4.2. It should also allow the compiler to DTRT in userland uses of this if -msse4.2 is enabled. Back in 2012, when I submitted a similar patch, bde noted __builtin_popcount*() cannot be used with GCC 4.2 for *kernel* because it emits a library call. http://docs.freebsd.org/cgi/mid.cgi?20121116171923.L1135 FYI... Weird, I though I built a kernel with this in a tree that uses it in the igb(4) driver. We need a CPU_COUNT() no matter what, but if this emits a library call under GCC I will need to add the call. We could also adopt your bitcount header, though I think it is more consistent to keep the loop in BIT_COUNT() and use something that emulates popcountl() rather than directly using bitcount() in BIT_COUNT() (primarily because the rest of sys/bitset.h is structured that way: explicit loops in sys/bitset.h itself). I think you should back it out for now and move the discussion to arch or hackers. I gave it up at the time but you may have better luck. :-) FYI, the following was the last version of my patch at the time. https://people.freebsd.org/~jkim/bitcount5.diff I could also just make it userland only for now? Rui wants to use it in userland. However, I can back it out if that is preferred. To be honest, I'm not sure how valuable it is at this point to expend a lot of effort to support GCC older than 3.4 (i.e. the non-builtin popcount approach). -- John Baldwin ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
svn commit: r278479 - in head: etc sys/kern
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.
Modified:
head/etc/devd.conf
head/sys/kern/kern_sig.c
Modified: head/etc/devd.conf
==
--- head/etc/devd.conf Mon Feb 9 23:04:30 2015(r278478)
+++ head/etc/devd.conf Mon 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.cMon Feb 9 23:04:30 2015(r278478)
+++ head/sys/kern/kern_sig.cMon 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;
+ data = malloc(len, M_TEMP, 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);
+ freepath = NULL;
+ }
+ if (vn_fullpath_global(td, vp, fullpath, freepath) != 0)
+ goto out;
+ snprintf(data, len, %s core=%s, data, fullpath);
+ 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-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r278474 - head/sys/sys
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 02/09/2015 18:08, John Baldwin wrote: On Monday, February 09, 2015 05:27:45 PM Jung-uk Kim wrote: On 02/09/2015 17:12, John Baldwin wrote: On Monday, February 09, 2015 04:55:52 PM Jung-uk Kim wrote: On 02/09/2015 16:08, John Baldwin wrote: On Monday, February 09, 2015 09:03:24 PM John Baldwin wrote: Author: jhb Date: Mon Feb 9 21:03:23 2015 New Revision: 278474 URL: https://svnweb.freebsd.org/changeset/base/278474 Log: Use __builtin_popcnt() to implement a BIT_COUNT() operation for bitsets and use this to implement CPU_COUNT() to count the number of CPUs in a cpuset. MFC after: 2 weeks Yes, __builtin_popcnt() works with GCC 4.2. It should also allow the compiler to DTRT in userland uses of this if -msse4.2 is enabled. Back in 2012, when I submitted a similar patch, bde noted __builtin_popcount*() cannot be used with GCC 4.2 for *kernel* because it emits a library call. http://docs.freebsd.org/cgi/mid.cgi?20121116171923.L1135 FYI... Weird, I though I built a kernel with this in a tree that uses it in the igb(4) driver. We need a CPU_COUNT() no matter what, but if this emits a library call under GCC I will need to add the call. We could also adopt your bitcount header, though I think it is more consistent to keep the loop in BIT_COUNT() and use something that emulates popcountl() rather than directly using bitcount() in BIT_COUNT() (primarily because the rest of sys/bitset.h is structured that way: explicit loops in sys/bitset.h itself). I think you should back it out for now and move the discussion to arch or hackers. I gave it up at the time but you may have better luck. :-) FYI, the following was the last version of my patch at the time. https://people.freebsd.org/~jkim/bitcount5.diff I could also just make it userland only for now? Rui wants to use it in userland. However, I can back it out if that is preferred. I am fine with #if defined(__clang__) || !define(_KERNEL) case. To be honest, I'm not sure how valuable it is at this point to expend a lot of effort to support GCC older than 3.4 (i.e. the non-builtin popcount approach). No, I don't care about GCC 3.x any more. Jung-uk Kim -BEGIN PGP SIGNATURE- Version: GnuPG v2 iQEcBAEBCAAGBQJU2UALAAoJEHyflib82/FGy8wH/jviXO8fcvprc1y4DGkc87rD ylBjoHimjsDt6Wr9MixYNZKbTXhd4BIgQVvFphgCshGjBbmkvII9DH/OleSTa/p8 /YJk87dZHE336An+GkXGTshESvLHw3l8hACR5FcussDLswJArBcDMuzbIW9Q7ASY dJsWTajP3r4rBqAtQQGxAmNfIvWC6iUP7mELSIoP8vBbUfO4HVZZWh7u5gqXxXnk dDX4kc7XuGweOtMydIY8bYiQYWb+IqMjnCEuucpJ4yktj3kUr8v9To+xAW6AmX7V OZQjMRzFbVKHPB6gs5fSaNZBD/D6MUpeDAXRQCy52xQB7vCQE1UNrR850upvxxY= =NARs -END PGP SIGNATURE- ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-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-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-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-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-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-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-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-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
svn commit: r278488 - head/usr.sbin/bsdconfig/usermgmt/share
Author: dteske Date: Tue Feb 10 02:53:26 2015 New Revision: 278488 URL: https://svnweb.freebsd.org/changeset/base/278488 Log: Whitespace. MFC after:3 days Modified: head/usr.sbin/bsdconfig/usermgmt/share/user.subr Modified: head/usr.sbin/bsdconfig/usermgmt/share/user.subr == --- head/usr.sbin/bsdconfig/usermgmt/share/user.subrTue Feb 10 02:02:24 2015(r278487) +++ head/usr.sbin/bsdconfig/usermgmt/share/user.subrTue Feb 10 02:53:26 2015(r278488) @@ -830,8 +830,7 @@ f_user_delete() f_eval_catch $funcname \ pw '%s -H 0' $cmd else - f_eval_catch $funcname \ - pw '%s -h -' $cmd + f_eval_catch $funcname pw '%s -h -' $cmd fi fi fi ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-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-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r278494 - head/sys/kern
On Mon, 2015-02-09 at 20:46 -0800, Rui Paulo wrote: On Feb 9, 2015, at 20:34, Rui Paulo rpa...@freebsd.org wrote: Author: rpaulo Date: Tue Feb 10 04:34:39 2015 New Revision: 278494 URL: https://svnweb.freebsd.org/changeset/base/278494 Log: Sanitise the coredump file names sent to devd. While there, add a sysctl to turn this feature off as requested by kib@. I wanted to get the sanitiser code in ASAP, but, as suggested by stas@ offline, we think devd should also provide an action mode that runs a command outside sh(1) and without using $PATH. -- Rui Paulo Or... we could consider restoring devd to its original relatively benign existance handling device-related events, and move handling of crash dumps into a separate daemon which can shoulder the burden of security for itself. At $work we listen to the devd re-distribute port to handle device events in our apps, and having an ever-growing flood of stuff that's got nothing to do with devices is going to have a negative impact on applications that do such things. -- Ian ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
svn commit: r278470 - head/usr.sbin/bsdconfig/includes
Author: dteske Date: Mon Feb 9 19:20:59 2015 New Revision: 278470 URL: https://svnweb.freebsd.org/changeset/base/278470 Log: Add new alias bsdconfig api (same as bsdconfig includes) NB: My fingers like typing api a lot more than includes MFC after:3 days Modified: head/usr.sbin/bsdconfig/includes/INDEX Modified: head/usr.sbin/bsdconfig/includes/INDEX == --- head/usr.sbin/bsdconfig/includes/INDEX Mon Feb 9 19:19:44 2015 (r278469) +++ head/usr.sbin/bsdconfig/includes/INDEX Mon Feb 9 19:20:59 2015 (r278470) @@ -45,6 +45,7 @@ menu_help= # can be i18n'ed but `command' is the name of a script. # menu_selection=includes|includes +menu_selection=api|includes # # Items below this line do NOT need i18n translation ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r278467 - head/usr.sbin/bsdconfig/share
On Mon, Feb 09, 2015 at 07:16:19PM +, Devin Teske wrote:
Author: dteske
Date: Mon Feb 9 19:16:19 2015
New Revision: 278467
URL: https://svnweb.freebsd.org/changeset/base/278467
Log:
Replace the only instance of sed(1) in bsdconfig(8) with awk(1).
MFC after: 3 days
Modified:
head/usr.sbin/bsdconfig/share/keymap.subr
Modified: head/usr.sbin/bsdconfig/share/keymap.subr
==
--- head/usr.sbin/bsdconfig/share/keymap.subr Mon Feb 9 17:53:16 2015
(r278466)
+++ head/usr.sbin/bsdconfig/share/keymap.subr Mon Feb 9 19:16:19 2015
(r278467)
@@ -219,7 +219,7 @@ f_keymap_get_all()
echo -n $k
# NOTE: Translate '8x8' to '8x08' before sending to
# sort(1) so that things work out as we might expect.
- debug= keymap_$k get desc | sed -e 's/8x8/8x08/g'
+ debug= keymap_$k get desc | awk 'gsub(/8x8/,8x08)||1'
done | sort -k2 | awk '{
printf %s%s, (started ? : ), $1; started = 1
}'
Out of curiosity what is the point of preferring awk over sed? the awk binary
being larger and depending on 2 libraries versus sed only depending on one?
Best regards,
Bapt
pgpID4jIDRlEe.pgp
Description: PGP signature
svn commit: r278494 - head/sys/kern
Author: rpaulo
Date: Tue Feb 10 04:34:39 2015
New Revision: 278494
URL: https://svnweb.freebsd.org/changeset/base/278494
Log:
Sanitise the coredump file names sent to devd.
While there, add a sysctl to turn this feature off as requested by
kib@.
Modified:
head/sys/kern/kern_sig.c
Modified: head/sys/kern/kern_sig.c
==
--- head/sys/kern/kern_sig.cTue Feb 10 03:34:42 2015(r278493)
+++ head/sys/kern/kern_sig.cTue Feb 10 04:34:39 2015(r278494)
@@ -42,6 +42,7 @@ __FBSDID($FreeBSD$);
#include opt_core.h
#include sys/param.h
+#include sys/ctype.h
#include sys/systm.h
#include sys/signalvar.h
#include sys/vnode.h
@@ -179,6 +180,10 @@ static int set_core_nodump_flag = 0;
SYSCTL_INT(_kern, OID_AUTO, nodump_coredump, CTLFLAG_RW, set_core_nodump_flag,
0, Enable setting the NODUMP flag on coredump files);
+static int coredump_devctl = 1;
+SYSCTL_INT(_kern, OID_AUTO, coredump_devctl, CTLFLAG_RW, coredump_devctl,
+ 0, Generate a devctl notification when processes coredump);
+
/*
* Signal properties and actions.
* The array below categorizes the signals and their default actions
@@ -3217,6 +3222,25 @@ out:
return (0);
}
+static int
+coredump_sanitise_path(const char *path)
+{
+ size_t len, i;
+
+ /*
+* Only send a subset of ASCII to devd(8) because it
+* might pass these strings to sh -c.
+*/
+ len = strlen(path);
+ for (i = 0; i len; i++)
+ if (!(isalpha(path[i]) || isdigit(path[i]))
+ path[i] != '/' path[i] != '.'
+ path[i] != '-')
+ return (0);
+
+ return (1);
+}
+
/*
* Dump a process' core. The main routine does some
* policy checking, and creates the name of the coredump;
@@ -3238,9 +3262,9 @@ coredump(struct thread *td)
void *rl_cookie;
off_t limit;
int compress;
- char *data = NULL;
- size_t len;
+ char data[MAXPATHLEN * 2 + 16]; /* space for devctl notification */
char *fullpath, *freepath = NULL;
+ size_t len;
#ifdef COMPRESS_USER_CORES
compress = compress_user_cores;
@@ -3332,30 +3356,29 @@ close:
* 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;
- data = malloc(len, M_TEMP, M_NOWAIT);
- if (data == NULL)
+ if (coredump_devctl == 0)
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);
- freepath = NULL;
+ if (!coredump_sanitise_path(fullpath))
+ goto out;
+ snprintf(data, sizeof(data), comm=%s , fullpath);
+ free(freepath, M_TEMP);
+ freepath = NULL;
+ if (vn_fullpath_global(td, vp, fullpath, freepath) != 0) {
+ printf(could not find coredump\n);
+ goto out;
}
- if (vn_fullpath_global(td, vp, fullpath, freepath) != 0)
+ if (!coredump_sanitise_path(fullpath))
goto out;
- snprintf(data, len, %s core=%s, data, fullpath);
+ strlcat(data, core=, sizeof(data));
+ len = strlcat(data, fullpath, sizeof(data));
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(freepath, M_TEMP);
free(name, M_TEMP);
return (error);
}
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r278494 - head/sys/kern
On Feb 9, 2015, at 20:34, Rui Paulo rpa...@freebsd.org wrote: Author: rpaulo Date: Tue Feb 10 04:34:39 2015 New Revision: 278494 URL: https://svnweb.freebsd.org/changeset/base/278494 Log: Sanitise the coredump file names sent to devd. While there, add a sysctl to turn this feature off as requested by kib@. I wanted to get the sanitiser code in ASAP, but, as suggested by stas@ offline, we think devd should also provide an action mode that runs a command outside sh(1) and without using $PATH. -- Rui Paulo ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r278494 - head/sys/kern
On Feb 9, 2015, at 21:02, Ian Lepore i...@freebsd.org wrote: Or... we could consider restoring devd to its original relatively benign existance handling device-related events, and move handling of crash dumps into a separate daemon which can shoulder the burden of security for itself. At $work we listen to the devd re-distribute port to handle device events in our apps, and having an ever-growing flood of stuff that's got nothing to do with devices is going to have a negative impact on applications that do such things. That's perfectly reasonable given that devd handles everything: it gets notified when a new pty is created, when rctls reach their limit, when zfs does something, when devices show up/disappear, when you unplug the power adapter, when GEOM devices show up/disappear, when your CPU is too hot, when you press the brightness keys, etc. Some of these use cases are fine, others aren't. This is pretty much what happened to udev, if I'm not mistaken. We need a way to avoid reinventing the wheel. I really don't want to duplicate another /dev/devctl in the kernel. -- Rui Paulo ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
