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

[Robert Richter <robert.richter@amd.com>]

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@google.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