* [patch] Modify flag's value in tapset's probe delete_module
[not found] <1188780895.5736.ezmlm@sourceware.org>
@ 2007-09-03 12:48 ` Zhaolei
2007-09-05 14:46 ` [patch] For memory access error when calling _stp_sockaddr_str with addrlen set to 0 Zhaolei
0 siblings, 1 reply; 8+ messages in thread
From: Zhaolei @ 2007-09-03 12:48 UTC (permalink / raw)
To: systemtap; +Cc: Zhaolei
Hi, everyone
In the probe delete_module(syscalls.stp), function _module_flags_str
(aux_syscalls.stp) is used for converting the flags to string. But the
returned string is different from the man page of delete_module:
delete_module's flag values in "man delete_module"(RHEL5,IA64) is O_TRUNC
and O_NONBLOCK, but in probe delete_module, flag's value is WAIT and FORCE
(From other non-intel-arch?).
Is there anyone knows why tapset uses WAIT/FORCE values for delete_module's
flag, please tell me(or I will modify it unify with man).
And there may be another problem exists in delete_module's probe:
O_TRUNC and O_NONBLOCK's value is different between architectures, so I
think it is better using defined value for compare with flag's value.
(It we use static value as 2048/512, it will get different result across
architectures):
I write a patch for this problem:
and if no objection, I will commit it.
Signed-off-by: "Zhaolei" zhaolei@cn.fujitsu.com
function _module_flags_str:string(flags:long)
--- aux_syscalls.stp.old 2007-08-24 15:35:05.000000000 +0900
+++ aux_syscalls.stp 2007-08-24 15:34:44.000000000 +0900
@@ -1131,11 +1131,19 @@ function _mlockall_flags_str:string(flag
%}
/* used by sys_delete_module */
-function _module_flags_str(f) {
- if(!(f & 2048)) bs="WAIT|"
- if(f & 512) bs=bs."FORCE|"
- return substr(bs,0,strlen(bs)-1)
-}
+function _module_flags_str:string(flags:long)
+%{ /* pure */
+ int len;
+ long flags = THIS->flags;
+ char *str = THIS->__retvalue;
+ if (flags & O_TRUNC)
+ strlcat(str,"O_TRUNC|", MAXSTRINGLEN);
+ if (flags & O_NONBLOCK)
+ strlcat(str,"O_NONBLOCK|", MAXSTRINGLEN);
+ len = strlen(str);
+ if (len)
+ str[strlen(str)-1] = 0;
+%}
function _sched_policy_str(policy) {
if(policy==0) return "SCHED_OTHER"
Regards
Zhaolei
^ permalink raw reply [flat|nested] 8+ messages in thread
* [patch] For memory access error when calling _stp_sockaddr_str with addrlen set to 0
2007-09-03 12:48 ` [patch] Modify flag's value in tapset's probe delete_module Zhaolei
@ 2007-09-05 14:46 ` Zhaolei
2007-09-05 22:49 ` Frank Ch. Eigler
2007-09-10 5:03 ` [patch] For getrusage's argstr in tapset Zhaolei
0 siblings, 2 replies; 8+ messages in thread
From: Zhaolei @ 2007-09-05 14:46 UTC (permalink / raw)
To: systemtap; +Cc: Zhaolei
Hi, everyone
I found a memory access error in aux_syscall.stp's _stp_sockaddr_str
function.
When probing a function which uses _stp_sockaddr_str with addrlen set to 0,
Ex: bind(sockfd, my_addr, 0);
the calling stack is:
probe bind [bind(sockfd, my_addr, 0)]
--> _struct_sockaddr_u [copy 0 byte from my_addr in userspace to buf]
--> _stp_sockaddr_str [access buf's content in switch (sa->sa_family)]
In this case, sa->sa_family will not be initialized.
Also, I think the buf's length should be considered when using it.
If no objection, I will commit the following patch:
Signed-off-by: "Zhaolei" zhaolei@cn.fujitsu.com
--- aux_syscalls.stp.old 2007-08-30 14:56:29.000000000 +0900
+++ aux_syscalls.stp 2007-08-30 15:38:40.000000000 +0900
@@ -309,36 +309,31 @@ function _struct_itimerval:string(addr:l
void _stp_sockaddr_str(char *str, const int strlen, char *buf, int len)
{
struct sockaddr *sa = (struct sockaddr *)buf;
- switch (sa->sa_family) {
- case AF_INET:
+ if ((sa->sa_family == AF_INET)&&(len == 16))
{
struct sockaddr_in *sin = (struct sockaddr_in *)buf;
const unsigned char *addr = (unsigned char *)&sin->sin_addr;
snprintf(str, strlen, "{AF_INET, %d.%d.%d.%d, %d}",
addr[0], addr[1], addr[2], addr[3], ntohs(sin->sin_port));
- break;
}
- case AF_UNIX:
+ else if ((sa->sa_family == AF_UNIX)&&(len == 110))
{
struct sockaddr_un *sun = (struct sockaddr_un *)buf;
snprintf(str, strlen, "{AF_UNIX, %s}", sun->sun_path);
- break;
}
- case AF_NETLINK:
+ else if ((sa->sa_family == AF_NETLINK)&&(len == 12))
{
struct sockaddr_nl *nl = (struct sockaddr_nl *)buf;
snprintf(str, strlen, "{AF_NETLINK, pid=%d, groups=%08x}", nl->nl_pid, nl->nl_groups);
- break;
}
- case AF_INET6:
+ else if ((sa->sa_family == AF_INET6)&&(len == 28))
{
// FIXME. Address is probably not correctly displayed
struct sockaddr_in6 *sin = (struct sockaddr_in6 *)buf;
snprintf(str, strlen, "{AF_INET6, %016llx, %d}",
*(long long *)&sin->sin6_addr, ntohs(sin->sin6_port));
- break;
}
- case AF_PACKET:
+ else if ((sa->sa_family == AF_PACKET)&&(len == 18))
{
/* FIXME. This needs tested */
struct sockaddr_ll *sll = (struct sockaddr_ll *)buf;
@@ -351,11 +346,17 @@ void _stp_sockaddr_str(char *str, const
(int)sll->sll_protocol, sll->sll_ifindex, (int)sll->sll_hatype, (int)sll->sll_pkttype,
(int)sll->sll_halen, *(uint64_t *)sll->sll_addr);
#endif
- break;
}
-
- default:
- snprintf(str, strlen, "{unknown address family %d}", sa->sa_family);
+ else
+ {
+ if (len >= 2)
+ {
+ snprintf(str, strlen, "{unknown sockaddr with sa=%d, salen=%d}", sa->sa_family, len);
+ }
+ else
+ {
+ snprintf(str, strlen, "{unknown sockaddr with salen=%d}", len);
+ }
}
}
%}
Regards
Zhaolei
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [patch] For memory access error when calling _stp_sockaddr_str with addrlen set to 0
2007-09-05 14:46 ` [patch] For memory access error when calling _stp_sockaddr_str with addrlen set to 0 Zhaolei
@ 2007-09-05 22:49 ` Frank Ch. Eigler
2007-09-10 5:03 ` [patch] For getrusage's argstr in tapset Zhaolei
1 sibling, 0 replies; 8+ messages in thread
From: Frank Ch. Eigler @ 2007-09-05 22:49 UTC (permalink / raw)
To: Zhaolei; +Cc: systemtap
"Zhaolei" <zhaolei@cn.fujitsu.com> writes:
> I found a memory access error in aux_syscall.stp's _stp_sockaddr_str
> function.
Thanks.
> [...] Also, I think the buf's length should be considered when
> using it. If no objection, I will commit the following patch:
Looks good, except that I suggest using sizeof()-based expressions
instead of literal numbers for the length comparisons, if possible.
- FChE
^ permalink raw reply [flat|nested] 8+ messages in thread
* [patch] For getrusage's argstr in tapset
2007-09-05 14:46 ` [patch] For memory access error when calling _stp_sockaddr_str with addrlen set to 0 Zhaolei
2007-09-05 22:49 ` Frank Ch. Eigler
@ 2007-09-10 5:03 ` Zhaolei
2007-09-10 5:21 ` Wenji Huang
1 sibling, 1 reply; 8+ messages in thread
From: Zhaolei @ 2007-09-10 5:03 UTC (permalink / raw)
To: systemtap; +Cc: Zhaolei
Hi, everyone
In [tapset->probe getrusage], argstr's "who" argument is printed as
"RUSAGE_BOTH" when its value is -2.
But "RUSAGE_BOTH" is not used in getrusage syscall due to the following
kernel source:
asmlinkage long sys_getrusage(int who, struct rusage __user *ru)
{
if (who != RUSAGE_SELF && who != RUSAGE_CHILDREN)
return -EINVAL;
return getrusage(current, who, ru);
}
So i want to remove RUSAGE_BOTH from _rusage_who_str.
Is there anyone knows why RUSAGE_BOTH is necessary, please tell me.
and if no objection, I will commit it.
Signed-off-by: "Zhaolei" zhaolei@cn.fujitsu.com
--- aux_syscalls.stp.old 2007-09-06 11:51:14.000000000 +0900
+++ aux_syscalls.stp 2007-09-06 11:52:22.000000000 +0900
@@ -1661,7 +1661,6 @@ function _rlimit_resource_str(r) {
function _rusage_who_str(w) {
if(w==0) return "RUSAGE_SELF"
if(w==-1) return "RUSAGE_CHILDREN"
- if(w==-2) return "RUSAGE_BOTH"
return sprintf("UNKNOWN VALUE: %d", w)
}
Regards
Zhaolei
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [patch] For getrusage's argstr in tapset
2007-09-10 5:03 ` [patch] For getrusage's argstr in tapset Zhaolei
@ 2007-09-10 5:21 ` Wenji Huang
2007-09-10 9:28 ` Zhaolei
0 siblings, 1 reply; 8+ messages in thread
From: Wenji Huang @ 2007-09-10 5:21 UTC (permalink / raw)
To: Zhaolei; +Cc: systemtap
Hi Zhaolei,
probe getrusage set probe point at the entry of the function. So the
arguments may be any value.
So I think to keep the entry in _rusage_who_str, just give another
mapping in case of some certain value.
Of course, if you can ensure that "who" only be
RUSAGE_SELF/RUSAGE_CHILDREN when the syscall called. Just remove.
Thanks,
Wenji
Zhaolei wrote:
>Hi, everyone
>
>In [tapset->probe getrusage], argstr's "who" argument is printed as
> "RUSAGE_BOTH" when its value is -2.
>But "RUSAGE_BOTH" is not used in getrusage syscall due to the following
> kernel source:
>asmlinkage long sys_getrusage(int who, struct rusage __user *ru)
>{
> if (who != RUSAGE_SELF && who != RUSAGE_CHILDREN)
> return -EINVAL;
> return getrusage(current, who, ru);
>}
>
>So i want to remove RUSAGE_BOTH from _rusage_who_str.
>Is there anyone knows why RUSAGE_BOTH is necessary, please tell me.
>and if no objection, I will commit it.
>
>Signed-off-by: "Zhaolei" zhaolei@cn.fujitsu.com
>
>--- aux_syscalls.stp.old 2007-09-06 11:51:14.000000000 +0900
>+++ aux_syscalls.stp 2007-09-06 11:52:22.000000000 +0900
>@@ -1661,7 +1661,6 @@ function _rlimit_resource_str(r) {
> function _rusage_who_str(w) {
> if(w==0) return "RUSAGE_SELF"
> if(w==-1) return "RUSAGE_CHILDREN"
>- if(w==-2) return "RUSAGE_BOTH"
> return sprintf("UNKNOWN VALUE: %d", w)
> }
>
>Regards
>Zhaolei
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [patch] For getrusage's argstr in tapset
2007-09-10 5:21 ` Wenji Huang
@ 2007-09-10 9:28 ` Zhaolei
2007-09-10 14:01 ` Wenji Huang
0 siblings, 1 reply; 8+ messages in thread
From: Zhaolei @ 2007-09-10 9:28 UTC (permalink / raw)
To: Wenji Huang; +Cc: systemtap
Hello, Wenji.
Thank you for your suggestion.
I investigated RUSAGE_BOTH and got the following result:
1: _rusage_who_str is only used for sys_getrusage(and compat_sys_getrusage)
in current tapset, and there is no other syscall which needs to call
_rusage_who_str.
2: RUSAGE_BOTH is only used in kernel's internal function getrusage, but
there is no syscall using this function with RUSAGE_BOTH.
So I think RUSAGE_BOTH is not necessary in tracing syscall, but it may be
needed when tracing kernel function as getrusage.
It is good idea disable RUSAGE_BOTH only in sys_getrusage
(and compat_sys_getrusage).
Regards
Zhaolei
----- Original Message -----
From: "Wenji Huang" <wenji.huang@oracle.com>
To: "Zhaolei" <zhaolei@cn.fujitsu.com>
Cc: <systemtap@sourceware.org>
Sent: Monday, September 10, 2007 11:44 AM
Subject: Re: [patch] For getrusage's argstr in tapset
> Hi Zhaolei,
>
> probe getrusage set probe point at the entry of the function. So the
> arguments may be any value.
>
> So I think to keep the entry in _rusage_who_str, just give another
> mapping in case of some certain value.
>
> Of course, if you can ensure that "who" only be
> RUSAGE_SELF/RUSAGE_CHILDREN when the syscall called. Just remove.
>
> Thanks,
> Wenji
>
>
> Zhaolei wrote:
>
> >Hi, everyone
> >
> >In [tapset->probe getrusage], argstr's "who" argument is printed as
> > "RUSAGE_BOTH" when its value is -2.
> >But "RUSAGE_BOTH" is not used in getrusage syscall due to the following
> > kernel source:
> >asmlinkage long sys_getrusage(int who, struct rusage __user *ru)
> >{
> > if (who != RUSAGE_SELF && who != RUSAGE_CHILDREN)
> > return -EINVAL;
> > return getrusage(current, who, ru);
> >}
> >
> >So i want to remove RUSAGE_BOTH from _rusage_who_str.
> >Is there anyone knows why RUSAGE_BOTH is necessary, please tell me.
> >and if no objection, I will commit it.
> >
> >Signed-off-by: "Zhaolei" zhaolei@cn.fujitsu.com
> >
> >--- aux_syscalls.stp.old 2007-09-06 11:51:14.000000000 +0900
> >+++ aux_syscalls.stp 2007-09-06 11:52:22.000000000 +0900
> >@@ -1661,7 +1661,6 @@ function _rlimit_resource_str(r) {
> > function _rusage_who_str(w) {
> > if(w==0) return "RUSAGE_SELF"
> > if(w==-1) return "RUSAGE_CHILDREN"
> >- if(w==-2) return "RUSAGE_BOTH"
> > return sprintf("UNKNOWN VALUE: %d", w)
> > }
> >
> >Regards
> >Zhaolei
> >
> >
> >
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [patch] For getrusage's argstr in tapset
2007-09-10 9:28 ` Zhaolei
@ 2007-09-10 14:01 ` Wenji Huang
2007-09-13 16:06 ` Zhaolei
0 siblings, 1 reply; 8+ messages in thread
From: Wenji Huang @ 2007-09-10 14:01 UTC (permalink / raw)
To: Zhaolei; +Cc: systemtap
Hi zhaolei,
In normal case, thing may be like that. But in abnormal case, maybe
wrong arguments are passed to this function. So new entry will
give a clearer description, instead of general UNKNOWN.
Thanks,
Wenji
Zhaolei wrote:
>Hello, Wenji.
>
>Thank you for your suggestion.
>
>I investigated RUSAGE_BOTH and got the following result:
>1: _rusage_who_str is only used for sys_getrusage(and compat_sys_getrusage)
> in current tapset, and there is no other syscall which needs to call
> _rusage_who_str.
>
>2: RUSAGE_BOTH is only used in kernel's internal function getrusage, but
> there is no syscall using this function with RUSAGE_BOTH.
>
>So I think RUSAGE_BOTH is not necessary in tracing syscall, but it may be
> needed when tracing kernel function as getrusage.
>
>It is good idea disable RUSAGE_BOTH only in sys_getrusage
>(and compat_sys_getrusage).
>
>Regards
>Zhaolei
>----- Original Message -----
>From: "Wenji Huang" <wenji.huang@oracle.com>
>To: "Zhaolei" <zhaolei@cn.fujitsu.com>
>Cc: <systemtap@sourceware.org>
>Sent: Monday, September 10, 2007 11:44 AM
>Subject: Re: [patch] For getrusage's argstr in tapset
>
>
>
>
>>Hi Zhaolei,
>>
>> probe getrusage set probe point at the entry of the function. So the
>>arguments may be any value.
>>
>> So I think to keep the entry in _rusage_who_str, just give another
>>mapping in case of some certain value.
>>
>> Of course, if you can ensure that "who" only be
>>RUSAGE_SELF/RUSAGE_CHILDREN when the syscall called. Just remove.
>>
>>Thanks,
>>Wenji
>>
>>
>>Zhaolei wrote:
>>
>>
>>
>>>Hi, everyone
>>>
>>>In [tapset->probe getrusage], argstr's "who" argument is printed as
>>>"RUSAGE_BOTH" when its value is -2.
>>>But "RUSAGE_BOTH" is not used in getrusage syscall due to the following
>>>kernel source:
>>>asmlinkage long sys_getrusage(int who, struct rusage __user *ru)
>>>{
>>> if (who != RUSAGE_SELF && who != RUSAGE_CHILDREN)
>>> return -EINVAL;
>>> return getrusage(current, who, ru);
>>>}
>>>
>>>So i want to remove RUSAGE_BOTH from _rusage_who_str.
>>>Is there anyone knows why RUSAGE_BOTH is necessary, please tell me.
>>>and if no objection, I will commit it.
>>>
>>>Signed-off-by: "Zhaolei" zhaolei@cn.fujitsu.com
>>>
>>>--- aux_syscalls.stp.old 2007-09-06 11:51:14.000000000 +0900
>>>+++ aux_syscalls.stp 2007-09-06 11:52:22.000000000 +0900
>>>@@ -1661,7 +1661,6 @@ function _rlimit_resource_str(r) {
>>>function _rusage_who_str(w) {
>>> if(w==0) return "RUSAGE_SELF"
>>> if(w==-1) return "RUSAGE_CHILDREN"
>>>- if(w==-2) return "RUSAGE_BOTH"
>>> return sprintf("UNKNOWN VALUE: %d", w)
>>>}
>>>
>>>Regards
>>>Zhaolei
>>>
>>>
>>>
>>>
>>>
>>
>>
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [patch] For getrusage's argstr in tapset
2007-09-10 14:01 ` Wenji Huang
@ 2007-09-13 16:06 ` Zhaolei
0 siblings, 0 replies; 8+ messages in thread
From: Zhaolei @ 2007-09-13 16:06 UTC (permalink / raw)
To: Wenji Huang; +Cc: systemtap
Hello, Wenji.
Within the syscall getrusage, there are only 2 types of flag.
So in my opinion, all the other values are not getrusage's flag and should
be printed as UNKNOWN.
To output a clearer description, we need also print the value of flag in
the case of "UNKNOWN".
So I think it will be better if it can be revised in my way.
Regards
Zhaolei
----- Original Message -----
From: "Wenji Huang" <wenji.huang@oracle.com>
To: "Zhaolei" <zhaolei@cn.fujitsu.com>
Cc: <systemtap@sourceware.org>
Sent: Monday, September 10, 2007 2:18 PM
Subject: Re: [patch] For getrusage's argstr in tapset
> Hi zhaolei,
>
> In normal case, thing may be like that. But in abnormal case, maybe
> wrong arguments are passed to this function. So new entry will
> give a clearer description, instead of general UNKNOWN.
>
> Thanks,
> Wenji
>
> Zhaolei wrote:
>
> >Hello, Wenji.
> >
> >Thank you for your suggestion.
> >
> >I investigated RUSAGE_BOTH and got the following result:
> >1: _rusage_who_str is only used for sys_getrusage(and compat_sys_getrusage)
> > in current tapset, and there is no other syscall which needs to call
> > _rusage_who_str.
> >
> >2: RUSAGE_BOTH is only used in kernel's internal function getrusage, but
> > there is no syscall using this function with RUSAGE_BOTH.
> >
> >So I think RUSAGE_BOTH is not necessary in tracing syscall, but it may be
> > needed when tracing kernel function as getrusage.
> >
> >It is good idea disable RUSAGE_BOTH only in sys_getrusage
> >(and compat_sys_getrusage).
> >
> >Regards
> >Zhaolei
> >----- Original Message -----
> >From: "Wenji Huang" <wenji.huang@oracle.com>
> >To: "Zhaolei" <zhaolei@cn.fujitsu.com>
> >Cc: <systemtap@sourceware.org>
> >Sent: Monday, September 10, 2007 11:44 AM
> >Subject: Re: [patch] For getrusage's argstr in tapset
> >
> >
> >
> >
> >>Hi Zhaolei,
> >>
> >> probe getrusage set probe point at the entry of the function. So the
> >>arguments may be any value.
> >>
> >> So I think to keep the entry in _rusage_who_str, just give another
> >>mapping in case of some certain value.
> >>
> >> Of course, if you can ensure that "who" only be
> >>RUSAGE_SELF/RUSAGE_CHILDREN when the syscall called. Just remove.
> >>
> >>Thanks,
> >>Wenji
> >>
> >>
> >>Zhaolei wrote:
> >>
> >>
> >>
> >>>Hi, everyone
> >>>
> >>>In [tapset->probe getrusage], argstr's "who" argument is printed as
> >>>"RUSAGE_BOTH" when its value is -2.
> >>>But "RUSAGE_BOTH" is not used in getrusage syscall due to the following
> >>>kernel source:
> >>>asmlinkage long sys_getrusage(int who, struct rusage __user *ru)
> >>>{
> >>> if (who != RUSAGE_SELF && who != RUSAGE_CHILDREN)
> >>> return -EINVAL;
> >>> return getrusage(current, who, ru);
> >>>}
> >>>
> >>>So i want to remove RUSAGE_BOTH from _rusage_who_str.
> >>>Is there anyone knows why RUSAGE_BOTH is necessary, please tell me.
> >>>and if no objection, I will commit it.
> >>>
> >>>Signed-off-by: "Zhaolei" zhaolei@cn.fujitsu.com
> >>>
> >>>--- aux_syscalls.stp.old 2007-09-06 11:51:14.000000000 +0900
> >>>+++ aux_syscalls.stp 2007-09-06 11:52:22.000000000 +0900
> >>>@@ -1661,7 +1661,6 @@ function _rlimit_resource_str(r) {
> >>>function _rusage_who_str(w) {
> >>> if(w==0) return "RUSAGE_SELF"
> >>> if(w==-1) return "RUSAGE_CHILDREN"
> >>>- if(w==-2) return "RUSAGE_BOTH"
> >>> return sprintf("UNKNOWN VALUE: %d", w)
> >>>}
> >>>
> >>>Regards
> >>>Zhaolei
> >>>
> >>>
> >>>
> >>>
> >>>
> >>
> >>
> >
> >
> >
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2007-09-13 7:57 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1188780895.5736.ezmlm@sourceware.org>
2007-09-03 12:48 ` [patch] Modify flag's value in tapset's probe delete_module Zhaolei
2007-09-05 14:46 ` [patch] For memory access error when calling _stp_sockaddr_str with addrlen set to 0 Zhaolei
2007-09-05 22:49 ` Frank Ch. Eigler
2007-09-10 5:03 ` [patch] For getrusage's argstr in tapset Zhaolei
2007-09-10 5:21 ` Wenji Huang
2007-09-10 9:28 ` Zhaolei
2007-09-10 14:01 ` Wenji Huang
2007-09-13 16:06 ` Zhaolei
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).