Skip to content

Commit a2a8474

Browse files
oleg-nesterovtorvalds
oleg-nesterov
authored and
torvalds
committed
exec: do not sleep in TASK_TRACED under ->cred_guard_mutex
Tom Horsley reports that his debugger hangs when it tries to read /proc/pid_of_tracee/maps, this happens since "mm_for_maps: take ->cred_guard_mutex to fix the race with exec" 04b836cbf19e885f8366bccb2e4b0474346c02d commit in 2.6.31. But the root of the problem lies in the fact that do_execve() path calls tracehook_report_exec() which can stop if the tracer sets PT_TRACE_EXEC. The tracee must not sleep in TASK_TRACED holding this mutex. Even if we remove ->cred_guard_mutex from mm_for_maps() and proc_pid_attr_write(), another task doing PTRACE_ATTACH should not hang until it is killed or the tracee resumes. With this patch do_execve() does not use ->cred_guard_mutex directly and we do not hold it throughout, instead: - introduce prepare_bprm_creds() helper, it locks the mutex and calls prepare_exec_creds() to initialize bprm->cred. - install_exec_creds() drops the mutex after commit_creds(), and thus before tracehook_report_exec()->ptrace_stop(). or, if exec fails, free_bprm() drops this mutex when bprm->cred != NULL which indicates install_exec_creds() was not called. Reported-by: Tom Horsley <tom.horsley@att.net> Signed-off-by: Oleg Nesterov <oleg@redhat.com> Acked-by: David Howells <dhowells@redhat.com> Cc: Roland McGrath <roland@redhat.com> Cc: James Morris <jmorris@namei.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
1 parent dd5d241 commit a2a8474

File tree

3 files changed

+43
-38
lines changed

3 files changed

+43
-38
lines changed

‎fs/compat.c

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1485,20 +1485,15 @@ int compat_do_execve(char * filename,
14851485
if (!bprm)
14861486
goto out_files;
14871487

1488-
retval = -ERESTARTNOINTR;
1489-
if (mutex_lock_interruptible(&current->cred_guard_mutex))
1488+
retval = prepare_bprm_creds(bprm);
1489+
if (retval)
14901490
goto out_free;
1491-
current->in_execve = 1;
1492-
1493-
retval = -ENOMEM;
1494-
bprm->cred = prepare_exec_creds();
1495-
if (!bprm->cred)
1496-
goto out_unlock;
14971491

14981492
retval = check_unsafe_exec(bprm);
14991493
if (retval < 0)
1500-
goto out_unlock;
1494+
goto out_free;
15011495
clear_in_exec = retval;
1496+
current->in_execve = 1;
15021497

15031498
file = open_exec(filename);
15041499
retval = PTR_ERR(file);
@@ -1547,7 +1542,6 @@ int compat_do_execve(char * filename,
15471542
/* execve succeeded */
15481543
current->fs->in_exec = 0;
15491544
current->in_execve = 0;
1550-
mutex_unlock(&current->cred_guard_mutex);
15511545
acct_update_integrals(current);
15521546
free_bprm(bprm);
15531547
if (displaced)
@@ -1567,10 +1561,7 @@ int compat_do_execve(char * filename,
15671561
out_unmark:
15681562
if (clear_in_exec)
15691563
current->fs->in_exec = 0;
1570-
1571-
out_unlock:
15721564
current->in_execve = 0;
1573-
mutex_unlock(&current->cred_guard_mutex);
15741565

15751566
out_free:
15761567
free_bprm(bprm);

‎fs/exec.c

Lines changed: 38 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,6 +1015,35 @@ int flush_old_exec(struct linux_binprm * bprm)
10151015

10161016
EXPORT_SYMBOL(flush_old_exec);
10171017

1018+
/*
1019+
* Prepare credentials and lock ->cred_guard_mutex.
1020+
* install_exec_creds() commits the new creds and drops the lock.
1021+
* Or, if exec fails before, free_bprm() should release ->cred and
1022+
* and unlock.
1023+
*/
1024+
int prepare_bprm_creds(struct linux_binprm *bprm)
1025+
{
1026+
if (mutex_lock_interruptible(&current->cred_guard_mutex))
1027+
return -ERESTARTNOINTR;
1028+
1029+
bprm->cred = prepare_exec_creds();
1030+
if (likely(bprm->cred))
1031+
return 0;
1032+
1033+
mutex_unlock(&current->cred_guard_mutex);
1034+
return -ENOMEM;
1035+
}
1036+
1037+
void free_bprm(struct linux_binprm *bprm)
1038+
{
1039+
free_arg_pages(bprm);
1040+
if (bprm->cred) {
1041+
mutex_unlock(&current->cred_guard_mutex);
1042+
abort_creds(bprm->cred);
1043+
}
1044+
kfree(bprm);
1045+
}
1046+
10181047
/*
10191048
* install the new credentials for this executable
10201049
*/
@@ -1024,12 +1053,13 @@ void install_exec_creds(struct linux_binprm *bprm)
10241053

10251054
commit_creds(bprm->cred);
10261055
bprm->cred = NULL;
1027-
1028-
/* cred_guard_mutex must be held at least to this point to prevent
1056+
/*
1057+
* cred_guard_mutex must be held at least to this point to prevent
10291058
* ptrace_attach() from altering our determination of the task's
1030-
* credentials; any time after this it may be unlocked */
1031-
1059+
* credentials; any time after this it may be unlocked.
1060+
*/
10321061
security_bprm_committed_creds(bprm);
1062+
mutex_unlock(&current->cred_guard_mutex);
10331063
}
10341064
EXPORT_SYMBOL(install_exec_creds);
10351065

@@ -1246,14 +1276,6 @@ int search_binary_handler(struct linux_binprm *bprm,struct pt_regs *regs)
12461276

12471277
EXPORT_SYMBOL(search_binary_handler);
12481278

1249-
void free_bprm(struct linux_binprm *bprm)
1250-
{
1251-
free_arg_pages(bprm);
1252-
if (bprm->cred)
1253-
abort_creds(bprm->cred);
1254-
kfree(bprm);
1255-
}
1256-
12571279
/*
12581280
* sys_execve() executes a new program.
12591281
*/
@@ -1277,20 +1299,15 @@ int do_execve(char * filename,
12771299
if (!bprm)
12781300
goto out_files;
12791301

1280-
retval = -ERESTARTNOINTR;
1281-
if (mutex_lock_interruptible(&current->cred_guard_mutex))
1302+
retval = prepare_bprm_creds(bprm);
1303+
if (retval)
12821304
goto out_free;
1283-
current->in_execve = 1;
1284-
1285-
retval = -ENOMEM;
1286-
bprm->cred = prepare_exec_creds();
1287-
if (!bprm->cred)
1288-
goto out_unlock;
12891305

12901306
retval = check_unsafe_exec(bprm);
12911307
if (retval < 0)
1292-
goto out_unlock;
1308+
goto out_free;
12931309
clear_in_exec = retval;
1310+
current->in_execve = 1;
12941311

12951312
file = open_exec(filename);
12961313
retval = PTR_ERR(file);
@@ -1340,7 +1357,6 @@ int do_execve(char * filename,
13401357
/* execve succeeded */
13411358
current->fs->in_exec = 0;
13421359
current->in_execve = 0;
1343-
mutex_unlock(&current->cred_guard_mutex);
13441360
acct_update_integrals(current);
13451361
free_bprm(bprm);
13461362
if (displaced)
@@ -1360,10 +1376,7 @@ int do_execve(char * filename,
13601376
out_unmark:
13611377
if (clear_in_exec)
13621378
current->fs->in_exec = 0;
1363-
1364-
out_unlock:
13651379
current->in_execve = 0;
1366-
mutex_unlock(&current->cred_guard_mutex);
13671380

13681381
out_free:
13691382
free_bprm(bprm);

‎include/linux/binfmts.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ extern int setup_arg_pages(struct linux_binprm * bprm,
117117
int executable_stack);
118118
extern int bprm_mm_init(struct linux_binprm *bprm);
119119
extern int copy_strings_kernel(int argc,char ** argv,struct linux_binprm *bprm);
120+
extern int prepare_bprm_creds(struct linux_binprm *bprm);
120121
extern void install_exec_creds(struct linux_binprm *bprm);
121122
extern void do_coredump(long signr, int exit_code, struct pt_regs *regs);
122123
extern int set_binfmt(struct linux_binfmt *new);

0 commit comments

Comments
 (0)