Does exist any _real_ reason why getrusage can't be invoked by a task to know
statistics of another task ?
The changes would be very trivial, as shown by the following patch.
Claudio
diff --git a/kernel/sys.c b/kernel/sys.c
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1746,9 +1746,13 @@ int getrusage(struct task_struct *p, int
asmlinkage long sys_getrusage(int who, struct rusage __user *ru)
{
- if (who != RUSAGE_SELF && who != RUSAGE_CHILDREN)
- return -EINVAL;
- return getrusage(current, who, ru);
+ if (who != RUSAGE_SELF && who != RUSAGE_CHILDREN) {
+ struct task_struct* tsk = find_task_by_pid(who);
+ if (tsk == NULL)
+ return -EINVAL;
+ return getrusage(tsk, RUSAGE_SELF, ru);
+ } else
+ return getrusage(current, who, ru);
}
asmlinkage long sys_umask(int mask)
The reason is what if tsk is no longer available when you call
getrusage?
> -----Original Message-----
> From: [email protected]
> [mailto:[email protected]] On Behalf Of
> Claudio Scordino
> Sent: Thursday, November 10, 2005 2:34 PM
> To: [email protected]; [email protected]
> Subject: [PATCH] getrusage sucks
>
> Does exist any _real_ reason why getrusage can't be invoked
> by a task to know
> statistics of another task ?
>
> The changes would be very trivial, as shown by the following patch.
>
> Claudio
>
>
> diff --git a/kernel/sys.c b/kernel/sys.c
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -1746,9 +1746,13 @@ int getrusage(struct task_struct *p, int
>
> asmlinkage long sys_getrusage(int who, struct rusage __user *ru)
> {
> - if (who != RUSAGE_SELF && who != RUSAGE_CHILDREN)
> - return -EINVAL;
> - return getrusage(current, who, ru);
> + if (who != RUSAGE_SELF && who != RUSAGE_CHILDREN) {
> + struct task_struct* tsk = find_task_by_pid(who);
> + if (tsk == NULL)
> + return -EINVAL;
> + return getrusage(tsk, RUSAGE_SELF, ru);
> + } else
> + return getrusage(current, who, ru);
> }
>
> asmlinkage long sys_umask(int mask)
> -
> 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/
On Friday 11 November 2005 00:47, Hua Zhong (hzhong) wrote:
> The reason is what if tsk is no longer available when you call
> getrusage?
Sorry, but honestly I don't see any problem: as you can see from my patch, if
tsk is no longer available, getrusage returns -1 and sets errno appropriately
(equal to EINVAL, which means that who is invalid).
Claudio
>
> > -----Original Message-----
> >
> > Does exist any _real_ reason why getrusage can't be invoked
> > by a task to know
> > statistics of another task ?
> >
> > The changes would be very trivial, as shown by the following patch.
> >
> > Claudio
> >
> >
> > diff --git a/kernel/sys.c b/kernel/sys.c
> > --- a/kernel/sys.c
> > +++ b/kernel/sys.c
> > @@ -1746,9 +1746,13 @@ int getrusage(struct task_struct *p, int
> >
> > asmlinkage long sys_getrusage(int who, struct rusage __user *ru)
> > {
> > - if (who != RUSAGE_SELF && who != RUSAGE_CHILDREN)
> > - return -EINVAL;
> > - return getrusage(current, who, ru);
> > + if (who != RUSAGE_SELF && who != RUSAGE_CHILDREN) {
> > + struct task_struct* tsk = find_task_by_pid(who);
> > + if (tsk == NULL)
> > + return -EINVAL;
> > + return getrusage(tsk, RUSAGE_SELF, ru);
> > + } else
> > + return getrusage(current, who, ru);
> > }
> >
> > asmlinkage long sys_umask(int mask)
Claudio Scordino wrote:
> On Friday 11 November 2005 00:47, Hua Zhong (hzhong) wrote:
>
>>The reason is what if tsk is no longer available when you call
>>getrusage?
>
>
> Sorry, but honestly I don't see any problem: as you can see from my
patch, if
> tsk is no longer available, getrusage returns -1 and sets errno
appropriately
> (equal to EINVAL, which means that who is invalid).
>
> Claudio
>
You need to wrap this with a read_lock(&tasklist_lock) to be safe, I think.
Regards,
Magnus
> You need to wrap this with a read_lock(&tasklist_lock) to be safe, I think.
Right. Probably this was the meaning also of Hua's mail. Sorry, but I didn't
get it immediately.
So, what if I do as follows ? Do you see any problem with this solution ?
Many thanks,
Claudio
diff --git a/kernel/sys.c b/kernel/sys.c
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1746,9 +1746,22 @@ int getrusage(struct task_struct *p, int
asmlinkage long sys_getrusage(int who, struct rusage __user *ru)
{
- if (who != RUSAGE_SELF && who != RUSAGE_CHILDREN)
- return -EINVAL;
- return getrusage(current, who, ru);
+ int res;
+ if (who != RUSAGE_SELF && who != RUSAGE_CHILDREN) {
+ struct task_struct* tsk;
+ read_lock(&tasklist_lock);
+ tsk = find_task_by_pid(who);
+ if (tsk == NULL) {
+ read_unlock(&tasklist_lock);
+ return -EINVAL;
+ }
+ res = getrusage(tsk, RUSAGE_SELF, ru);
+ read_unlock(&tasklist_lock);
+ return res;
+ } else {
+ res = getrusage(current, who, ru);
+ return res;
+ }
}
asmlinkage long sys_umask(int mask)
Claudio Scordino wrote:
>Does exist any _real_ reason why getrusage can't be invoked by a task to know
>statistics of another task ?
Probably only super-user should be permitted to read the usage information
about other processes. Allowing anyone to read anyone else's rusage would
open up a bunch of side channels that sound pretty dangerous. For instance,
user #1 might be able to mount a timing attack against crypto code being
executed by user #2, and that doesn't sound good.
On Gwe, 2005-11-11 at 02:11 +0100, Claudio Scordino wrote:
> > You need to wrap this with a read_lock(&tasklist_lock) to be safe, I think.
>
> Right. Probably this was the meaning also of Hua's mail. Sorry, but I didn't
> get it immediately.
>
> So, what if I do as follows ? Do you see any problem with this solution ?
It will depend on the data accuracy. If the information on cpu usage is
very accurate then it becomes a way to analyse what is happening to
tasks you don't own - such as say cryptographic functions in the web
server...
Otherwise, or with an owner check I see no real problem with the concept
On Fri, 2005-11-11 at 05:06 +0000, David Wagner wrote:
> Claudio Scordino wrote:
> >Does exist any _real_ reason why getrusage can't be invoked by a task to know
> >statistics of another task ?
>
> Probably only super-user should be permitted to read the usage information
> about other processes. Allowing anyone to read anyone else's rusage would
> open up a bunch of side channels that sound pretty dangerous. For instance,
> user #1 might be able to mount a timing attack against crypto code being
> executed by user #2, and that doesn't sound good.
Why restrict it to root? Why not just prevent users from reading other
users rusage. How could it be a security hole for joeuser's process be
able to read the rusage of joeuser's other processes?
Lee
Lee Revell wrote:
>On Fri, 2005-11-11 at 05:06 +0000, David Wagner wrote:
>> Probably only super-user should be permitted to read the usage information
>> about other processes.
>
>Why restrict it to root? Why not just prevent users from reading other
>users rusage. How could it be a security hole for joeuser's process be
>able to read the rusage of joeuser's other processes?
Sorry; you're absolutely right. I agree. I overlooked that case.
> > > You need to wrap this with a read_lock(&tasklist_lock) to be safe, I
> > > think.
> >
>
> It will depend on the data accuracy. If the information on cpu usage is
> very accurate then it becomes a way to analyse what is happening to
> tasks you don't own - such as say cryptographic functions in the web
> server...
>
> Otherwise, or with an owner check I see no real problem with the concept
So, is the following patch right ? I've added both the lock and the owner
check...
Do you think it may be an interesting feature to be inserted in the kernel ?
Many thanks,
Claudio
diff --git a/kernel/sys.c b/kernel/sys.c
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1746,9 +1746,25 @@ int getrusage(struct task_struct *p, int
asmlinkage long sys_getrusage(int who, struct rusage __user *ru)
{
- if (who != RUSAGE_SELF && who != RUSAGE_CHILDREN)
- return -EINVAL;
- return getrusage(current, who, ru);
+ if (who != RUSAGE_SELF && who != RUSAGE_CHILDREN) {
+ struct task_struct* tsk;
+ struct rusage r;
+ read_lock(&tasklist_lock);
+ tsk = find_task_by_pid(who);
+ if (tsk == NULL) {
+ read_unlock(&tasklist_lock);
+ return -EINVAL;
+ }
+ if ((current->euid != tsk->euid) &&
+ (current->euid != tsk->uid)) {
+ read_unlock(&tasklist_lock);
+ return -EINVAL;
+ }
+ k_getrusage(tsk, RUSAGE_SELF, &r);
+ read_unlock(&tasklist_lock);
+ return copy_to_user(ru, &r, sizeof(r)) ? -EFAULT : 0;
+ } else
+ return getrusage(current, who, ru);
}
asmlinkage long sys_umask(int mask)
On Gwe, 2005-11-11 at 23:38 +0100, Claudio Scordino wrote:
> + if ((current->euid != tsk->euid) &&
> + (current->euid != tsk->uid)) {
> + read_unlock(&tasklist_lock);
> + return -EINVAL;
Would be -EPERM also wants a 'privilege' check. Not sure which would be
best here - CAP_SYS_ADMIN seems to be the 'default' used
* Alan Cox ([email protected]) wrote:
> On Gwe, 2005-11-11 at 23:38 +0100, Claudio Scordino wrote:
> > + if ((current->euid != tsk->euid) &&
> > + (current->euid != tsk->uid)) {
> > + read_unlock(&tasklist_lock);
> > + return -EINVAL;
>
>
> Would be -EPERM also wants a 'privilege' check. Not sure which would be
> best here - CAP_SYS_ADMIN seems to be the 'default' used
It's already available via /proc w/out protection. And ditto via posix
cpu timers.
On Saturday 12 November 2005 00:23, Alan Cox wrote:
> On Gwe, 2005-11-11 at 23:38 +0100, Claudio Scordino wrote:
> > + if ((current->euid != tsk->euid) &&
> > + (current->euid != tsk->uid)) {
> > + read_unlock(&tasklist_lock);
> > + return -EINVAL;
>
> Would be -EPERM also wants a 'privilege' check. Not sure which would be
> best here - CAP_SYS_ADMIN seems to be the 'default' used
I would say -EPERM. Any other comment about the patch ?
On Gwe, 2005-11-11 at 15:02 -0800, Chris Wright wrote:
> * Alan Cox ([email protected]) wrote:
> > On Gwe, 2005-11-11 at 23:38 +0100, Claudio Scordino wrote:
> > > + if ((current->euid != tsk->euid) &&
> > > + (current->euid != tsk->uid)) {
> > > + read_unlock(&tasklist_lock);
> > > + return -EINVAL;
> >
> >
> > Would be -EPERM also wants a 'privilege' check. Not sure which would be
> > best here - CAP_SYS_ADMIN seems to be the 'default' used
>
> It's already available via /proc w/out protection. And ditto via posix
> cpu timers.
In which case the only comment I have is the one about accuracy - and
that is true for procfs too so will only come up if someone gets the
urge to use perfctr timers for precision resource management
Claudio Scordino wrote:
>So, is the following patch right ? I've added both the lock and the owner
>check...
I think this patch is too permissive. It lets me run a setuid-root
program and then call getrusage() on it. That's not a good idea.
(I might, say, run /bin/su and then mount a timing or cache side-channel
attacks on its password hashing code. That particular example might or
might not work, but I hope it illustrates my concern.)
Should you be using the same permission check that ptrace() does?
ptrace() is more restrictive than what you seemed to have in mind.
However, if ptrace() lets you attach to a process, then it's probably
pretty safe to let you call getrusage(), as you could have just used
ptrace() to read the process's entire memory image.
kernel/ptrace.c:ptrace_may_attach() might be relevant.
TOCTTOU vulnerabilities could be a problem. If I understand correctly,
your locking code should take care of this, but you might want to
double-check, as I'm not very familiar with the kernel's locking scheme.
>
> In which case the only comment I have is the one about accuracy - and
> that is true for procfs too so will only come up if someone gets the
> urge to use perfctr timers for precision resource management
According to your comments, this the final patch.
Should it be committed ?
Claudio
diff --git a/kernel/sys.c b/kernel/sys.c
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1746,9 +1746,26 @@ int getrusage(struct task_struct *p, int
asmlinkage long sys_getrusage(int who, struct rusage __user *ru)
{
- if (who != RUSAGE_SELF && who != RUSAGE_CHILDREN)
- return -EINVAL;
- return getrusage(current, who, ru);
+ if (who != RUSAGE_SELF && who != RUSAGE_CHILDREN) {
+ struct task_struct* tsk;
+ struct rusage r;
+ read_lock(&tasklist_lock);
+ tsk = find_task_by_pid(who);
+ if (tsk == NULL) {
+ read_unlock(&tasklist_lock);
+ return -EINVAL;
+ }
+ if ((current->euid != tsk->euid) &&
+ (current->euid != tsk->uid) &&
+ (!capable(CAP_SYS_ADMIN))){
+ read_unlock(&tasklist_lock);
+ return -EPERM;
+ }
+ k_getrusage(tsk, RUSAGE_SELF, &r);
+ read_unlock(&tasklist_lock);
+ return copy_to_user(ru, &r, sizeof(r)) ? -EFAULT : 0;
+ } else
+ return getrusage(current, who, ru);
}
asmlinkage long sys_umask(int mask)
Chris Wright wrote:
>It's already available via /proc w/out protection. And ditto via posix
>cpu timers.
If so, maybe that code should be fixed. Where exactly in /proc would
I find the getrusage() info of another process? Is there any argument
that disclosing it to everyone is safe? Or is it just that no one has
ever given the security considerations much thought up till now?
On Sat, 12 Nov 2005, Claudio Scordino wrote:
> >
> > In which case the only comment I have is the one about accuracy - and
> > that is true for procfs too so will only come up if someone gets the
> > urge to use perfctr timers for precision resource management
>
> According to your comments, this the final patch.
this only lets you get RUSAGE_SELF data for the target... for many
processes it's more important to get the RUSAGE_CHILDREN data... and
really i'm having a hard time imagining a use for this code which on
further inspection doesn't eventually blow up to the requirements of a
proper accounting subsystem... (of which i understand there are two or
three competining implementations in progress?)
do you have a use case for this new code?
-dean
I think it's better to use "goto" for error exit. Too many read_unlock
here..
> -----Original Message-----
> From: Claudio Scordino [mailto:[email protected]]
> Sent: Friday, November 11, 2005 3:44 PM
> To: Alan Cox
> Cc: Chris Wright; Magnus Naeslund(f); Hua Zhong (hzhong);
> [email protected]; [email protected]; David Wagner
> Subject: Re: [PATCH] getrusage sucks
>
> >
> > In which case the only comment I have is the one about
> accuracy - and
> > that is true for procfs too so will only come up if someone gets the
> > urge to use perfctr timers for precision resource management
>
> According to your comments, this the final patch.
>
> Should it be committed ?
>
> Claudio
>
>
>
> diff --git a/kernel/sys.c b/kernel/sys.c
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -1746,9 +1746,26 @@ int getrusage(struct task_struct *p, int
>
> asmlinkage long sys_getrusage(int who, struct rusage __user *ru)
> {
> - if (who != RUSAGE_SELF && who != RUSAGE_CHILDREN)
> - return -EINVAL;
> - return getrusage(current, who, ru);
> + if (who != RUSAGE_SELF && who != RUSAGE_CHILDREN) {
> + struct task_struct* tsk;
> + struct rusage r;
> + read_lock(&tasklist_lock);
> + tsk = find_task_by_pid(who);
> + if (tsk == NULL) {
> + read_unlock(&tasklist_lock);
> + return -EINVAL;
> + }
> + if ((current->euid != tsk->euid) &&
> + (current->euid != tsk->uid) &&
> + (!capable(CAP_SYS_ADMIN))){
> + read_unlock(&tasklist_lock);
> + return -EPERM;
> + }
> + k_getrusage(tsk, RUSAGE_SELF, &r);
> + read_unlock(&tasklist_lock);
> + return copy_to_user(ru, &r, sizeof(r)) ? -EFAULT : 0;
> + } else
> + return getrusage(current, who, ru);
> }
>
> asmlinkage long sys_umask(int mask)
>
* David Wagner ([email protected]) wrote:
> Chris Wright wrote:
> >It's already available via /proc w/out protection. And ditto via posix
> >cpu timers.
>
> If so, maybe that code should be fixed. Where exactly in /proc would
> I find the getrusage() info of another process?
Just from /proc/[pid]/stat. (fs/proc/array.c::do_task_stat).
> Is there any argument
> that disclosing it to everyone is safe? Or is it just that no one has
> ever given the security considerations much thought up till now?
I guess it keeps falling in the "too theoretical" category. It can be
protected by policy, but default is open.
thanks,
-chris
* dean gaudet ([email protected]) wrote:
> do you have a use case for this new code?
I'm with Dean. What problem are you trying to solve?
thanks,
-chris
Chris Wright wrote:
>/proc/[pid]/stat. (fs/proc/array.c::do_task_stat).
>
>> Is there any argument
>> that disclosing it to everyone is safe? Or is it just that no one has
>> ever given the security considerations much thought up till now?
>
>I guess it keeps falling in the "too theoretical" category. It can be
>protected by policy, but default is open.
Ahh, I see. I had never looked at /proc/[pid]/stat carefully before.
Well, making /proc/[pid]/stat world-readable by default looks pretty
dubious to me. There's all sorts of stuff there that I suspect should
not be revealed: EIP, stack pointer, stats on paging and swapping, and
so on. I suspect that this is not at all safe. Most crypto algorithms
tend to fall apart when you have side channels like this.
Maybe no one cares, because no one uses Linux in a multi-user setting
where users are motivated to attack each other or attack the system.
But baking this kind of "privilege escalation" vulnerability into the
kernel by default doesn't seem like a good idea to me.
--- Chris Wright <[email protected]> wrote:
> * dean gaudet ([email protected])
> wrote:
> > do you have a use case for this new code?
>
> I'm with Dean. What problem are you trying to
> solve?
>
> thanks,
> -chris
>
> --
> Kernelnewbies: Help each other learn about the Linux
> kernel.
> Archive:
> http://mail.nl.linux.org/kernelnewbies/
> FAQ: http://kernelnewbies.org/faq/
>
>
__________________________________
Yahoo! Mail - PC Magazine Editors' Choice 2005
http://mail.yahoo.com
Hi
I want to makefile for my kernel module..
1.c 2.c 3.c files are places in /home/anil folder but
these files contain .h (hearder)files from 3 different
directory 1) /home/include 2) /root/incluent 3)
/opt/include
can u suggest me a makefile to generate a common
module target.ko using these .C and .h files.
Thanks in advance
Regards,
anil
__________________________________
Yahoo! FareChase: Search multiple travel sites in one click.
http://farechase.yahoo.com
Hi,
I am currently doing a kernel module involves detecting/changing
disk mode among STANDBY and ACTIVE/IDLE. I used ide_cmd_wait() to issue
commands like WIN_IDLEIMMEDIATELY and WIN_STANDBYNOW1. The problem is, a
drive in standby mode will automatically awake whenever a disk operation
is requested and I need to know the mode change as soon as possible. (So I
donot want to periodically use the WIN_CHECKPOWERMODE to detect it). I was
wondering if the standby disk is waken by the kernel or by the disk
firmware? If it is not in the kernel, is there any good way to detect it?
Thanks,
Hui
On Sat, Nov 12, 2005 at 07:16:52AM -0800, anil dahiya wrote:
> Hi
> I want to makefile for my kernel module..
> 1.c 2.c 3.c files are places in /home/anil folder but
> these files contain .h (hearder)files from 3 different
> directory 1) /home/include 2) /root/incluent 3)
> /opt/include
>
> can u suggest me a makefile to generate a common
> module target.ko using these .C and .h files.
Makefile
obj-m := foo.o
foo-y := 1.o 2.o 3.o
EXTRA_CFLGAS := -I/home/include -I/root/incluent -I/opt/include
And then build the module with:
make -C $KERNEL_SRC M=`pwd`
See Documentation/kbuild/makefiles.txt for reference.
Note: Kernel being pointed out must be a fully build kernel
Sam
On Saturday 12 November 2005 02:10, Chris Wright wrote:
> * dean gaudet ([email protected]) wrote:
> > do you have a use case for this new code?
>
> I'm with Dean. What problem are you trying to solve?
I just want to improve getrusage to account for the case in which a server
process needs to have usage information about client processes at run-time.
In this case RUSAGE_SELF is more important than RUSAGE_CHILDREN.
I think it would be an useful feature for some user-level applications.
At the beginning, I didn't want to propose any different prototype for the
function, even if I think that a more general (and correct) one would be
int getrusage(int who, struct rusage *usage, pid_t pid);
which accounts for all the situations we discussed so far.
However, I've added a more restrictive check (as asked by David) and the goto
proposed by Hua. Is now the patch right ? Do you think that it's useful ?
Many thanks,
Claudio
diff --git a/kernel/sys.c b/kernel/sys.c
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1746,9 +1746,29 @@ int getrusage(struct task_struct *p, int
asmlinkage long sys_getrusage(int who, struct rusage __user *ru)
{
- if (who != RUSAGE_SELF && who != RUSAGE_CHILDREN)
- return -EINVAL;
- return getrusage(current, who, ru);
+ struct rusage r;
+ struct task_struct* tsk = current;
+ read_lock(&tasklist_lock);
+ if ((who != RUSAGE_SELF) && (who != RUSAGE_CHILDREN)) {
+ tsk = find_task_by_pid(who);
+ if ((tsk == NULL) || (who <=0))
+ goto bad;
+ if (((current->uid != tsk->euid) ||
+ (current->uid != tsk->suid) ||
+ (current->uid != tsk->uid) ||
+ (current->gid != tsk->egid) ||
+ (current->gid != tsk->sgid) ||
+ (current->gid != tsk->gid)) && !capable(CAP_SYS_PTRACE))
+ goto bad;
+ who = RUSAGE_SELF;
+ }
+ k_getrusage(tsk, who, &r);
+ read_unlock(&tasklist_lock);
+ return copy_to_user(ru, &r, sizeof(r)) ? -EFAULT : 0;
+
+ bad:
+ read_unlock(&tasklist_lock);
+ return tsk ? -EPERM : -EINVAL;
}
asmlinkage long sys_umask(int mask)
>>>>> "Claudio" == Claudio Scordino <[email protected]> writes:
>> You need to wrap this with a read_lock(&tasklist_lock) to be safe,
>> I think.
Claudio> Right. Probably this was the meaning also of Hua's
Claudio> mail. Sorry, but I didn't get it immediately.
Claudio> So, what if I do as follows ? Do you see any problem with
Claudio> this solution ?
You should probably restrict the ability to read a process's usage to
a suitably privileged user -- i.e., effective uid same as the task's,
or capable(CAP_SYS_RESOURCE) or maybe capable(CAP_SYS_ADMIN)
--
Dr Peter Chubb http://www.gelato.unsw.edu.au peterc AT gelato.unsw.edu.au
The technical we do immediately, the political takes *forever*
On Tuesday 15 November 2005 02:08, Peter Chubb wrote:
> >> You need to wrap this with a read_lock(&tasklist_lock) to be safe,
> >> I think.
>
> Claudio> Right. Probably this was the meaning also of Hua's
> Claudio> mail. Sorry, but I didn't get it immediately.
>
> Claudio> So, what if I do as follows ? Do you see any problem with
> Claudio> this solution ?
>
> You should probably restrict the ability to read a process's usage to
> a suitably privileged user -- i.e., effective uid same as the task's,
> or capable(CAP_SYS_RESOURCE) or maybe capable(CAP_SYS_ADMIN)
So, is CAP_SYS_PTRACE (as done in the patch below) not enough ?
Honestly, I don't see any problem in allowing any user to know usage
information about _his_ processes...
Many thanks,
Claudio
Signed-off-by: Claudio Scordino <[email protected]>
diff --git a/kernel/sys.c b/kernel/sys.c
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1746,9 +1746,29 @@ int getrusage(struct task_struct *p, int
asmlinkage long sys_getrusage(int who, struct rusage __user *ru)
{
- if (who != RUSAGE_SELF && who != RUSAGE_CHILDREN)
- return -EINVAL;
- return getrusage(current, who, ru);
+ struct rusage r;
+ struct task_struct* tsk = current;
+ read_lock(&tasklist_lock);
+ if ((who != RUSAGE_SELF) && (who != RUSAGE_CHILDREN)) {
+ tsk = find_task_by_pid(who);
+ if ((tsk == NULL) || (who <=0))
+ goto bad;
+ if (((current->uid != tsk->euid) ||
+ (current->uid != tsk->suid) ||
+ (current->uid != tsk->uid) ||
+ (current->gid != tsk->egid) ||
+ (current->gid != tsk->sgid) ||
+ (current->gid != tsk->gid)) && !capable(CAP_SYS_PTRACE))
+ goto bad;
+ who = RUSAGE_SELF;
+ }
+ k_getrusage(tsk, who, &r);
+ read_unlock(&tasklist_lock);
+ return copy_to_user(ru, &r, sizeof(r)) ? -EFAULT : 0;
+
+ bad:
+ read_unlock(&tasklist_lock);
+ return tsk ? -EPERM : -EINVAL;
}
asmlinkage long sys_umask(int mask)
Actually, I think that a better implementation of getrusage would be the
following one, but it requires a (new) third parameter. That's why I didn't
suggest it...
Many thanks,
Claudio
Signed-off-by: Claudio Scordino <[email protected]>
asmlinkage long sys_getrusage(int who, struct rusage __user *ru, pid_t pid)
{
struct rusage r;
struct task_struct* tsk = current;
if ((pid < 0) ||
((who != RUSAGE_SELF) && (who != RUSAGE_CHILDREN)))
return -EINVAL;
read_lock(&tasklist_lock);
if (pid > 0) {
tsk = find_task_by_pid(pid);
if (tsk == NULL)
goto bad;
if (((current->uid != tsk->euid) ||
(current->uid != tsk->suid) ||
(current->uid != tsk->uid) ||
(current->gid != tsk->egid) ||
(current->gid != tsk->sgid) ||
(current->gid != tsk->gid)) && !capable(CAP_SYS_PTRACE))
goto bad;
}
k_getrusage(tsk, who, &r);
read_unlock(&tasklist_lock);
return copy_to_user(ru, &r, sizeof(r)) ? -EFAULT : 0;
bad:
read_unlock(&tasklist_lock);
return tsk ? -EPERM : -EINVAL;
}
I might mention that ESRCH is the usual "that process does not exist"
error return, not EINVAL. (See kill(2), ptrace(2), setpgrp(2), etc.)
> I am currently doing a kernel module involves detecting/changing
> disk mode among STANDBY and ACTIVE/IDLE. I used ide_cmd_wait() to issue
> commands like WIN_IDLEIMMEDIATELY and WIN_STANDBYNOW1. The problem is, a
> drive in standby mode will automatically awake whenever a disk operation
> is requested and I need to know the mode change as soon as possible. (So I
AFAIK there's no nice way to get that info, but it is useful, so patch would
be welcome.
--
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms
On Sun, 13 Nov 2005 15:10:56 +0000, Pavel Machek wrote
> > I am currently doing a kernel module involves detecting/changing
> > disk mode among STANDBY and ACTIVE/IDLE. I used ide_cmd_wait() to issue
> > commands like WIN_IDLEIMMEDIATELY and WIN_STANDBYNOW1. The problem is, a
> > drive in standby mode will automatically awake whenever a disk operation
> > is requested and I need to know the mode change as soon as possible. (So I
>
> AFAIK there's no nice way to get that info, but it is useful, so
> patch would be welcome.
I would check the hdparm man page again. Still, it sounds interesting.
Additionally, it could be cool if someone could finish up or make the option
for the HD freeze to use it with the HDAPS driver. ;-)
.Alejandro
On Wed, 16 Nov 2005, Alejandro Bonilla wrote:
> On Sun, 13 Nov 2005 15:10:56 +0000, Pavel Machek wrote
> > > I am currently doing a kernel module involves detecting/changing
> > > disk mode among STANDBY and ACTIVE/IDLE. I used ide_cmd_wait() to issue
> > > commands like WIN_IDLEIMMEDIATELY and WIN_STANDBYNOW1. The problem is, a
> > > drive in standby mode will automatically awake whenever a disk operation
> > > is requested and I need to know the mode change as soon as possible. (So I
> >
> > AFAIK there's no nice way to get that info, but it is useful, so
> > patch would be welcome.
>
> I would check the hdparm man page again. Still, it sounds interesting.
>
> Additionally, it could be cool if someone could finish up or make the option
> for the HD freeze to use it with the HDAPS driver. ;-)
>
> .Alejandro
>
Thanks for reply :) What I did to handle this problem is a little stupid
: Suppose the disk is now in a standby mode. In case that there is a
request sent to the disk drive, a kernel thread is awake to detect/update
the current disk power mode. The disk power mode is stored in the
ide_drive_t structure and be protected by lock. It seems to work fine in
my simple tests. But again, there should be better solutions.
Hui
Hi!
> > > > I am currently doing a kernel module involves detecting/changing
> > > > disk mode among STANDBY and ACTIVE/IDLE. I used ide_cmd_wait() to issue
> > > > commands like WIN_IDLEIMMEDIATELY and WIN_STANDBYNOW1. The problem is, a
> > > > drive in standby mode will automatically awake whenever a disk operation
> > > > is requested and I need to know the mode change as soon as possible. (So I
> > >
> > > AFAIK there's no nice way to get that info, but it is useful, so
> > > patch would be welcome.
> >
> > I would check the hdparm man page again. Still, it sounds interesting.
> >
> > Additionally, it could be cool if someone could finish up or make the option
> > for the HD freeze to use it with the HDAPS driver. ;-)
> >
> > .Alejandro
> >
>
> Thanks for reply :) What I did to handle this problem is a little stupid
> : Suppose the disk is now in a standby mode. In case that there is a
> request sent to the disk drive, a kernel thread is awake to detect/update
> the current disk power mode. The disk power mode is stored in the
> ide_drive_t structure and be protected by lock. It seems to work fine in
> my simple tests. But again, there should be better solutions.
Can we get the patch?
--
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms
On Sat, 19 Nov 2005, Pavel Machek wrote:
Hi,
> Hi!
>
> > > > > I am currently doing a kernel module involves detecting/changing
> > > > > disk mode among STANDBY and ACTIVE/IDLE. I used ide_cmd_wait() to issue
> > > > > commands like WIN_IDLEIMMEDIATELY and WIN_STANDBYNOW1. The problem is, a
> > > > > drive in standby mode will automatically awake whenever a disk operation
> > > > > is requested and I need to know the mode change as soon as possible. (So I
> > > >
> > > > AFAIK there's no nice way to get that info, but it is useful, so
> > > > patch would be welcome.
> > >
> > > I would check the hdparm man page again. Still, it sounds interesting.
> > >
> > > Additionally, it could be cool if someone could finish up or make the option
> > > for the HD freeze to use it with the HDAPS driver. ;-)
> > >
> > > .Alejandro
> > >
> >
> > Thanks for reply :) What I did to handle this problem is a little stupid
> > : Suppose the disk is now in a standby mode. In case that there is a
> > request sent to the disk drive, a kernel thread is awake to detect/update
> > the current disk power mode. The disk power mode is stored in the
> > ide_drive_t structure and be protected by lock. It seems to work fine in
> > my simple tests. But again, there should be better solutions.
>
> Can we get the patch?
> --
Sure. I will make the patch and test it again. But it may take a while
because I have an urgent task now...Thanks,
Hui