linux-kernel - Re: [PATCH] oprofilefs: Handle zero-length writes.

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] [day] [month] [year] [list]

Message-ID: <20110817003941.GC11702@erda.amd.com>
Date:	Wed, 17 Aug 2011 02:39:42 +0200
From:	Robert Richter <robert.richter@....com>
To:	Mike Waychison <mikew@...gle.com>
CC:	"oprofile-list@...ts.sf.net" <oprofile-list@...ts.sf.net>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] oprofilefs: Handle zero-length writes.

On 12.08.11 19:42:19, Mike Waychison wrote:
> Currently in oprofilefs, files that use ulong_fops mis-handle writes of
> zero length.  A count of 0 causes oprofilefs_ulong_from_user to return
> 0 (success), which then leads to oprofile_set_ulong being called to
> stuff "value" into file->private_data without it being initialized.
> 
> Fix this by moving the check for a zero-length write up into
> ulong_write_file.
> 
> Signed-off-by: Mike Waychison <mikew@...gle.com>
> ---
>  drivers/oprofile/oprofilefs.c |    5 ++---
>  1 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/oprofile/oprofilefs.c b/drivers/oprofile/oprofilefs.c
> index e9ff6f7..ee14e6e 100644
> --- a/drivers/oprofile/oprofilefs.c
> +++ b/drivers/oprofile/oprofilefs.c
> @@ -65,9 +65,6 @@ int oprofilefs_ulong_from_user(unsigned long *val, char const __user *buf, size_
>  	char tmpbuf[TMPBUFSIZE];
>  	unsigned long flags;
>  
> -	if (!count)
> -		return 0;
> -

Yes, *val is clearly used uninitialized for !count.

But it might be ok, not to touch it in oprofilefs_ulong_from_user.
>From man 3 write:

 "if nbyte is zero and the file is a regular file ... the write()
 function shall return zero and have no other results"

Actually, oprofilefs_ulong_from_user() must be called with an
initialized value ...

>  	if (count > TMPBUFSIZE - 1)
>  		return -EINVAL;
>  
> @@ -97,6 +94,8 @@ static ssize_t ulong_write_file(struct file *file, char const __user *buf, size_
>  
>  	if (*offset)
>  		return -EINVAL;
> +	if (count == 0)
> +		return 0;

... or we add this check to all other users of
oprofilefs_ulong_from_user() too. Without those checks they would set
its value to 0 if count is 0.

A small nitpick: I would prefer

	if (!count) ...

-Robert

>  
>  	retval = oprofilefs_ulong_from_user(&value, buf, count);
>  	if (retval)
> -- 
> 1.7.3.1
> 
> 

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

--
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/