d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Do not run pmclog_configure_log() without pmc_sx protection.

Browse Source 
The r195005 unlocked pmc_sx before calling into pmclog_configure_log()
to avoid the LOR, but it allows flush or closelog to run in parallel
with the configuration, causing many failure modes.

Revert r195005.  Pre-create the logging process, allowing it to run
after the set up succeeded, otherwise the process terminates itself.

Reported and tested by:	pho
Reviewed by:	markj
Sponsored by:	The FreeBSD Foundation
MFC after:	1 week
Differential revision:	https://reviews.freebsd.org/D12882
This commit is contained in:
Konstantin Belousov 2017-11-01 11:43:39 +00:00
parent 1121a37474
commit 20b555e146
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=325277
3 changed files with 109 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

94
sys/dev/hwpmc/hwpmc_logging.c
View File

@ -235,6 +235,54 @@ pmclog_get_buffer(struct pmc_owner *po)
return (plb ? 0 : ENOMEM);
}
struct pmclog_proc_init_args {
struct proc *kthr;
struct pmc_owner *po;
bool exit;
bool acted;
};
int
pmclog_proc_create(struct thread *td, void **handlep)
{
struct pmclog_proc_init_args *ia;
int error;
ia = malloc(sizeof(*ia), M_TEMP, M_WAITOK | M_ZERO);
error = kproc_create(pmclog_loop, ia, &ia->kthr,
RFHIGHPID, 0, "hwpmc: proc(%d)", td->td_proc->p_pid);
if (error == 0)
*handlep = ia;
return (error);
}
void
pmclog_proc_ignite(void *handle, struct pmc_owner *po)
{
struct pmclog_proc_init_args *ia;
ia = handle;
mtx_lock(&pmc_kthread_mtx);
MPASS(!ia->acted);
MPASS(ia->po == NULL);
MPASS(!ia->exit);
MPASS(ia->kthr != NULL);
if (po == NULL) {
ia->exit = true;
} else {
ia->po = po;
KASSERT(po->po_kthread == NULL,
("[pmclog,%d] po=%p kthread (%p) already present",
__LINE__, po, po->po_kthread));
po->po_kthread = ia->kthr;
}
wakeup(ia);
while (!ia->acted)
msleep(ia, &pmc_kthread_mtx, PWAIT, "pmclogw", 0);
mtx_unlock(&pmc_kthread_mtx);
free(ia, M_TEMP);
}
/*
* Log handler loop.
*
@ -244,7 +292,7 @@ pmclog_get_buffer(struct pmc_owner *po)
static void
pmclog_loop(void *arg)
{
int error;
struct pmclog_proc_init_args *ia;
struct pmc_owner *po;
struct pmclog_buffer *lb;
struct proc *p;
@ -255,15 +303,34 @@ pmclog_loop(void *arg)
struct uio auio;
struct iovec aiov;
size_t nbytes;
int error;
po = (struct pmc_owner *) arg;
p = po->po_owner;
td = curthread;
SIGEMPTYSET(unb);
SIGADDSET(unb, SIGHUP);
(void)kern_sigprocmask(td, SIG_UNBLOCK, &unb, NULL, 0);
ia = arg;
MPASS(ia->kthr == curproc);
MPASS(!ia->acted);
mtx_lock(&pmc_kthread_mtx);
while (ia->po == NULL && !ia->exit)
msleep(ia, &pmc_kthread_mtx, PWAIT, "pmclogi", 0);
if (ia->exit) {
ia->acted = true;
wakeup(ia);
mtx_unlock(&pmc_kthread_mtx);
kproc_exit(0);
}
MPASS(ia->po != NULL);
po = ia->po;
ia->acted = true;
wakeup(ia);
mtx_unlock(&pmc_kthread_mtx);
ia = NULL;
p = po->po_owner;
mycred = td->td_ucred;
PROC_LOCK(p);
@ -568,15 +635,11 @@ pmclog_stop_kthread(struct pmc_owner *po)
int
pmclog_configure_log(struct pmc_mdep *md, struct pmc_owner *po, int logfd)
{
int error;
struct proc *p;
cap_rights_t rights;
/*
* As long as it is possible to get a LOR between pmc_sx lock and
* proctree/allproc sx locks used for adding a new process, assure
* the former is not held here.
*/
sx_assert(&pmc_sx, SA_UNLOCKED);
int error;
sx_assert(&pmc_sx, SA_XLOCKED);
PMCDBG2(LOG,CFG,1, "config po=%p logfd=%d", po, logfd);
p = po->po_owner;
@ -585,9 +648,6 @@ pmclog_configure_log(struct pmc_mdep *md, struct pmc_owner *po, int logfd)
if (po->po_flags & PMC_PO_OWNS_LOGFILE)
return (EBUSY);
KASSERT(po->po_kthread == NULL,
("[pmclog,%d] po=%p kthread (%p) already present", __LINE__, po,
po->po_kthread));
KASSERT(po->po_file == NULL,
("[pmclog,%d] po=%p file (%p) already present", __LINE__, po,
po->po_file));
@ -600,10 +660,6 @@ pmclog_configure_log(struct pmc_mdep *md, struct pmc_owner *po, int logfd)
/* mark process as owning a log file */
po->po_flags |= PMC_PO_OWNS_LOGFILE;
error = kproc_create(pmclog_loop, po, &po->po_kthread,
RFHIGHPID, 0, "hwpmc: proc(%d)", p->p_pid);
if (error)
goto error;
/* mark process as using HWPMCs */
PROC_LOCK(p);
@ -620,10 +676,6 @@ pmclog_configure_log(struct pmc_mdep *md, struct pmc_owner *po, int logfd)
return (0);
error:
/* shutdown the thread */
if (po->po_kthread)
pmclog_stop_kthread(po);
KASSERT(po->po_kthread == NULL, ("[pmclog,%d] po=%p kthread not "
"stopped", __LINE__, po));

58
sys/dev/hwpmc/hwpmc_mod.c
View File

@ -2854,19 +2854,30 @@ static const char *pmc_op_to_name[] = {
static int
pmc_syscall_handler(struct thread *td, void *syscall_args)
{
int error, is_sx_downgraded, is_sx_locked, op;
int error, is_sx_downgraded, op;
struct pmc_syscall_args *c;
void *pmclog_proc_handle;
void *arg;
PMC_GET_SX_XLOCK(ENOSYS);
is_sx_downgraded = 0;
is_sx_locked = 1;
c = (struct pmc_syscall_args *) syscall_args;
c = (struct pmc_syscall_args *)syscall_args;
op = c->pmop_code;
arg = c->pmop_data;
if (op == PMC_OP_CONFIGURELOG) {
/*
* We cannot create the logging process inside
* pmclog_configure_log() because there is a LOR
* between pmc_sx and process structure locks.
* Instead, pre-create the process and ignite the loop
* if everything is fine, otherwise direct the process
* to exit.
*/
error = pmclog_proc_create(td, &pmclog_proc_handle);
if (error != 0)
goto done_syscall;
}
PMC_GET_SX_XLOCK(ENOSYS);
is_sx_downgraded = 0;
PMCDBG3(MOD,PMS,1, "syscall op=%d \"%s\" arg=%p", op,
pmc_op_to_name[op], arg);
@ -2890,15 +2901,16 @@ pmc_syscall_handler(struct thread *td, void *syscall_args)
struct pmc_owner *po;
struct pmc_op_configurelog cl;
sx_assert(&pmc_sx, SX_XLOCKED);
if ((error = copyin(arg, &cl, sizeof(cl))) != 0)
if ((error = copyin(arg, &cl, sizeof(cl))) != 0) {
pmclog_proc_ignite(pmclog_proc_handle, NULL);
break;
}
/* mark this process as owning a log file */
p = td->td_proc;
if ((po = pmc_find_owner_descriptor(p)) == NULL)
if ((po = pmc_allocate_owner_descriptor(p)) == NULL) {
pmclog_proc_ignite(pmclog_proc_handle, NULL);
error = ENOMEM;
break;
}
@ -2910,10 +2922,11 @@ pmc_syscall_handler(struct thread *td, void *syscall_args)
* de-configure it.
*/
if (cl.pm_logfd >= 0) {
sx_xunlock(&pmc_sx);
is_sx_locked = 0;
error = pmclog_configure_log(md, po, cl.pm_logfd);
pmclog_proc_ignite(pmclog_proc_handle, error == 0 ?
po : NULL);
} else if (po->po_flags & PMC_PO_OWNS_LOGFILE) {
pmclog_proc_ignite(pmclog_proc_handle, NULL);
pmclog_process_closelog(po);
error = pmclog_close(po);
if (error == 0) {
@ -2923,11 +2936,10 @@ pmc_syscall_handler(struct thread *td, void *syscall_args)
pmc_stop(pm);
error = pmclog_deconfigure_log(po);
}
} else
} else {
pmclog_proc_ignite(pmclog_proc_handle, NULL);
error = EINVAL;
if (error)
break;
}
}
break;
@ -4022,13 +4034,11 @@ pmc_syscall_handler(struct thread *td, void *syscall_args)
break;
}
if (is_sx_locked != 0) {
if (is_sx_downgraded)
sx_sunlock(&pmc_sx);
else
sx_xunlock(&pmc_sx);
}
if (is_sx_downgraded)
sx_sunlock(&pmc_sx);
else
sx_xunlock(&pmc_sx);
done_syscall:
if (error)
atomic_add_int(&pmc_stats.pm_syscall_errors, 1);

2
sys/sys/pmclog.h
View File

@ -260,6 +260,8 @@ int pmclog_deconfigure_log(struct pmc_owner *_po);
int pmclog_flush(struct pmc_owner *_po);
int pmclog_close(struct pmc_owner *_po);
void pmclog_initialize(void);
int pmclog_proc_create(struct thread *td, void **handlep);
void pmclog_proc_ignite(void *handle, struct pmc_owner *po);
void pmclog_process_callchain(struct pmc *_pm, struct pmc_sample *_ps);
void pmclog_process_closelog(struct pmc_owner *po);
void pmclog_process_dropnotify(struct pmc_owner *po);