netdev - Re: net/can: warning in raw_setsockopt/__alloc_pages

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: <6fe05efd-eb2c-a5e2-9d45-48f2c3098abb@hartkopp.net>
Date:   Fri, 2 Dec 2016 18:05:48 +0100
From:   Oliver Hartkopp <socketcan@...tkopp.net>
To:     Marc Kleine-Budde <mkl@...gutronix.de>,
        Andrey Konovalov <andreyknvl@...gle.com>,
        "David S. Miller" <davem@...emloft.net>, linux-can@...r.kernel.org,
        netdev <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Cc:     Dmitry Vyukov <dvyukov@...gle.com>,
        Kostya Serebryany <kcc@...gle.com>,
        syzkaller <syzkaller@...glegroups.com>
Subject: Re: net/can: warning in raw_setsockopt/__alloc_pages_slowpath



On 12/02/2016 04:42 PM, Marc Kleine-Budde wrote:
> On 12/02/2016 04:11 PM, Oliver Hartkopp wrote:
>>
>>
>> On 12/02/2016 02:24 PM, Marc Kleine-Budde wrote:
>>> On 12/02/2016 01:43 PM, Andrey Konovalov wrote:
>>
>>
>>>>  [<ffffffff8369e0de>] raw_setsockopt+0x1be/0x9f0 net/can/raw.c:506
>>>
>>> We should add a check for a sensible optlen....
>>>
>>>> static int raw_setsockopt(struct socket *sock, int level, int optname,
>>>> 			  char __user *optval, unsigned int optlen)
>>>> {
>>>> 	struct sock *sk = sock->sk;
>>>> 	struct raw_sock *ro = raw_sk(sk);
>>>> 	struct can_filter *filter = NULL;  /* dyn. alloc'ed filters */
>>>> 	struct can_filter sfilter;         /* single filter */
>>>> 	struct net_device *dev = NULL;
>>>> 	can_err_mask_t err_mask = 0;
>>>> 	int count = 0;
>>>> 	int err = 0;
>>>>
>>>> 	if (level != SOL_CAN_RAW)
>>>> 		return -EINVAL;
>>>>
>>>> 	switch (optname) {
>>>>
>>>> 	case CAN_RAW_FILTER:
>>>> 		if (optlen % sizeof(struct can_filter) != 0)
>>>> 			return -EINVAL;
>>>
>>> here...
>>>
>>> 		if (optlen > 64 * sizeof(struct can_filter))
>>> 			return -EINVAL;
>>>
>>
>> Agreed.
>>
>> But what is sensible here?
>> 64 filters is way to small IMO.
>>
>> When thinking about picking a bunch of single CAN IDs I would tend to
>> something like 512 filters.
>
> Ok - 64 was just an arbitrary chosen value for demonstration purposes :)
>

:-)

Would you like to send a patch?

Regards,
Oliver