prctl extension in a sake of c/r
cover prctl extension in a sake of c/r
Hi,
this series is resulting one from long discuss about ability to restore /proc/pid/exe. Hope it will fit everyone.
While exe symlink has been reviewed (and actually fixed, thanks a lot Oleg!) I've got no opinions on "Add ability to get clear_tid_address" one. Anyway both patches have been tested intensively.
Cyrill
PATCH 1/2 c/r: prctl: Add ability to set new mm_struct::exe_file
When we do restore we would like to have a way to setup a former mm_struct::exe_file so that /proc/pid/exe would point to the original executable file a process had at checkpoint time.
For this the PR_SET_MM_EXE_FILE code is introduced. This option takes a file descriptor which will be set as a source for new /proc/$pid/exe symlink.
Note it allows to change /proc/$pid/exe iif there are no VM_EXECUTABLE vmas present for current process, simply because this feature is a special to C/R and mm::num_exe_file_vmas become meaningless after that.
Also this action is one-shot only. For security reason we don't allow to change the symlink several times.
This feature is available iif CONFIG_CHECKPOINT_RESTORE is set.
--- include/linux/prctl.h | 1 kernel/sys.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+)
include/linux/prctl.h +1 -0
@@ -118,5 +118,6 @@# define PR_SET_MM_ENV_START 10# define PR_SET_MM_ENV_END 11# define PR_SET_MM_AUXV 12 +# define PR_SET_MM_EXE_FILE 13#endif /* _LINUX_PRCTL_H */
kernel/sys.c +56 -0
74 lines not on this page · raw message
Re: [patch 1/2] c/r: prctl: Add ability to set new mm_struct::exe_file
Quoted 9 lines · On Sat, 17 Mar 2012 00:55:57 +0400 Cyrill Gorcunov <[email protected]> wrote:
When we do restore we would like to have a way to setup a former
mm_struct::exe_fileso that/proc/pid/exewould point to the original executable file a process had at checkpoint time.For this the
PR_SET_MM_EXE_FILEcode is introduced. This option takes a file descriptor which will be set as a source for new/proc/$pid/exesymlink.
Quoted 8 lines
Note it allows to change
/proc/$pid/exeiif there are noVM_EXECUTABLEvmas present for current process, simply because this feature is a special to C/R andmm::num_exe_file_vmasbecome meaningless after that.Also this action is one-shot only. For security reason we don't allow to change the symlink several times.
What is this mysterious "security reason"?
Quoted 32 lines
...
kernel/sys.c +29
-0+static int prctl_set_mm_exe_file (struct mm_struct *mm,unsigned int fd) +{ +struct file *exe_file; +struct dentry *dentry; +int err; + +/* +* Setting new mm::exe_file is only allowed +* when no VM_EXECUTABLE vma's left. So make +* a fast test first. +*/ +if (mm->num_exe_file_vmas) +return -EBUSY ; + + exe_file =fget (fd); +if (!exe_file) +return -EBADF ; + + dentry = exe_file->f_path.dentry; + +/* +* Because the original mm->exe_file +* points to executable file, make sure +* this one is executable as well to not +* break an overall picture. +*/ + err = -EACCES ; +if (!S_ISREG (dentry->d_inode->i_mode) || + exe_file->f_path.mnt->mnt_flags &MNT_NOEXEC )
kernel/sys.c static bool vma_flags_mismatch(struct vm · line 1744
Quoted 10 lines
kernel/sys.c +10
-0+ goto exit; + + err =inode_permission (dentry->d_inode,MAY_EXEC ); +if (err) + goto exit; + +/* +* For security reason changing mm->exe_file +* is one-shot action. +*/
It should be explained here also. The comment is pretty useless - if we don't tell people what this "security reason" is, how can future developers be sure that they aren't violating it?
Quoted 18 lines
kernel/sys.c +12
-0+down_write (&mm->mmap_sem); +if (likely (!mm->exe_file)) +set_mm_exe_file (mm, exe_file); +else + err = -EBUSY ; +up_write (&mm->mmap_sem); + +exit: +fput (exe_file); +return err; +} +static int prctl_set_mm (int opt,unsigned long addr,unsigned long arg4,unsigned long arg5) {
Re: [patch 1/2] c/r: prctl: Add ability to set new mm_struct::exe_file
On Mon, Mar 19, 2012 at 03:15:07PM -0700, Andrew Morton wrote:
...
Quoted 6 lines
Quoted 3 lines
Also this action is one-shot only. For security reason we don't allow to change the symlink several times.
What is this mysterious "security reason"?
Oh, sorry I should have included Matt's comment here
| Before this patch that state was rather ephemeral and almost entirely | under the control of the kernel. The only way userspace could change it | was by unmapping the region(s) mapped during exec*(). At that point it | could not "lie" and insert some other symlink there and the admin would | be better able to determine what had happened. | | With this patch -- especially the multi-shot form -- the symlink will | be entirely under the control of (potentially untrusted) userspace code | and the admin is totally at the mercy of the userspace code. In | single-shot form programs could use the prctl() to ensure the symlink | could not be changed later -- the restart tool would be the only program | that would need to ensure that prctl() had not been used since the last | exec*().
...
Quoted 5 lines
It should be explained here also. The comment is pretty useless - if we don't tell people what this "security reason" is, how can future developers be sure that they aren't violating it?
Actually I liked multi-shot version more but Matt arguments convinced me that one-short fashion is more "secure" in terms of overall kernel state and potential transitions/changes of this /proc/pid/exe symlink.
At least with one-shot version the admin may be sure that the symlink is never changed more than once, ever.
Cyrill
Re: [patch 1/2] c/r: prctl: Add ability to set new mm_struct::exe_file
Quoted 31 lines · Cyrill Gorcunov
On Mon, Mar 19, 2012 at 03:15:07PM -0700, Andrew Morton wrote:
...Quoted 6 lines
Quoted 3 lines
Also this action is one-shot only. For security reason we don't allow to change the symlink several times.
What is this mysterious "security reason"?
Oh, sorry I should have included Matt's comment here
| Before this patch that state was rather ephemeral and almost entirely | under the control of the kernel. The only way userspace could change it | was by unmapping the region(s) mapped during exec*(). At that point it | could not "lie" and insert some other symlink there and the admin would | be better able to determine what had happened. | | With this patch -- especially the multi-shot form -- the symlink will | be entirely under the control of (potentially untrusted) userspace code | and the admin is totally at the mercy of the userspace code. In | single-shot form programs could use the prctl() to ensure the symlink | could not be changed later -- the restart tool would be the only program | that would need to ensure that prctl() had not been used since the last | exec*()....
Quoted 5 lines
It should be explained here also. The comment is pretty useless - if we don't tell people what this "security reason" is, how can future developers be sure that they aren't violating it?
Quoted 7 lines
Actually I liked multi-shot version more but Matt arguments convinced me that one-short fashion is more "secure" in terms of overall kernel state and potential transitions/changes of this
/proc/pid/exesymlink.At least with one-shot version the admin may be sure that the symlink is never changed more than once, ever.
And changing it once does not harm security? I'm sure that rootkit writers will like this feature...
Signature · 2 lines
Thanks, //richard
Re: [patch 1/2] c/r: prctl: Add ability to set new mm_struct::exe_file
Quoted 11 lines · On Mon, 19 Mar 2012 23:41:36 +0100 richard -rw- weinberger…
Quoted 10 lines · Cyrill Gorcunov
On Mon, Mar 19, 2012 at 03:15:07PM -0700, Andrew Morton wrote:
...Quoted 6 lines
Quoted 3 lines
Also this action is one-shot only. For security reason we don't allow to change the symlink several times.
What is this mysterious "security reason"?
Oh, sorry I should have included Matt's comment here
Please send a patch with the updated changelog and improved comment?
Quoted 11 lines
Quoted 8 lines
Actually I liked multi-shot version more but Matt arguments convinced me that one-short fashion is more "secure" in terms of overall kernel state and potential transitions/changes of this
/proc/pid/exesymlink.At least with one-shot version the admin may be sure that the symlink is never changed more than once, ever.
And changing it once does not harm security? I'm sure that rootkit writers will like this feature...
Well, let's discuss this more completely. In what ways could an attacker use this? How serious is the problem? What actions can be taken to lessen it? etcetera.
Re: [patch 1/2] c/r: prctl: Add ability to set new mm_struct::exe_file
Quoted 2 lines · Andrew Morton
Please send a patch with the updated changelog and improved comment?
Sure I'll resend.
Quoted 10 lines
Quoted 10 lines
Quoted 9 lines
Actually I liked multi-shot version more but Matt arguments convinced me that one-short fashion is more "secure" in terms of overall kernel state and potential transitions/changes of this
/proc/pid/exesymlink.At least with one-shot version the admin may be sure that the symlink is never changed more than once, ever.
Quoted 6 lines
Quoted 2 lines
And changing it once does not harm security? I'm sure that rootkit writers will like this feature...
Well, let's discuss this more completely. In what ways could an attacker use this? How serious is the problem? What actions can be taken to lessen it? etcetera.
It can use it iif CAP_SYS_RESOURCE is granted.
Otherwise you'll get -eaccess.
Cyrill
Re: [patch 1/2] c/r: prctl: Add ability to set new mm_struct::exe_file
Quoted 19 lines · On Tue, 20 Mar 2012 02:50:20 +0400 Cyrill Gorcunov <[email protected]> wrote:
Quoted 2 lines · Andrew Morton
Please send a patch with the updated changelog and improved comment?
Sure I'll resend.
Quoted 13 lines
Quoted 12 lines
Quoted 9 lines
Actually I liked multi-shot version more but Matt arguments convinced me that one-short fashion is more "secure" in terms of overall kernel state and potential transitions/changes of this
/proc/pid/exesymlink.At least with one-shot version the admin may be sure that the symlink is never changed more than once, ever.
And changing it once does not harm security? I'm sure that rootkit writers will like this feature...
Quoted 6 lines
Quoted 3 lines
Well, let's discuss this more completely. In what ways could an attacker use this? How serious is the problem? What actions can be taken to lessen it? etcetera.
It can use it iif
CAP_SYS_RESOURCEis granted.
Otherwise you'll get -eaccess.
A rootkit already obtained CAP_SYS_RESOURCE. What we're concerned about here is its ability to hide itself from view and its ability to obscure the way in which it obtained elevated privs.
How much this patch worsens the situation is unclear to me, so let's think it through.
Re: [patch 1/2] c/r: prctl: Add ability to set new mm_struct::exe_file
On Mon, Mar 19, 2012 at 03:59:26PM -0700, Andrew Morton wrote:
...
Quoted 7 lines
Quoted 3 lines
It can use it iif
CAP_SYS_RESOURCEis granted.
Otherwise you'll get -eaccess.A rootkit already obtained
CAP_SYS_RESOURCE. What we're concerned about here is its ability to hide itself from view and its ability to obscure the way in which it obtained elevated privs.
Well, if rootkit got CAP_SYS_RESOURCE I think we're in bad situation then -- it might change the symlink to some 'known' and trusted application and you'll never notice that (without scanning the memory area such rootkit uses, and note 'scanning' here because you need to scan for memory contents to figure out that memory do not correspond the file symlinks point to).
Actually being able to restore program 'transparently' is a primary aim of checkpoint-restore itself.
Quoted 3 lines
How much this patch worsens the situation is unclear to me, so let's think it through.
Dunno Andrew, /proc/exe/symlink is never trusted source of info I guess. But I need to think some more...
Cyrill
Re: [patch 1/2] c/r: prctl: Add ability to set new mm_struct::exe_file
In reply to Andrew MortonQuoted 3 lines · Andrew Morton
Well, let's discuss this more completely. In what ways could an attacker use this? How serious is the problem? What actions can be taken to lessen it? etcetera.
After considering the problem a bit more I think it's not a big problem. We must not trust /proc/pid/exe in anyway.
An attacker can always execute another binary without calling execve().
So, why makes that one-short fashion the feature more secure? Let the user change the exe symlink as often as he wants.
Quoted 1 line
From a security point of view the exe symlink is anyway useless.
Signature · 2 lines
Thanks, //richard
Re: [patch 1/2] c/r: prctl: Add ability to set new mm_struct::exe_file
Quoted 8 lines · richard -rw- weinberger
Quoted 3 lines · Andrew Morton
Well, let's discuss this more completely. In what ways could an attacker use this? How serious is the problem? What actions can be taken to lessen it? etcetera.
After considering the problem a bit more I think it's not a big problem. We must not trust
/proc/pid/exein anyway.
Well, Richard, we probably do not trust it anyway but sysadmins might do (and this was another reason for one-shot behaviour -- to not bring
heart attacks to sysadmins, and everyone would know this link might be changed only one time ;)
Quoted 1 line
An attacker can always execute another binary without calling
execve().
That's what c/r basically does :)
Quoted 4 lines
So, why makes that one-short fashion the feature more secure? Let the user change the exe symlink as often as he wants. From a security point of view the exe symlink is anyway useless.
Maybe better to call it 'predictable' then rather than 'secure'?
Cyrill
Re: [patch 1/2] c/r: prctl: Add ability to set new mm_struct::exe_file
Quoted 5 lines · Cyrill Gorcunov
Quoted 3 lines
So, why makes that one-short fashion the feature more secure? Let the user change the exe symlink as often as he wants. From a security point of view the exe symlink is anyway useless.
Maybe better to call it 'predictable' then rather than 'secure'?
I'd say one-short makes it less confusing. :-)
Signature · 2 lines
Thanks, //richard
Cyrill Gorcunov
Reviewed-by Oleg NesterovRe: [patch 1/2] c/r: prctl: Add ability to set new mm_struct::exe_file
In reply to Andrew MortonQuoted 8 lines · Andrew Morton
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
What is this mysterious "security reason"?
Oh, sorry I should have included Matt's comment here
Please send a patch with the updated changelog and improved comment?
Andrew, take a look please, will the changelog and comments look better?
Cyrill
---
From: Cyrill Gorcunov <[email protected]>
Subject: c/r: prctl: add ability to set new mm_struct::exe_file
When we do restore we would like to have a way to setup a former mm_struct::exe_file so that /proc/pid/exe would point to the original executable file a process had at checkpoint time.
For this the PR_SET_MM_EXE_FILE code is introduced. This option takes a file descriptor which will be set as a source for new /proc/$pid/exe symlink.
Note it allows to change /proc/$pid/exe iif there are no VM_EXECUTABLE vmas present for current process, simply because this feature is a special to C/R and mm::num_exe_file_vmas become meaningless after that.
To minimize the amount of transition the /proc/pid/exe symlink might have, this feature is implemented in one-shot manner. Thus once changed the symlink can't be changed again. This should help sysadmins to monitor the symlinks over all process running in a system.
In particular one could make a snapshot of processes and ring alarm if there unexpected changes of /proc/pid/exe's in a system.
Note -- this feature is available iif CONFIG_CHECKPOINT_RESTORE is set and the caller must have CAP_SYS_RESOURCE capability granted, otherwise the request to change symlink will be rejected.
--- include/linux/prctl.h | 1 kernel/sys.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+)
include/linux/prctl.h +1 -0
@@ -118,5 +118,6 @@# define PR_SET_MM_ENV_START 10# define PR_SET_MM_ENV_END 11# define PR_SET_MM_AUXV 12 +# define PR_SET_MM_EXE_FILE 13#endif /* _LINUX_PRCTL_H */
kernel/sys.c +56 -0
74 lines not on this page · raw message
Re: [patch 1/2] c/r: prctl: Add ability to set new mm_struct::exe_file
Quoted 5 lines · Cyrill Gorcunov
Quoted 4 lines · Andrew Morton
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
What is this mysterious "security reason"?
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Quoted 1 line
Oh, sorry I should have included Matt's comment here
Please send a patch with the updated changelog and improved comment?
Andrew, take a look please, will the changelog and comments look better?
Can you change this to take an actual address and get the exe_file from an mmapped area and make certain that the mmaped_area is already mapped MAP_EXEC.
That will prevent out-right lies.
At least then we will know that exe_file will at least be a file that is mapped executable in the process's address space. It's not a lot better but it makes /proc/<pid>/exe at almost as trustable as it is now.
Quoted 145 lines
Cyrill
---
From: Cyrill Gorcunov <[email protected]>
Subject: c/r: prctl: add ability to set newmm_struct::exe_fileWhen we do restore we would like to have a way to setup a former
mm_struct::exe_fileso that/proc/pid/exewould point to the original executable file a process had at checkpoint time.For this the
PR_SET_MM_EXE_FILEcode is introduced. This option takes a file descriptor which will be set as a source for new/proc/$pid/exesymlink.Note it allows to change
/proc/$pid/exeiif there are noVM_EXECUTABLEvmas present for current process, simply because this feature is a special to C/R andmm::num_exe_file_vmasbecome meaningless after that.To minimize the amount of transition the
/proc/pid/exesymlink might have, this feature is implemented in one-shot manner. Thus once changed the symlink can't be changed again. This should help sysadmins to monitor the symlinks over all process running in a system.In particular one could make a snapshot of processes and ring alarm if there unexpected changes of /proc/pid/exe's in a system.
Note -- this feature is available iif
CONFIG_CHECKPOINT_RESTOREis set and the caller must haveCAP_SYS_RESOURCEcapability granted, otherwise the request to change symlink will be rejected.Signed-off-by Cyrill Gorcunov <[email protected]>Reviewed-by Oleg Nesterov <[email protected]>CC KOSAKI Motohiro <[email protected]>CC Pavel Emelyanov <[email protected]>CC Kees Cook <[email protected]>CC Tejun Heo <[email protected]>CC Matt Helsley <[email protected]>CC richard -rw- weinberger <[email protected]>--- include/linux/prctl.h | 1 kernel/sys.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+)Index:
linux-2.6.git/include/linux/prctl.h
===================================================================include/linux/prctl.h +1
-0@@ -118,5 +118,6 @@# define PR_SET_MM_ENV_START 10# define PR_SET_MM_ENV_END 11# define PR_SET_MM_AUXV 12 +# define PR_SET_MM_EXE_FILE 13#endif /* _LINUX_PRCTL_H */ Index:
linux-2.6.git/kernel/sys.c
===================================================================kernel/sys.c +56
-1@@ -36,6 +36,8 @@#include <linux/personality.h> #include <linux/ptrace.h> #include <linux/fs_struct.h> +#include <linux/file.h> +#include <linux/mount.h> #include <linux/gfp.h> #include <linux/syscore_ops.h> #include <linux/version.h> @@ -1701,6 +1703,57 @@ static bool vma_flags_mismatch(struct vm (vma->vm_flags & banned); } +static int prctl_set_mm_exe_file (struct mm_struct *mm,unsigned int fd) +{ +struct file *exe_file; +struct dentry *dentry; +int err; + +/* +* Setting new mm::exe_file is only allowed when no VM_EXECUTABLE vma's +* remain. So perform a quick test first. +*/ +if (mm->num_exe_file_vmas) +return -EBUSY ; + + exe_file =fget (fd); +if (!exe_file) +return -EBADF ; + + dentry = exe_file->f_path.dentry; + +/* +* Because the original mm->exe_file points to executable file, make +* sure that this one is executable as well, to avoid breaking an +* overall picture. +*/ + err = -EACCES ; +if (!S_ISREG (dentry->d_inode->i_mode) || + exe_file->f_path.mnt->mnt_flags &MNT_NOEXEC ) + goto exit; + + err =inode_permission (dentry->d_inode,MAY_EXEC ); +if (err) + goto exit; + +/* +* The symlink can be changed only once, just to disallow arbitrary +* transitions malicious software might bring in. This means one +* could make a snapshot over all processes running and monitor +* /proc/pid/exe changes to notice unusual activity if needed. +*/ +down_write (&mm->mmap_sem); +if (likely (!mm->exe_file)) +set_mm_exe_file (mm, exe_file); +else + err = -EBUSY ; +up_write (&mm->mmap_sem); + +exit: +fput (exe_file); +return err; +} +static int prctl_set_mm (int opt,unsigned long addr,unsigned long arg4,unsigned long arg5) {@@ -1715,6 +1768,9 @@ static int prctl_set_mm(int opt, unsigneif (!capable (CAP_SYS_RESOURCE ))return -EPERM ; +if (opt ==PR_SET_MM_EXE_FILE ) +return prctl_set_mm_exe_file (mm, (unsigned int )addr); +if (addr >=TASK_SIZE )return -EINVAL ; --To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/2] c/r: prctl: Add ability to set new mm_struct::exe_file
Quoted 8 lines · Eric W. Biederman
Quoted 3 lines
Andrew, take a look please, will the changelog and comments look better?
Can you change this to take an actual address and get the
exe_filefrom an mmapped area and make certain that themmaped_areais already mappedMAP_EXEC.
Quoted 5 lines
That will prevent out-right lies.
At least then we will know that
exe_filewill at least be a file that is mapped executable in the process's address space. It's not a lot better but it makes /proc/<pid>/exe at almost as trustable as it is now.
This won't work for all cases. When we restore a program we map new VM_EXEC areas _without_ vma::vm_file field.
Cyrill
Re: [patch 1/2] c/r: prctl: Add ability to set new mm_struct::exe_file
Quoted 9 lines · Cyrill Gorcunov
Quoted 8 lines · Eric W. Biederman
Quoted 3 lines
Andrew, take a look please, will the changelog and comments look better?
Can you change this to take an actual address and get the
exe_filefrom an mmapped area and make certain that themmaped_areais already mappedMAP_EXEC.
Quoted 9 lines
Quoted 5 lines
That will prevent out-right lies.
At least then we will know that
exe_filewill at least be a file that is mapped executable in the process's address space. It's not a lot better but it makes /proc/<pid>/exe at almost as trustable as it is now.This won't work for all cases. When we restore a program we map new
VM_EXECareas _without_vma::vm_filefield.
Well, it's not complete true ;) At moment all exec areas we re-map during restore do correspond to executable files. but I think having ability to set this symlink without requirement such as 'every VM_EXEC should be mapped with file' is a win. That said I would prefer to leave this interface as is, until there strong objections.
Cyrill
Re: [patch 1/2] c/r: prctl: Add ability to set new mm_struct::exe_file
In reply to Eric W. BiedermanQuoted 12 lines · Eric W. Biederman
Quoted 10 lines · Cyrill Gorcunov
Quoted 8 lines · Andrew Morton
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
What is this mysterious "security reason"?
Oh, sorry I should have included Matt's comment here
Please send a patch with the updated changelog and improved comment?
Quoted 6 lines
Quoted 2 lines
Andrew, take a look please, will the changelog and comments look better?
Can you change this to take an actual address and get the
exe_filefrom an mmapped area and make certain that themmaped_areais already mappedMAP_EXEC.
Do you mean PROT_EXEC/VM_EXEC?
Quoted 6 lines
That will prevent out-right lies.
At least then we will know that
exe_filewill at least be a file that is mapped executable in the process's address space. It's not a lot better but it makes /proc/<pid>/exe at almost as trustable as it is now.
I don't dislike the idea. However just because it's mapped with one of those flags does not mean that a single instruction of it will ever be executed. So it's not much better than using the fd :/.
Perhaps there is some way to use the userspace stack and/or regs to get a reasonable instruction pointer, lookup its VMA, and use that? I'm not sure that would work for c/r though...
Cheers,
-Matt
Re: [patch 1/2] c/r: prctl: Add ability to set new mm_struct::exe_file
In reply to richard -rw- weinbergerQuoted 11 lines · richard -rw- weinberger
Quoted 8 lines
Actually I liked multi-shot version more but Matt arguments convinced me that one-short fashion is more "secure" in terms of overall kernel state and potential transitions/changes of this
/proc/pid/exesymlink.At least with one-shot version the admin may be sure that the symlink is never changed more than once, ever.
And changing it once does not harm security? I'm sure that rootkit writers will like this feature...
The one-shot limits the amount of transitions, but you still have to obtain CAP_SYS_RESOURCE before you'll be able to change this symlink (ie it's not 'anyone-can-change-it' feature).
Cyrill
PATCH 2/2 c/r: prctl: Add ability to get clear_tid_address
In reply to Cyrill GorcunovZero is written at clear_tid_address, when
the process exits. This functionality is used
by pthread_join().
We already have sys_set_tid_address() to change this address for current task but there is no way to obtain it from a user space.
Without ability to find this address and dump it we can't restore pthread'ed apps which do call pthread_join() once they have been restored.
This patch introduces PR_GET_TID_ADDRESS prctl option which allow current process to obtain own clear_tid_address.
This feature is available iif CONFIG_CHECKPOINT_RESTORE is set.
--- include/linux/prctl.h | 2 ++ kernel/sys.c | 13 +++++++++++++ 2 files changed, 15 insertions(+)
include/linux/prctl.h +2 -0
@@ -120,4 +120,6 @@# define PR_SET_MM_AUXV 12# define PR_SET_MM_EXE_FILE 13 +#define PR_GET_TID_ADDRESS 36 +#endif /* _LINUX_PRCTL_H */
kernel/sys.c +13 -0
@@ -1901,12 +1901,22 @@ out:up_read (&mm->mmap_sem);return error; } + +static int prctl_get_tid_address (struct task_struct *me,int __user **tid_addr) +{ +return put_user (me->clear_child_tid, tid_addr); +} +#else /* CONFIG_CHECKPOINT_RESTORE */ static int prctl_set_mm (int opt,unsigned long addr,unsigned long arg4,unsigned long arg5) {return -EINVAL ; } +static int prctl_get_tid_address (struct task_struct *me,int __user **tid_addr) +{ +return -EINVAL ; +}#endif SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
@@ -2061,6 +2071,9 @@ SYSCALL_DEFINE5(prctl, int, option, unsicase PR_SET_MM : error =prctl_set_mm (arg2, arg3, arg4, arg5);break ; +case PR_GET_TID_ADDRESS : + error =prctl_get_tid_address (me, (int __user **)arg2); +break ;default : error = -EINVAL ;break ;
Re: [patch 2/2] c/r: prctl: Add ability to get clear_tid_address
Quoted 7 lines · Cyrill Gorcunov
Zero is written at
clear_tid_address, when the process exits. This functionality is used bypthread_join().We already have
sys_set_tid_address()to change this address for current task but there is no way to obtain it from a user space.
Is it worth introducing a syscall for this just for symmetry? I suspect not, in which case:
-Kees
Signature · 2 lines
Kees Cook ChromeOS Security
Re: [patch 2/2] c/r: prctl: Add ability to get clear_tid_address
Quoted 5 lines · Kees Cook
Quoted 4 lines · Cyrill Gorcunov
Zero is written at
clear_tid_address, when the process exits. This functionality is used bypthread_join().
Quoted 9 lines
Quoted 3 lines
We already have
sys_set_tid_address()to change this address for current task but there is no way to obtain it from a user space.Is it worth introducing a syscall for this just for symmetry? I suspect not, in which case:
Acked-by Kees Cook <[email protected]>
Thanks Kees! syscall was considered as "no-no" in previous convesation about this patch, so we switched to prctl.
Cyrill