Re: [PATCH net] dn_getsockoptdecnet: move nf_{get/set}sockopt outside sock lock

2018-02-16 Thread David Miller
From: Paolo Abeni 
Date: Thu, 15 Feb 2018 16:59:49 +0100

> After commit 3f34cfae1238 ("netfilter: on sockopt() acquire sock lock
> only in the required scope"), the caller of nf_{get/set}sockopt() must
> not hold any lock, but, in such changeset, I forgot to cope with DECnet.
> 
> This commit addresses the issue moving the nf call outside the lock,
> in the dn_{get,set}sockopt() with the same schema currently used by
> ipv4 and ipv6. Also moves the unhandled sockopts of the end of the main
> switch statements, to improve code readability.
> 
> Reported-by: Petr Vandrovec 
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=198791#c2
> Fixes: 3f34cfae1238 ("netfilter: on sockopt() acquire sock lock only in the 
> required scope")
> Signed-off-by: Paolo Abeni 

Applied, thank you.


[PATCH net] dn_getsockoptdecnet: move nf_{get/set}sockopt outside sock lock

2018-02-15 Thread Paolo Abeni
After commit 3f34cfae1238 ("netfilter: on sockopt() acquire sock lock
only in the required scope"), the caller of nf_{get/set}sockopt() must
not hold any lock, but, in such changeset, I forgot to cope with DECnet.

This commit addresses the issue moving the nf call outside the lock,
in the dn_{get,set}sockopt() with the same schema currently used by
ipv4 and ipv6. Also moves the unhandled sockopts of the end of the main
switch statements, to improve code readability.

Reported-by: Petr Vandrovec 
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=198791#c2
Fixes: 3f34cfae1238 ("netfilter: on sockopt() acquire sock lock only in the 
required scope")
Signed-off-by: Paolo Abeni 
---
 net/decnet/af_decnet.c | 62 +++---
 1 file changed, 33 insertions(+), 29 deletions(-)

diff --git a/net/decnet/af_decnet.c b/net/decnet/af_decnet.c
index 91dd09f79808..791aff68af88 100644
--- a/net/decnet/af_decnet.c
+++ b/net/decnet/af_decnet.c
@@ -1338,6 +1338,12 @@ static int dn_setsockopt(struct socket *sock, int level, 
int optname, char __use
lock_sock(sk);
err = __dn_setsockopt(sock, level, optname, optval, optlen, 0);
release_sock(sk);
+#ifdef CONFIG_NETFILTER
+   /* we need to exclude all possible ENOPROTOOPTs except default case */
+   if (err == -ENOPROTOOPT && optname != DSO_LINKINFO &&
+   optname != DSO_STREAM && optname != DSO_SEQPACKET)
+   err = nf_setsockopt(sk, PF_DECnet, optname, optval, optlen);
+#endif
 
return err;
 }
@@ -1445,15 +1451,6 @@ static int __dn_setsockopt(struct socket *sock, int 
level,int optname, char __us
dn_nsp_send_disc(sk, 0x38, 0, sk->sk_allocation);
break;
 
-   default:
-#ifdef CONFIG_NETFILTER
-   return nf_setsockopt(sk, PF_DECnet, optname, optval, optlen);
-#endif
-   case DSO_LINKINFO:
-   case DSO_STREAM:
-   case DSO_SEQPACKET:
-   return -ENOPROTOOPT;
-
case DSO_MAXWINDOW:
if (optlen != sizeof(unsigned long))
return -EINVAL;
@@ -1501,6 +1498,12 @@ static int __dn_setsockopt(struct socket *sock, int 
level,int optname, char __us
return -EINVAL;
scp->info_loc = u.info;
break;
+
+   case DSO_LINKINFO:
+   case DSO_STREAM:
+   case DSO_SEQPACKET:
+   default:
+   return -ENOPROTOOPT;
}
 
return 0;
@@ -1514,6 +1517,20 @@ static int dn_getsockopt(struct socket *sock, int level, 
int optname, char __use
lock_sock(sk);
err = __dn_getsockopt(sock, level, optname, optval, optlen, 0);
release_sock(sk);
+#ifdef CONFIG_NETFILTER
+   if (err == -ENOPROTOOPT && optname != DSO_STREAM &&
+   optname != DSO_SEQPACKET && optname != DSO_CONACCEPT &&
+   optname != DSO_CONREJECT) {
+   int len;
+
+   if (get_user(len, optlen))
+   return -EFAULT;
+
+   err = nf_getsockopt(sk, PF_DECnet, optname, optval, &len);
+   if (err >= 0)
+   err = put_user(len, optlen);
+   }
+#endif
 
return err;
 }
@@ -1579,26 +1596,6 @@ static int __dn_getsockopt(struct socket *sock, int 
level,int optname, char __us
r_data = &link;
break;
 
-   default:
-#ifdef CONFIG_NETFILTER
-   {
-   int ret, len;
-
-   if (get_user(len, optlen))
-   return -EFAULT;
-
-   ret = nf_getsockopt(sk, PF_DECnet, optname, optval, &len);
-   if (ret >= 0)
-   ret = put_user(len, optlen);
-   return ret;
-   }
-#endif
-   case DSO_STREAM:
-   case DSO_SEQPACKET:
-   case DSO_CONACCEPT:
-   case DSO_CONREJECT:
-   return -ENOPROTOOPT;
-
case DSO_MAXWINDOW:
if (r_len > sizeof(unsigned long))
r_len = sizeof(unsigned long);
@@ -1630,6 +1627,13 @@ static int __dn_getsockopt(struct socket *sock, int 
level,int optname, char __us
r_len = sizeof(unsigned char);
r_data = &scp->info_rem;
break;
+
+   case DSO_STREAM:
+   case DSO_SEQPACKET:
+   case DSO_CONACCEPT:
+   case DSO_CONREJECT:
+   default:
+   return -ENOPROTOOPT;
}
 
if (r_data) {
-- 
2.14.3