Re: [net-next PATCH v4 1/3] sysctl: refactor integer handling proc code

[ebiederm@xmission.com (Eric W. Biederman)]

Cong Wang <amwang@redhat.com> writes:

> Octavian Purdila wrote:
>> On Tuesday 16 February 2010 15:09:51 you wrote:
>>> Octavian Purdila wrote:
>>>> On Tuesday 16 February 2010 10:41:07 you wrote:
>>>>>> +static int proc_skip_wspace(char __user **buf, size_t *size)
>>>>>> +{
>>>>>> +     char c;
>>>>>> +
>>>>>> +     while (*size) {
>>>>>> +             if (get_user(c, *buf))
>>>>>> +                     return -EFAULT;
>>>>>> +             if (!isspace(c))
>>>>>> +                     break;
>>>>>> +             (*size)--; (*buf)++;
>>>>>> +     }
>>>>>> +
>>>>>> +     return 0;
>>>>>> +}
>>>>> In lib/string.c we have skip_spaces(), I think we can use it
>>>>> here instead of inventing another one.
>>>> I'm afraid we can't, skip_spaces does not accept userspace buffers.
>>> Well, you need to use copy_from_user() before call it.
>>>
>>
>> And how much would you copy? You need to either use a stack buffer and do a
>> loop copy or you would need to copy the whole userspace buffer which means we
>> need to allocate a kernel buffer. I think its much cleaner the way is
>> currently done.
>
> Yeah, maybe just a personal preference. :-/

There can be valid security reasons for copying all of the data before
processing it.

Semantically if we an guarantee that we either have processed the
entire buffer or failed the entire buffer and no changes have occurred
in the kernel that seems like a much easier semantic to work with in
user space.

Eric