* [PATCH] kernel/delayacct.c: check NULL for member variable 'delays' in all extern functions.
@ 2013-08-20 2:40 Chen Gang
2013-08-20 2:44 ` [PATCH] kernel/taskstats.c: add nla_nest_cancel() for failure processing between nla_nest_start() and nla_nest_end() Chen Gang
2013-08-21 22:10 ` [PATCH] kernel/delayacct.c: check NULL for member variable 'delays' in all extern functions Andrew Morton
0 siblings, 2 replies; 6+ messages in thread
From: Chen Gang @ 2013-08-20 2:40 UTC (permalink / raw)
To: bsingharora; +Cc: linux-kernel@vger.kernel.org
The member variable 'delays' may be NULL, so need check NULL before use
it for all extern functions, just like __delayacct_tsk_init() and
__delayacct_add_tsk() have already done.
Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
kernel/delayacct.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/kernel/delayacct.c b/kernel/delayacct.c
index d473988..10cddce 100644
--- a/kernel/delayacct.c
+++ b/kernel/delayacct.c
@@ -81,11 +81,15 @@ static void delayacct_end(struct timespec *start, struct timespec *end,
void __delayacct_blkio_start(void)
{
+ if (!current->delays)
+ return;
delayacct_start(¤t->delays->blkio_start);
}
void __delayacct_blkio_end(void)
{
+ if (!current->delays)
+ return;
if (current->delays->flags & DELAYACCT_PF_SWAPIN)
/* Swapin block I/O */
delayacct_end(¤t->delays->blkio_start,
@@ -167,6 +171,8 @@ __u64 __delayacct_blkio_ticks(struct task_struct *tsk)
__u64 ret;
unsigned long flags;
+ if (!tsk->delays)
+ return 0;
spin_lock_irqsave(&tsk->delays->lock, flags);
ret = nsec_to_clock_t(tsk->delays->blkio_delay +
tsk->delays->swapin_delay);
@@ -176,11 +182,15 @@ __u64 __delayacct_blkio_ticks(struct task_struct *tsk)
void __delayacct_freepages_start(void)
{
+ if (!current->delays)
+ return;
delayacct_start(¤t->delays->freepages_start);
}
void __delayacct_freepages_end(void)
{
+ if (!current->delays)
+ return;
delayacct_end(¤t->delays->freepages_start,
¤t->delays->freepages_end,
¤t->delays->freepages_delay,
--
1.7.7.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] kernel/taskstats.c: add nla_nest_cancel() for failure processing between nla_nest_start() and nla_nest_end()
2013-08-20 2:40 [PATCH] kernel/delayacct.c: check NULL for member variable 'delays' in all extern functions Chen Gang
@ 2013-08-20 2:44 ` Chen Gang
2013-09-03 5:07 ` Chen Gang
2013-08-21 22:10 ` [PATCH] kernel/delayacct.c: check NULL for member variable 'delays' in all extern functions Andrew Morton
1 sibling, 1 reply; 6+ messages in thread
From: Chen Gang @ 2013-08-20 2:44 UTC (permalink / raw)
To: bsingharora; +Cc: linux-kernel@vger.kernel.org
When failure occurs between nla_nest_start() and nla_nest_end(), need
call nla_nest_cancel() to clean up related things.
Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
kernel/taskstats.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/kernel/taskstats.c b/kernel/taskstats.c
index 145bb4d..1db6808 100644
--- a/kernel/taskstats.c
+++ b/kernel/taskstats.c
@@ -404,11 +404,15 @@ static struct taskstats *mk_reply(struct sk_buff *skb, int type, u32 pid)
if (!na)
goto err;
- if (nla_put(skb, type, sizeof(pid), &pid) < 0)
+ if (nla_put(skb, type, sizeof(pid), &pid) < 0) {
+ nla_nest_cancel(skb, na);
goto err;
+ }
ret = nla_reserve(skb, TASKSTATS_TYPE_STATS, sizeof(struct taskstats));
- if (!ret)
+ if (!ret) {
+ nla_nest_cancel(skb, na);
goto err;
+ }
nla_nest_end(skb, na);
return nla_data(ret);
--
1.7.7.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] kernel/taskstats.c: add nla_nest_cancel() for failure processing between nla_nest_start() and nla_nest_end()
2013-08-20 2:44 ` [PATCH] kernel/taskstats.c: add nla_nest_cancel() for failure processing between nla_nest_start() and nla_nest_end() Chen Gang
@ 2013-09-03 5:07 ` Chen Gang
0 siblings, 0 replies; 6+ messages in thread
From: Chen Gang @ 2013-09-03 5:07 UTC (permalink / raw)
To: bsingharora; +Cc: linux-kernel@vger.kernel.org, Andrew Morton
Hello maintainers:
Please help check this patch, when you have time.
Thanks.
On 08/20/2013 10:44 AM, Chen Gang wrote:
> When failure occurs between nla_nest_start() and nla_nest_end(), need
> call nla_nest_cancel() to clean up related things.
>
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
> kernel/taskstats.c | 8 ++++++--
> 1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/taskstats.c b/kernel/taskstats.c
> index 145bb4d..1db6808 100644
> --- a/kernel/taskstats.c
> +++ b/kernel/taskstats.c
> @@ -404,11 +404,15 @@ static struct taskstats *mk_reply(struct sk_buff *skb, int type, u32 pid)
> if (!na)
> goto err;
>
> - if (nla_put(skb, type, sizeof(pid), &pid) < 0)
> + if (nla_put(skb, type, sizeof(pid), &pid) < 0) {
> + nla_nest_cancel(skb, na);
> goto err;
> + }
> ret = nla_reserve(skb, TASKSTATS_TYPE_STATS, sizeof(struct taskstats));
> - if (!ret)
> + if (!ret) {
> + nla_nest_cancel(skb, na);
> goto err;
> + }
> nla_nest_end(skb, na);
>
> return nla_data(ret);
>
--
Chen Gang
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kernel/delayacct.c: check NULL for member variable 'delays' in all extern functions.
2013-08-20 2:40 [PATCH] kernel/delayacct.c: check NULL for member variable 'delays' in all extern functions Chen Gang
2013-08-20 2:44 ` [PATCH] kernel/taskstats.c: add nla_nest_cancel() for failure processing between nla_nest_start() and nla_nest_end() Chen Gang
@ 2013-08-21 22:10 ` Andrew Morton
2013-08-22 2:17 ` Chen Gang
1 sibling, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2013-08-21 22:10 UTC (permalink / raw)
To: Chen Gang; +Cc: bsingharora, linux-kernel@vger.kernel.org
On Tue, 20 Aug 2013 10:40:03 +0800 Chen Gang <gang.chen@asianux.com> wrote:
> The member variable 'delays' may be NULL, so need check NULL before use
> it for all extern functions, just like __delayacct_tsk_init() and
> __delayacct_add_tsk() have already done.
If task.delays is NULL, the kernel will oops. I don't recall seeing
such oops reports hence I suspect that you missed some check somewhere.
For example, delayacct_blkio_start() checks current->delays before
calling __delayacct_blkio_start(), so why retest current->delays in
__delayacct_blkio_start()?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kernel/delayacct.c: check NULL for member variable 'delays' in all extern functions.
2013-08-21 22:10 ` [PATCH] kernel/delayacct.c: check NULL for member variable 'delays' in all extern functions Andrew Morton
@ 2013-08-22 2:17 ` Chen Gang
2013-09-03 5:03 ` [PATCH] kernel/delayacct.c: remove redundancy checking in __delayacct_add_tsk() Chen Gang
0 siblings, 1 reply; 6+ messages in thread
From: Chen Gang @ 2013-08-22 2:17 UTC (permalink / raw)
To: Andrew Morton; +Cc: bsingharora, linux-kernel@vger.kernel.org
On 08/22/2013 06:10 AM, Andrew Morton wrote:
> On Tue, 20 Aug 2013 10:40:03 +0800 Chen Gang <gang.chen@asianux.com> wrote:
>
>> The member variable 'delays' may be NULL, so need check NULL before use
>> it for all extern functions, just like __delayacct_tsk_init() and
>> __delayacct_add_tsk() have already done.
>
> If task.delays is NULL, the kernel will oops. I don't recall seeing
> such oops reports hence I suspect that you missed some check somewhere.
>
> For example, delayacct_blkio_start() checks current->delays before
> calling __delayacct_blkio_start(), so why retest current->delays in
> __delayacct_blkio_start()?
>
>
Hmm... Can we assume "__*" extern function is treated as "should not be
used by outside" ?
If so, the original implementation have done (check NULL before use it
for all extern functions), just like your valuable example, thanks.
Hmm... and better to remove the useless code in __delayacct_add_tsk(),
which the related wrapper function has done, the diff is below:
------------------------------diff begin--------------------------------
diff --git a/kernel/delayacct.c b/kernel/delayacct.c
index d473988..da8b153 100644
--- a/kernel/delayacct.c
+++ b/kernel/delayacct.c
@@ -108,12 +108,6 @@ int __delayacct_add_tsk(struct taskstats *d, struct task_struct *tsk)
struct timespec ts;
cputime_t utime, stime, stimescaled, utimescaled;
- /* Though tsk->delays accessed later, early exit avoids
- * unnecessary returning of other data
- */
- if (!tsk->delays)
- goto done;
-
tmp = (s64)d->cpu_run_real_total;
task_cputime(tsk, &utime, &stime);
cputime_to_timespec(utime + stime, &ts);
------------------------------diff end----------------------------------
If this diff is OK, I will send related patch for it.
Thanks.
--
Chen Gang
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] kernel/delayacct.c: remove redundancy checking in __delayacct_add_tsk()
2013-08-22 2:17 ` Chen Gang
@ 2013-09-03 5:03 ` Chen Gang
0 siblings, 0 replies; 6+ messages in thread
From: Chen Gang @ 2013-09-03 5:03 UTC (permalink / raw)
To: Andrew Morton, bsingharora; +Cc: linux-kernel@vger.kernel.org
The wrapper function delayacct_add_tsk() already checked 'tsk->delays',
and __delayacct_add_tsk() has no another direct callers, so can remove
the redundancy checking code.
And the label 'done' is also useless, so remove it, too.
Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
kernel/delayacct.c | 7 -------
1 files changed, 0 insertions(+), 7 deletions(-)
diff --git a/kernel/delayacct.c b/kernel/delayacct.c
index d473988..54996b7 100644
--- a/kernel/delayacct.c
+++ b/kernel/delayacct.c
@@ -108,12 +108,6 @@ int __delayacct_add_tsk(struct taskstats *d, struct task_struct *tsk)
struct timespec ts;
cputime_t utime, stime, stimescaled, utimescaled;
- /* Though tsk->delays accessed later, early exit avoids
- * unnecessary returning of other data
- */
- if (!tsk->delays)
- goto done;
-
tmp = (s64)d->cpu_run_real_total;
task_cputime(tsk, &utime, &stime);
cputime_to_timespec(utime + stime, &ts);
@@ -158,7 +152,6 @@ int __delayacct_add_tsk(struct taskstats *d, struct task_struct *tsk)
d->freepages_count += tsk->delays->freepages_count;
spin_unlock_irqrestore(&tsk->delays->lock, flags);
-done:
return 0;
}
--
1.7.7.6
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-09-03 5:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-20 2:40 [PATCH] kernel/delayacct.c: check NULL for member variable 'delays' in all extern functions Chen Gang
2013-08-20 2:44 ` [PATCH] kernel/taskstats.c: add nla_nest_cancel() for failure processing between nla_nest_start() and nla_nest_end() Chen Gang
2013-09-03 5:07 ` Chen Gang
2013-08-21 22:10 ` [PATCH] kernel/delayacct.c: check NULL for member variable 'delays' in all extern functions Andrew Morton
2013-08-22 2:17 ` Chen Gang
2013-09-03 5:03 ` [PATCH] kernel/delayacct.c: remove redundancy checking in __delayacct_add_tsk() Chen Gang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox