linux-kernel - Re: [PATCH] fs/proc/: fix input sanity check.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.1.00.0908121003180.21620@mail.selltech.ca>
Date:	Wed, 12 Aug 2009 10:58:19 -0700 (PDT)
From:	Vincent Li <macli@....ubc.ca>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	linux-kernel@...r.kernel.org, kosaki.motohiro@...fujitsu.com,
	rientjes@...gle.com, mel@....ul.ie, mpm@...enic.com
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@....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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/