Re: [PATCH] rose: af_rose: avoid overflows in rose_setsockopt()

[Eric Dumazet]

On 6/4/19 5:11 AM, Young Xiao wrote:
> Check setsockopt arguments to avoid overflows and return -EINVAL for
> too large arguments.
> 
> See commit 32288eb4d940 ("netrom: avoid overflows in nr_setsockopt()")
> for details.
> 
> Signed-off-by: Young Xiao <92siuy...@gmail.com>
> ---
>  net/rose/af_rose.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c
> index e274bc6..af831ee9 100644
> --- a/net/rose/af_rose.c
> +++ b/net/rose/af_rose.c
> @@ -372,15 +372,15 @@ static int rose_setsockopt(struct socket *sock, int 
> level, int optname,
>  {
>   struct sock *sk = sock->sk;
>   struct rose_sock *rose = rose_sk(sk);
> - int opt;
> + unsigned long opt;
>  
>   if (level != SOL_ROSE)
>   return -ENOPROTOOPT;
>  
> - if (optlen < sizeof(int))
> + if (optlen < sizeof(unsigned int))
>   return -EINVAL;
>  
> - if (get_user(opt, (int __user *)optval))
> + if (get_user(opt, (unsigned int __user *)optval))
>   return -EFAULT;
>  
>   switch (optname) {
> @@ -389,31 +389,31 @@ static int rose_setsockopt(struct socket *sock, int 
> level, int optname,
>   return 0;
>  
>   case ROSE_T1:
> - if (opt < 1)
> + if (opt < 1 || opt > ULONG_MAX / HZ)
>   return -EINVAL;
>   rose->t1 = opt * HZ;
>   return 0;
>  
>   case ROSE_T2:
> - if (opt < 1)
> + if (opt < 1 || opt > ULONG_MAX / HZ)
>   return -EINVAL;
>   rose->t2 = opt * HZ;
>   return 0;
>  
>   case ROSE_T3:
> - if (opt < 1)
> + if (opt < 1 || opt > ULONG_MAX / HZ)
>   return -EINVAL;
>   rose->t3 = opt * HZ;
>   return 0;
>  
>   case ROSE_HOLDBACK:
> - if (opt < 1)
> + if (opt < 1 || opt > ULONG_MAX / HZ)
>   return -EINVAL;
>   rose->hb = opt * HZ;
>   return 0;
>  
>   case ROSE_IDLE:
> - if (opt < 0)
> + if (opt < 0 || opt > ULONG_MAX / HZ)

Buggy check.

>   return -EINVAL;
>   rose->idle = opt * 60 * HZ;
>   return 0;
> 

Re: [PATCH] net: ax25: replace bh_lock_sock with lock_sock_fast in ax25_rt_autobind

[Eric Dumazet]

On 04/03/2019 05:51 PM, Li RongQing wrote:
> ax25_rt_autobind is always called in user context, so lock_sock_fast
> should be used, and bh_lock_sock should only be used in BH context
> this replacement fixes a possible deadlock
> 


> 
> Reported-by: syzbot+e350b81e95a6a214d...@syzkaller.appspotmail.com
> Signed-off-by: Li RongQing 
> ---
>  net/ax25/ax25_route.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ax25/ax25_route.c b/net/ax25/ax25_route.c
> index 66f74c85cf6b..28e21259ff08 100644
> --- a/net/ax25/ax25_route.c
> +++ b/net/ax25/ax25_route.c
> @@ -429,9 +429,11 @@ int ax25_rt_autobind(ax25_cb *ax25, ax25_address *addr)
>   }
>  
>   if (ax25->sk != NULL) {
> - bh_lock_sock(ax25->sk);
> + bool slow;
> +
> + slow = lock_sock_fast(ax25->sk);
>   sock_reset_flag(ax25->sk, SOCK_ZAPPED);
> - bh_unlock_sock(ax25->sk);
> + unlock_sock_fast(ax25->sk, slow);
>   }
>  
>  put:
> 

How have you tested this patch exactly ?