Messages in this thread | | | Date | Wed, 12 Aug 2009 10:58:19 -0700 (PDT) | From | Vincent Li <> | Subject | Re: [PATCH] fs/proc/: fix input sanity check. |
| |
On Tue, 11 Aug 2009, Andrew Morton wrote:
> On Tue, 11 Aug 2009 14:34:26 -0700 > Vincent Li <macli@brc.ubc.ca> wrote: > > > fix fs/proc/task_mmu.c clear_refs_write(), fs/proc/base.c proc_fault_inject_write() > > and proc_fault_inject_operations() input sanity check by following the disccusion of > > http://marc.info/?l=linux-mm&m=124938168905463&w=2. > > Would prefer that the changelog not consist of a link to another patch > which is already quite poorly changelogged, please!
It make sense to me only after you point it out to me, That is process of learning :)
> > What, precisely was wrong with the old code and how does the patch > change its behaviour. >
It is after I read your comment about the strange handling of userspace string input http://marc.info/?l=linux-mm&m=124890923527186&w=2 and Kosaki's "fix oom_adjust_write() input sanity check" patch to match your comment http://marc.info/?l=linux-mm&m=124938168905463&w=2.
I re-read the clear_refs_write() in fs/proc/task_mmu.c, If I understand it correctly, The old code takes non-zero string from userspace and convert it to unsigned long interger value. for example,It accepts:
echo 1 > /proc/pid/clear_refs echo 12foo > /proc/pid/clear_refs
The new code add the stripping of leading and trailing whitespace, remove the obfuscated check for zero-length input at the end of the function as you commented. Will this new code break the existing application behaviour? David Rientjes is the author of clear_refs, I cced him to see if he has any comment. I have not read the proc_fault_inject_write() and proc_fault_inject_operations() yet, I will drop the changes to those two functions
> > This matters. The patch potentially changes the kernel<->userspace > interface and we must fully understand the nature of that change so as > to minimise the risk of breaking existing userspace application code. >
Point taken, I am in a rush to post the patch without fully understanding what the functions do and the impact of patch changes.
> Thanks.
You are welcome.
Vincent Li Biomedical Research Center University of British Columbia
| |