--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email]majordomo@vger.kernel.org[/email]
More majordomo info at [url]http://vger.kernel.org/majordomo-info.html[/url]
Please read the FAQ at [url]http://www.tux.org/lkml/[/url]

07-31-2008, 07:20 AM

unix

Re: [PATCH] cpuset: make ntasks to be a monotonic increasing value

Lai Jiangshan wrote:[color=blue]
> ntasks is not a monotonic increasing value,
> So maybe fudge+1 processes are created when kmalloc and killed
> when kfree in every loop. And the loop will not end or
> repetition a long time.
>
> This patch prevent this kind of attack.
>[/color]

Could you demonstrate how to manage to do this so-called attack in
real-life?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email]majordomo@vger.kernel.org[/email]
More majordomo info at [url]http://vger.kernel.org/majordomo-info.html[/url]
Please read the FAQ at [url]http://www.tux.org/lkml/[/url]

07-31-2008, 07:50 AM

unix

Re: [PATCH] cpuset: make ntasks to be a monotonic increasing value

Er, I can't. But this attack is existence theoretically indeed.
And IMO a monotonic increasing value is very helpful for this loop.

Li Zefan wrote:[color=blue]
> Lai Jiangshan wrote:[color=green]
>> ntasks is not a monotonic increasing value,
>> So maybe fudge+1 processes are created when kmalloc and killed
>> when kfree in every loop. And the loop will not end or
>> repetition a long time.
>>
>> This patch prevent this kind of attack.
>>[/color]
>
> Could you demonstrate how to manage to do this so-called attack in
> real-life?
>
>
>
>[/color]

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email]majordomo@vger.kernel.org[/email]
More majordomo info at [url]http://vger.kernel.org/majordomo-info.html[/url]
Please read the FAQ at [url]http://www.tux.org/lkml/[/url]

07-31-2008, 12:30 PM

unix

Re: [PATCH] cpuset: make ntasks to be a monotonic increasing value

I agree that in theory, this kernel/cpuset.c update_tasks_nodemask()
loop could loop forever, and that by forcing ntasks to keep increasing
monotonically, this guarantees that it cannot loop forever.

I also agree that no known exploit of this exists, and doubt that
any could be created.

I did find the added code logic to be a tad more difficult to read
than I'd like. How about the following patch, instead:

--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <pj@sgi.com> 1.940.382.4214
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email]majordomo@vger.kernel.org[/email]
More majordomo info at [url]http://vger.kernel.org/majordomo-info.html[/url]
Please read the FAQ at [url]http://www.tux.org/lkml/[/url]

07-31-2008, 01:20 PM

unix

Re: [PATCH] cpuset: make ntasks to be a monotonic increasing value

cgroup_task_count() was called twice in every loop. IMO, it's not need.
task number maybe have been increased after kfree. But kfree is generally
quicker than kmalloc. So considering the increasing task number when kfree
is not so useful.

My patch has removed one cgroup_task_count() in every loop.

My patch has an additional line: fudge += fudge >> 3;
This line will reduce loop times remarkably when loop times is large.
(but also loop times is large just in theory)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email]majordomo@vger.kernel.org[/email]
More majordomo info at [url]http://vger.kernel.org/majordomo-info.html[/url]
Please read the FAQ at [url]http://www.tux.org/lkml/[/url]

07-31-2008, 01:40 PM

unix

Re: [PATCH] cpuset: make ntasks to be a monotonic increasing value

Lai wrote:[color=blue]
> cgroup_task_count() was called twice in every loop. IMO, it's not need.[/color]

Ah - true - but I suspect you are trying to optimize the code runtime
(reduce CPU cycles) whereas I am trying to optimize the source code
readability for humans.

Optimizing out the second cgroup_task_count() then requires more
subtle semantics on the pre and post conditions on the ntasks variable
at various points in the code. This makes it slightly harder for
humans to understand the code. That in turn increases the chances
of a subsequent change to the code introducing a bug, because the
author of that subsequent change didn't quite realize all the details
that mattered. Contributing to the introduction of just one bug in
that code loop, at anytime in our lifetimes, would probably cause far
more grief than anything we are trying to fix today.
[color=blue]
> My patch has an additional line: fudge += fudge >> 3;
> This line will reduce loop times remarkably when loop times is large.
> (but also loop times is large just in theory)[/color]

Agreed to this much at least: "just in theory".

I don't usually add code lines to optimize a case that is just in
theory, in code paths that are not critical, when even without the
added code line, it would still work just fine. For one thing, that
hurts all the normal cases by slightly increasing the kernel text size,
hence slightly increasing the number of cache hit misses executing this
piece of code.

But more importantly, it is one more bit of stuff for humans to
have to read in the code.

I prefer to only add kernel source code complexity when it is needed
in practice for correct function or necessary performance. The above
more rapid growth of fudge is not needed for either reason, so far as
I can tell.

--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <pj@sgi.com> 1.940.382.4214
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email]majordomo@vger.kernel.org[/email]
More majordomo info at [url]http://vger.kernel.org/majordomo-info.html[/url]
Please read the FAQ at [url]http://www.tux.org/lkml/[/url]

07-31-2008, 05:10 PM

unix

Re: [PATCH] cpuset: make ntasks to be a monotonic increasing value

On Thu, Jul 31, 2008 at 6:37 AM, Paul Jackson <pj@sgi.com> wrote:[color=blue]
> I prefer to only add kernel source code complexity when it is needed
> in practice for correct function or necessary performance. The above
> more rapid growth of fudge is not needed for either reason, so far as
> I can tell.[/color]

That loop really could do with some updates though - currently it
looks at the mm for every task in the cpuset, rather than filtering
duplicate mms from threaded applications.

Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email]majordomo@vger.kernel.org[/email]
More majordomo info at [url]http://vger.kernel.org/majordomo-info.html[/url]
Please read the FAQ at [url]http://www.tux.org/lkml/[/url]

07-31-2008, 07:40 PM

unix

Re: [PATCH] cpuset: make ntasks to be a monotonic increasing value

Paul M wrote:[color=blue]
> That loop really could do with some updates though - currently it
> looks at the mm for every task in the cpuset, rather than filtering
> duplicate mms from threaded applications.[/color]

Interesting.

After a quick glance, I suppose that we'd still have:
1) allocate an mmarray[] in that particular loop as we do now,
sized large enough for all tasks,
2) convert each task to it's mm, in the next code chunk, with:
mm = get_task_mm(p);

but that then, before we call "mpol_rebind_mm()" for each such
mm, we could essentially do a "sort -u" (sort unique) on that
mmarray[], to remove duplicate mm's. This would not change any
of the existing loops; rather just add one more code paragraph,
to remove the duplicate mm's.

Is that what you're thinking, Paul M?

--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <pj@sgi.com> 1.940.382.4214
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email]majordomo@vger.kernel.org[/email]
More majordomo info at [url]http://vger.kernel.org/majordomo-info.html[/url]
Please read the FAQ at [url]http://www.tux.org/lkml/[/url]

07-31-2008, 08:10 PM

unix

Re: [PATCH] cpuset: make ntasks to be a monotonic increasing value

On Thu, Jul 31, 2008 at 12:38 PM, Paul Jackson <pj@sgi.com> wrote:[color=blue]
> Paul M wrote:[color=green]
>> That loop really could do with some updates though - currently it
>> looks at the mm for every task in the cpuset, rather than filtering
>> duplicate mms from threaded applications.[/color]
>
> Interesting.
>
> After a quick glance, I suppose that we'd still have:
> 1) allocate an mmarray[] in that particular loop as we do now,
> sized large enough for all tasks,
> 2) convert each task to it's mm, in the next code chunk, with:
> mm = get_task_mm(p);
>
> but that then, before we call "mpol_rebind_mm()" for each such
> mm, we could essentially do a "sort -u" (sort unique) on that
> mmarray[], to remove duplicate mm's. This would not change any
> of the existing loops; rather just add one more code paragraph,
> to remove the duplicate mm's.[/color]

Yes, something like that.

Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email]majordomo@vger.kernel.org[/email]
More majordomo info at [url]http://vger.kernel.org/majordomo-info.html[/url]
Please read the FAQ at [url]http://www.tux.org/lkml/[/url]

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email]majordomo@vger.kernel.org[/email]
More majordomo info at [url]http://vger.kernel.org/majordomo-info.html[/url]
Please read the FAQ at [url]http://www.tux.org/lkml/[/url]