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

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: <m1zl37256o.fsf@fess.ebiederm.org>
Date:	Wed, 17 Feb 2010 08:33:03 -0800
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Cong Wang <amwang@...hat.com>
Cc:	Octavian Purdila <opurdila@...acom.com>,
	David Miller <davem@...emloft.net>,
	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	Linux Kernel Developers <linux-kernel@...r.kernel.org>
Subject: Re: [net-next PATCH v4 1/3] sysctl: refactor integer handling proc code

Cong Wang <amwang@...hat.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

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html