Re: [PATCH] kernel/fork.c: VM accounting bugfix (2.6.11-rc3-bk5)
Chris Wright wrote: You missed one subtle point. That failure case actually unaccts 0 pages (note the use of charge). Not the nicest, but I believe correct. Right. I did miss that. Thanks for the explanations, Chris and Hugh, I appreciate it. Mark F. Haigh [EMAIL PROTECTED] - 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] kernel/fork.c: VM accounting bugfix (2.6.11-rc3-bk5)
On Wed, 9 Feb 2005, Chris Wright wrote: > * Hugh Dickins ([EMAIL PROTECTED]) wrote: > > dup_mmap's charge starts out at 0 and gets added to each time around > > the loop through vmas; if security_vm_enough_memory fails at any point > > in that loop, we need to vm_unacct_memory the charge already accumulated. > > If that's the requirement, then it's broken as is, because there is > nothing accumulating. len is re-determined each pass, and charge is > reset each pass. You're right, it's me who's wrong, sorry. I was remembering how it was before 2.6.7, when Oleg pointed out that it was doubly unaccounting (and it's probably me who was guilty of that double unaccounting too). > But I think that it's ok, as we only care about the > last pass. If dup_mmap() fails part way through, the cleanup path should > call unaccount for the (potentially) accounted by not fully setup vma then > call exit_mmap() and clear all the vmas that got accounted for already. Yes, that was Oleg's point when he fixed it. > Either way, Mark's patch is not needed, and I don't think anything needs > patching in this area. Hugh, do you agree? Yes I agree - thanks for clearing that up, Chris. Hugh - 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] kernel/fork.c: VM accounting bugfix (2.6.11-rc3-bk5)
* Hugh Dickins ([EMAIL PROTECTED]) wrote: > dup_mmap's charge starts out at 0 and gets added to each time around > the loop through vmas; if security_vm_enough_memory fails at any point > in that loop, we need to vm_unacct_memory the charge already accumulated. If that's the requirement, then it's broken as is, because there is nothing accumulating. len is re-determined each pass, and charge is reset each pass. But I think that it's ok, as we only care about the last pass. If dup_mmap() fails part way through, the cleanup path should call unaccount for the (potentially) accounted by not fully setup vma then call exit_mmap() and clear all the vmas that got accounted for already. Either way, Mark's patch is not needed, and I don't think anything needs patching in this area. Hugh, do you agree? thanks, -chris -- Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net - 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] kernel/fork.c: VM accounting bugfix (2.6.11-rc3-bk5)
On Tue, 8 Feb 2005, Chris Wright wrote: > * Mark F. Haigh ([EMAIL PROTECTED]) wrote: > > > > If security_vm_enough_memory() fails there, then we vm_unacct_memory() > > that we never accounted (if security_vm_enough_memory() fails, no memory > > is accounted). > > You missed one subtle point. That failure case actually unaccts 0 pages > (note the use of charge). Not the nicest, but I believe correct. Not quite: Mark's patch is worse than unnecessary, it's wrong. dup_mmap's charge starts out at 0 and gets added to each time around the loop through vmas; if security_vm_enough_memory fails at any point in that loop, we need to vm_unacct_memory the charge already accumulated. Hugh - 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] kernel/fork.c: VM accounting bugfix (2.6.11-rc3-bk5)
On Tue, 8 Feb 2005, Chris Wright wrote: * Mark F. Haigh ([EMAIL PROTECTED]) wrote: If security_vm_enough_memory() fails there, then we vm_unacct_memory() that we never accounted (if security_vm_enough_memory() fails, no memory is accounted). You missed one subtle point. That failure case actually unaccts 0 pages (note the use of charge). Not the nicest, but I believe correct. Not quite: Mark's patch is worse than unnecessary, it's wrong. dup_mmap's charge starts out at 0 and gets added to each time around the loop through vmas; if security_vm_enough_memory fails at any point in that loop, we need to vm_unacct_memory the charge already accumulated. Hugh - 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] kernel/fork.c: VM accounting bugfix (2.6.11-rc3-bk5)
* Hugh Dickins ([EMAIL PROTECTED]) wrote: dup_mmap's charge starts out at 0 and gets added to each time around the loop through vmas; if security_vm_enough_memory fails at any point in that loop, we need to vm_unacct_memory the charge already accumulated. If that's the requirement, then it's broken as is, because there is nothing accumulating. len is re-determined each pass, and charge is reset each pass. But I think that it's ok, as we only care about the last pass. If dup_mmap() fails part way through, the cleanup path should call unaccount for the (potentially) accounted by not fully setup vma then call exit_mmap() and clear all the vmas that got accounted for already. Either way, Mark's patch is not needed, and I don't think anything needs patching in this area. Hugh, do you agree? thanks, -chris -- Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net - 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] kernel/fork.c: VM accounting bugfix (2.6.11-rc3-bk5)
On Wed, 9 Feb 2005, Chris Wright wrote: * Hugh Dickins ([EMAIL PROTECTED]) wrote: dup_mmap's charge starts out at 0 and gets added to each time around the loop through vmas; if security_vm_enough_memory fails at any point in that loop, we need to vm_unacct_memory the charge already accumulated. If that's the requirement, then it's broken as is, because there is nothing accumulating. len is re-determined each pass, and charge is reset each pass. You're right, it's me who's wrong, sorry. I was remembering how it was before 2.6.7, when Oleg pointed out that it was doubly unaccounting (and it's probably me who was guilty of that double unaccounting too). But I think that it's ok, as we only care about the last pass. If dup_mmap() fails part way through, the cleanup path should call unaccount for the (potentially) accounted by not fully setup vma then call exit_mmap() and clear all the vmas that got accounted for already. Yes, that was Oleg's point when he fixed it. Either way, Mark's patch is not needed, and I don't think anything needs patching in this area. Hugh, do you agree? Yes I agree - thanks for clearing that up, Chris. Hugh - 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] kernel/fork.c: VM accounting bugfix (2.6.11-rc3-bk5)
Chris Wright wrote: snip You missed one subtle point. That failure case actually unaccts 0 pages (note the use of charge). Not the nicest, but I believe correct. Right. I did miss that. Thanks for the explanations, Chris and Hugh, I appreciate it. Mark F. Haigh [EMAIL PROTECTED] - 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] kernel/fork.c: VM accounting bugfix (2.6.11-rc3-bk5)
Hi Mark, * Mark F. Haigh ([EMAIL PROTECTED]) wrote: > [Aargh! Missing Signed-off-by.] > > Unless I'm missing something, in kernel/fork.c, dup_mmap(): > > if (security_vm_enough_memory(len)) > goto fail_nomem; > /* ... */ > fail_nomem: > retval = -ENOMEM; > vm_unacct_memory(charge); > /* ... */ > > If security_vm_enough_memory() fails there, then we vm_unacct_memory() > that we never accounted (if security_vm_enough_memory() fails, no memory > is accounted). You missed one subtle point. That failure case actually unaccts 0 pages (note the use of charge). Not the nicest, but I believe correct. thanks, -chris -- Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net - 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/
[PATCH] kernel/fork.c: VM accounting bugfix (2.6.11-rc3-bk5)
[Aargh! Missing Signed-off-by.]
Unless I'm missing something, in kernel/fork.c, dup_mmap():
if (security_vm_enough_memory(len))
goto fail_nomem;
/* ... */
fail_nomem:
retval = -ENOMEM;
vm_unacct_memory(charge);
/* ... */
If security_vm_enough_memory() fails there, then we vm_unacct_memory()
that we never accounted (if security_vm_enough_memory() fails, no memory
is accounted).
If it is in fact a bug, a simple but largely untested patch (against
2.6.11-rc3-bk5) is included.
Mark F. Haigh
[EMAIL PROTECTED]
Signed-off-by: Mark F. Haigh <[EMAIL PROTECTED]>
--- linux-2.6.11-rc3-bk5/kernel/fork.c.orig 2005-02-08 19:12:26.254589504
-0800
+++ linux-2.6.11-rc3-bk5/kernel/fork.c 2005-02-08 19:16:30.756419576 -0800
@@ -193,8 +193,10 @@
charge = 0;
if (mpnt->vm_flags & VM_ACCOUNT) {
unsigned int len = (mpnt->vm_end - mpnt->vm_start) >>
PAGE_SHIFT;
- if (security_vm_enough_memory(len))
- goto fail_nomem;
+ if (security_vm_enough_memory(len)) {
+ retval = -ENOMEM;
+ goto out;
+ }
charge = len;
}
tmp = kmem_cache_alloc(vm_area_cachep, SLAB_KERNEL);
[PATCH] kernel/fork.c: VM accounting bugfix (2.6.11-rc3-bk5)
Unless I'm missing something, in kernel/fork.c, dup_mmap():
if (security_vm_enough_memory(len))
goto fail_nomem;
/* ... */
fail_nomem:
retval = -ENOMEM;
vm_unacct_memory(charge);
/* ... */
If security_vm_enough_memory() fails there, then we vm_unacct_memory()
that we never accounted (if security_vm_enough_memory() fails, no memory
is accounted).
If it is in fact a bug, a simple but largely untested patch (against
2.6.11-rc3-bk5) is included.
Mark F. Haigh
[EMAIL PROTECTED]
--- linux-2.6.11-rc3-bk5/kernel/fork.c.orig 2005-02-08 19:12:26.254589504
-0800
+++ linux-2.6.11-rc3-bk5/kernel/fork.c 2005-02-08 19:16:30.756419576 -0800
@@ -193,8 +193,10 @@
charge = 0;
if (mpnt->vm_flags & VM_ACCOUNT) {
unsigned int len = (mpnt->vm_end - mpnt->vm_start) >>
PAGE_SHIFT;
- if (security_vm_enough_memory(len))
- goto fail_nomem;
+ if (security_vm_enough_memory(len)) {
+ retval = -ENOMEM;
+ goto out;
+ }
charge = len;
}
tmp = kmem_cache_alloc(vm_area_cachep, SLAB_KERNEL);
[PATCH] kernel/fork.c: VM accounting bugfix (2.6.11-rc3-bk5)
Unless I'm missing something, in kernel/fork.c, dup_mmap():
if (security_vm_enough_memory(len))
goto fail_nomem;
/* ... */
fail_nomem:
retval = -ENOMEM;
vm_unacct_memory(charge);
/* ... */
If security_vm_enough_memory() fails there, then we vm_unacct_memory()
that we never accounted (if security_vm_enough_memory() fails, no memory
is accounted).
If it is in fact a bug, a simple but largely untested patch (against
2.6.11-rc3-bk5) is included.
Mark F. Haigh
[EMAIL PROTECTED]
--- linux-2.6.11-rc3-bk5/kernel/fork.c.orig 2005-02-08 19:12:26.254589504
-0800
+++ linux-2.6.11-rc3-bk5/kernel/fork.c 2005-02-08 19:16:30.756419576 -0800
@@ -193,8 +193,10 @@
charge = 0;
if (mpnt-vm_flags VM_ACCOUNT) {
unsigned int len = (mpnt-vm_end - mpnt-vm_start)
PAGE_SHIFT;
- if (security_vm_enough_memory(len))
- goto fail_nomem;
+ if (security_vm_enough_memory(len)) {
+ retval = -ENOMEM;
+ goto out;
+ }
charge = len;
}
tmp = kmem_cache_alloc(vm_area_cachep, SLAB_KERNEL);
[PATCH] kernel/fork.c: VM accounting bugfix (2.6.11-rc3-bk5)
[Aargh! Missing Signed-off-by.]
Unless I'm missing something, in kernel/fork.c, dup_mmap():
if (security_vm_enough_memory(len))
goto fail_nomem;
/* ... */
fail_nomem:
retval = -ENOMEM;
vm_unacct_memory(charge);
/* ... */
If security_vm_enough_memory() fails there, then we vm_unacct_memory()
that we never accounted (if security_vm_enough_memory() fails, no memory
is accounted).
If it is in fact a bug, a simple but largely untested patch (against
2.6.11-rc3-bk5) is included.
Mark F. Haigh
[EMAIL PROTECTED]
Signed-off-by: Mark F. Haigh [EMAIL PROTECTED]
--- linux-2.6.11-rc3-bk5/kernel/fork.c.orig 2005-02-08 19:12:26.254589504
-0800
+++ linux-2.6.11-rc3-bk5/kernel/fork.c 2005-02-08 19:16:30.756419576 -0800
@@ -193,8 +193,10 @@
charge = 0;
if (mpnt-vm_flags VM_ACCOUNT) {
unsigned int len = (mpnt-vm_end - mpnt-vm_start)
PAGE_SHIFT;
- if (security_vm_enough_memory(len))
- goto fail_nomem;
+ if (security_vm_enough_memory(len)) {
+ retval = -ENOMEM;
+ goto out;
+ }
charge = len;
}
tmp = kmem_cache_alloc(vm_area_cachep, SLAB_KERNEL);
Re: [PATCH] kernel/fork.c: VM accounting bugfix (2.6.11-rc3-bk5)
Hi Mark, * Mark F. Haigh ([EMAIL PROTECTED]) wrote: [Aargh! Missing Signed-off-by.] Unless I'm missing something, in kernel/fork.c, dup_mmap(): if (security_vm_enough_memory(len)) goto fail_nomem; /* ... */ fail_nomem: retval = -ENOMEM; vm_unacct_memory(charge); /* ... */ If security_vm_enough_memory() fails there, then we vm_unacct_memory() that we never accounted (if security_vm_enough_memory() fails, no memory is accounted). You missed one subtle point. That failure case actually unaccts 0 pages (note the use of charge). Not the nicest, but I believe correct. thanks, -chris -- Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net - 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/
