Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 06 Jan 2016 05:32:44 +0000
From:      bugzilla-noreply@freebsd.org
To:        freebsd-bugs@FreeBSD.org
Subject:   [Bug 178396] [kernel] [patch] Add jid to kernel log when a process has been forced closed
Message-ID:  <bug-178396-8-3kkOyyauoS@https.bugs.freebsd.org/bugzilla/>
In-Reply-To: <bug-178396-8@https.bugs.freebsd.org/bugzilla/>
References:  <bug-178396-8@https.bugs.freebsd.org/bugzilla/>

next in thread | previous in thread | raw e-mail | index | archive | help
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D178396

Mateusz Guzik <mjg@FreeBSD.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mjg@FreeBSD.org

--- Comment #4 from Mateusz Guzik <mjg@FreeBSD.org> ---
The feature is definitely desirable.

I would argue the complete solution would just support jail-aware dmesgs and
print jail-specific stuff specific stuff with appropriate prefix to the 'ma=
in'
dmesg. This would require some effort and may be off the table for now.

Regardless, it would be good if the new message here got the format one wou=
ld
expect to see in the more advanced case.

There is further issue with increased infoleaks - now not only you learn wh=
at
segfaulting programs are being used by other jails, you get their (host)nam=
es.

Either way, the patch is wrong:

+                       if (jailed(p->p_ucred)) {
+                               char buf[MAXHOSTNAMELEN + 3];
+                               if (strcmp(p->p_ucred->cr_prison->pr_hostna=
me,
"") !=3D 0) {
+                                       sprintf(buf, " (%s)",
p->p_ucred->cr_prison->pr_hostname);
+                               } else {
+                                       *buf =3D '\0';
+                               }

This should have used getcredhostname, assuming hostname is desirable. I wo=
uld
argue hostname is not the field you are interested in - after all, jail can
change it. Instead, you should obtain jail name.

Also, this patch does not handle hierarchical jails.

+                               log(LOG_INFO,
+                                   "pid %d (%s), uid %d, jid %d%s: exited =
on
signal %d%s\n",
+                                   p->p_pid, p->p_comm,
+                                   td->td_ucred->cr_uid,
+                                   p->p_ucred->cr_prison->pr_id,
+                                   buf,
+                                   sig &~ WCOREFLAG,
+                                   sig & WCOREFLAG ? " (core dumped)" : ""=
);
+                       } else {
+                               log(LOG_INFO,
+                                   "pid %d (%s), uid %d: exited on signal
%d%s\n",
+                                   p->p_pid, p->p_comm,
+                                   td->td_ucred ? td->td_ucred->cr_uid : -=
1,
+                                   sig &~ WCOREFLAG,
+                                   sig & WCOREFLAG ? " (core dumped)" : ""=
);
+                       }

As a nit, just should have been handled with one log() call. Missing option=
al
jail bit could be provided with a pointer to "".

That said, I would be in favor of messages like this one:
[$jailname] $msg

That is:
pid 857 (perl), uid 1001: exited on signal 6

is turned into:
[foo] pid 857 (perl), uid 1001: exited on signal 6

Assuming jail name is 'foo'. For hierarchical jails this would be [foo.bar].

--=20
You are receiving this mail because:
You are the assignee for the bug.=



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bug-178396-8-3kkOyyauoS>