Re: net/can: warning in raw_setsockopt/__alloc_pages_slowpath

[Oliver Hartkopp]

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.

Regards,
Oliver

> > 		count = optlen / sizeof(struct can_filter);
> >
> > 		if (count > 1) {
> > 			/* filter does not fit into dfilter => alloc space */
> > 			filter = memdup_user(optval, optlen);
> > 			if (IS_ERR(filter))
> > 				return PTR_ERR(filter);
> > 		} else if (count == 1) {
> > 			if (copy_from_user(&sfilter, optval, sizeof(sfilter)))
> > 				return -EFAULT;
> > 		}
> >
> > 		lock_sock(sk);
>
> Marc