Re: openbsd ioctl fix (in6.c)
On Tue, Oct 01, 2013 at 10:25:45AM +0200, Martin Pieuchot wrote: On 30/09/13(Mon) 14:17, Loganaden Velvindron wrote: On Mon, Sep 30, 2013 at 10:51:47PM +0200, Alexander Bluhm wrote: On Wed, Sep 18, 2013 at 12:01:10AM -0700, Loganaden Velvindron wrote: Index: in6.c === RCS file: /cvs/src/sys/netinet6/in6.c,v retrieving revision 1.118 diff -u -p -r1.118 in6.c --- in6.c 26 Aug 2013 07:15:58 - 1.118 +++ in6.c 18 Sep 2013 06:54:13 - @@ -426,8 +426,11 @@ in6_control(struct socket *so, u_long cm sa6 = ifr-ifr_addr; break; case SIOCSIFADDR: + case SIOCSIFDSTADDR: + case SIOCSIFBRDADDR: + case SIOCSIFNETMASK: /* -* Do not pass this ioctl to driver handler since it is not +* Do not pass those ioctl to driver handler since they are not * properly setup. Instead just error out. */ return (EOPNOTSUPP); This diff uses spaces instead of tabs. Please use tabs to make diffs apply cleanly. The errno EAFNOSUPPORT Address family not supported by protocol family is more specific for that error, at least if_ppp and if_sl use that. Fixed style issues: Index: netinet6/in6.c === RCS file: /cvs/src/sys/netinet6/in6.c,v retrieving revision 1.118 diff -u -p -r1.118 in6.c --- netinet6/in6.c 26 Aug 2013 07:15:58 - 1.118 +++ netinet6/in6.c 30 Sep 2013 21:14:43 - @@ -426,8 +426,11 @@ in6_control(struct socket *so, u_long cm sa6 = ifr-ifr_addr; break; case SIOCSIFADDR: + case SIOCSIFDSTADDR: + case SIOCSIFBRDADDR: + case SIOCSIFNETMASK: /* -* Do not pass this ioctl to driver handler since it is not +* Do not pass those ioctl to driver handler since they are not * properly setup. Instead just error out. */ return (EOPNOTSUPP); anyway, code is correct, OK bluhm@ Committed, thanks. Can you please elaborate a bit concerning the cleanup for if_tun.c that mpi@ mentioned ? Well what could be done is to remove the SIOCSIFBRDADDR case because it is never reached, diff below. Index: net/if_tun.c === RCS file: /home/ncvs/src/sys/net/if_tun.c,v retrieving revision 1.115 diff -u -p -r1.115 if_tun.c --- net/if_tun.c 25 May 2013 10:05:52 - 1.115 +++ net/if_tun.c 10 Sep 2013 12:54:41 - @@ -493,10 +493,6 @@ tun_ioctl(struct ifnet *ifp, u_long cmd, tuninit(tp); TUNDEBUG((%s: destination address set\n, ifp-if_xname)); break; - case SIOCSIFBRDADDR: - tuninit(tp); - TUNDEBUG((%s: broadcast address set\n, ifp-if_xname)); - break; case SIOCSIFMTU: if (ifr-ifr_mtu ETHERMIN || ifr-ifr_mtu TUNMRU) error = EINVAL; OK. -- :wq Claudio
Re: openbsd ioctl fix (in6.c)
On 30/09/13(Mon) 14:17, Loganaden Velvindron wrote: On Mon, Sep 30, 2013 at 10:51:47PM +0200, Alexander Bluhm wrote: On Wed, Sep 18, 2013 at 12:01:10AM -0700, Loganaden Velvindron wrote: Index: in6.c === RCS file: /cvs/src/sys/netinet6/in6.c,v retrieving revision 1.118 diff -u -p -r1.118 in6.c --- in6.c 26 Aug 2013 07:15:58 - 1.118 +++ in6.c 18 Sep 2013 06:54:13 - @@ -426,8 +426,11 @@ in6_control(struct socket *so, u_long cm sa6 = ifr-ifr_addr; break; case SIOCSIFADDR: + case SIOCSIFDSTADDR: + case SIOCSIFBRDADDR: + case SIOCSIFNETMASK: /* -* Do not pass this ioctl to driver handler since it is not +* Do not pass those ioctl to driver handler since they are not * properly setup. Instead just error out. */ return (EOPNOTSUPP); This diff uses spaces instead of tabs. Please use tabs to make diffs apply cleanly. The errno EAFNOSUPPORT Address family not supported by protocol family is more specific for that error, at least if_ppp and if_sl use that. Fixed style issues: Index: netinet6/in6.c === RCS file: /cvs/src/sys/netinet6/in6.c,v retrieving revision 1.118 diff -u -p -r1.118 in6.c --- netinet6/in6.c26 Aug 2013 07:15:58 - 1.118 +++ netinet6/in6.c30 Sep 2013 21:14:43 - @@ -426,8 +426,11 @@ in6_control(struct socket *so, u_long cm sa6 = ifr-ifr_addr; break; case SIOCSIFADDR: + case SIOCSIFDSTADDR: + case SIOCSIFBRDADDR: + case SIOCSIFNETMASK: /* - * Do not pass this ioctl to driver handler since it is not + * Do not pass those ioctl to driver handler since they are not * properly setup. Instead just error out. */ return (EOPNOTSUPP); anyway, code is correct, OK bluhm@ Committed, thanks. Can you please elaborate a bit concerning the cleanup for if_tun.c that mpi@ mentioned ? Well what could be done is to remove the SIOCSIFBRDADDR case because it is never reached, diff below. Index: net/if_tun.c === RCS file: /home/ncvs/src/sys/net/if_tun.c,v retrieving revision 1.115 diff -u -p -r1.115 if_tun.c --- net/if_tun.c25 May 2013 10:05:52 - 1.115 +++ net/if_tun.c10 Sep 2013 12:54:41 - @@ -493,10 +493,6 @@ tun_ioctl(struct ifnet *ifp, u_long cmd, tuninit(tp); TUNDEBUG((%s: destination address set\n, ifp-if_xname)); break; - case SIOCSIFBRDADDR: - tuninit(tp); - TUNDEBUG((%s: broadcast address set\n, ifp-if_xname)); - break; case SIOCSIFMTU: if (ifr-ifr_mtu ETHERMIN || ifr-ifr_mtu TUNMRU) error = EINVAL;
Re: openbsd ioctl fix (in6.c)
On Wed, Sep 18, 2013 at 12:01:10AM -0700, Loganaden Velvindron wrote: Index: in6.c === RCS file: /cvs/src/sys/netinet6/in6.c,v retrieving revision 1.118 diff -u -p -r1.118 in6.c --- in6.c 26 Aug 2013 07:15:58 - 1.118 +++ in6.c 18 Sep 2013 06:54:13 - @@ -426,8 +426,11 @@ in6_control(struct socket *so, u_long cm sa6 = ifr-ifr_addr; break; case SIOCSIFADDR: + case SIOCSIFDSTADDR: + case SIOCSIFBRDADDR: + case SIOCSIFNETMASK: /* -* Do not pass this ioctl to driver handler since it is not +* Do not pass those ioctl to driver handler since they are not * properly setup. Instead just error out. */ return (EOPNOTSUPP); This diff uses spaces instead of tabs. Please use tabs to make diffs apply cleanly. The errno EAFNOSUPPORT Address family not supported by protocol family is more specific for that error, at least if_ppp and if_sl use that. anyway, code is correct, OK bluhm@
Re: openbsd ioctl fix (in6.c)
On Mon, Sep 30, 2013 at 10:51:47PM +0200, Alexander Bluhm wrote: On Wed, Sep 18, 2013 at 12:01:10AM -0700, Loganaden Velvindron wrote: Index: in6.c === RCS file: /cvs/src/sys/netinet6/in6.c,v retrieving revision 1.118 diff -u -p -r1.118 in6.c --- in6.c 26 Aug 2013 07:15:58 - 1.118 +++ in6.c 18 Sep 2013 06:54:13 - @@ -426,8 +426,11 @@ in6_control(struct socket *so, u_long cm sa6 = ifr-ifr_addr; break; case SIOCSIFADDR: + case SIOCSIFDSTADDR: + case SIOCSIFBRDADDR: + case SIOCSIFNETMASK: /* -* Do not pass this ioctl to driver handler since it is not +* Do not pass those ioctl to driver handler since they are not * properly setup. Instead just error out. */ return (EOPNOTSUPP); This diff uses spaces instead of tabs. Please use tabs to make diffs apply cleanly. The errno EAFNOSUPPORT Address family not supported by protocol family is more specific for that error, at least if_ppp and if_sl use that. Fixed style issues: Index: netinet6/in6.c === RCS file: /cvs/src/sys/netinet6/in6.c,v retrieving revision 1.118 diff -u -p -r1.118 in6.c --- netinet6/in6.c 26 Aug 2013 07:15:58 - 1.118 +++ netinet6/in6.c 30 Sep 2013 21:14:43 - @@ -426,8 +426,11 @@ in6_control(struct socket *so, u_long cm sa6 = ifr-ifr_addr; break; case SIOCSIFADDR: + case SIOCSIFDSTADDR: + case SIOCSIFBRDADDR: + case SIOCSIFNETMASK: /* -* Do not pass this ioctl to driver handler since it is not +* Do not pass those ioctl to driver handler since they are not * properly setup. Instead just error out. */ return (EOPNOTSUPP); anyway, code is correct, OK bluhm@ Can you please elaborate a bit concerning the cleanup for if_tun.c that mpi@ mentioned ?
Re: openbsd ioctl fix (in6.c)
ping ? On Wed, Sep 18, 2013 at 11:01 AM, Loganaden Velvindron lo...@elandsys.com wrote: On Tue, Aug 27, 2013 at 10:37:30AM +0200, Martin Pieuchot wrote: On 22/08/13(Thu) 23:31, Claudio Jeker wrote: On Wed, Aug 21, 2013 at 09:59:56AM -0700, Loganaden Velvindron wrote: I'm not sure if applies to OpenBSD as well, but NetBSD also disallowed SIOCSIFDSTADDR for ioctl. [...] Index: sys/netinet6/in6.c === RCS file: /cvs/src/sys/netinet6/in6.c,v retrieving revision 1.117 diff -u -p -r1.117 in6.c --- sys/netinet6/in6.c13 Aug 2013 05:52:25 - 1.117 +++ sys/netinet6/in6.c21 Aug 2013 15:50:00 - @@ -429,8 +429,9 @@ in6_control(struct socket *so, u_long cm sa6 = ifr-ifr_addr; break; case SIOCSIFADDR: + case SIOCSIFDSTADDR: /* - * Do not pass this ioctl to driver handler since it is not + * Do not pass those ioctl to driver handler since they are not * properly setup. Instead just error out. */ return (EOPNOTSUPP); Are any of our driver ioctl handlers handling SIOCSIFDSTADDR? I don't think so but I think this could be just extra safety and should therefor be commited. Only tun(4) seems to have some code for it, but I don't think it is correct. And if it is, we should probably use SIOCGIFDSTADDR_IN6 anyway. Maybe a small cleanup can be done? So I'm also in favor of this supplementary safety check, and I would even add SIOCSIFBRDADDR to this list. And SIOCSIFNETMASK as well to be safe ? Index: in6.c === RCS file: /cvs/src/sys/netinet6/in6.c,v retrieving revision 1.118 diff -u -p -r1.118 in6.c --- in6.c 26 Aug 2013 07:15:58 - 1.118 +++ in6.c 18 Sep 2013 06:54:13 - @@ -426,8 +426,11 @@ in6_control(struct socket *so, u_long cm sa6 = ifr-ifr_addr; break; case SIOCSIFADDR: + case SIOCSIFDSTADDR: + case SIOCSIFBRDADDR: + case SIOCSIFNETMASK: /* -* Do not pass this ioctl to driver handler since it is not +* Do not pass those ioctl to driver handler since they are not * properly setup. Instead just error out. */ return (EOPNOTSUPP); M. -- This message is strictly personal and the opinions expressed do not represent those of my employers, either past or present.
Re: openbsd ioctl fix (in6.c)
On Tue, Aug 27, 2013 at 10:37:30AM +0200, Martin Pieuchot wrote: On 22/08/13(Thu) 23:31, Claudio Jeker wrote: On Wed, Aug 21, 2013 at 09:59:56AM -0700, Loganaden Velvindron wrote: I'm not sure if applies to OpenBSD as well, but NetBSD also disallowed SIOCSIFDSTADDR for ioctl. [...] Index: sys/netinet6/in6.c === RCS file: /cvs/src/sys/netinet6/in6.c,v retrieving revision 1.117 diff -u -p -r1.117 in6.c --- sys/netinet6/in6.c13 Aug 2013 05:52:25 - 1.117 +++ sys/netinet6/in6.c21 Aug 2013 15:50:00 - @@ -429,8 +429,9 @@ in6_control(struct socket *so, u_long cm sa6 = ifr-ifr_addr; break; case SIOCSIFADDR: + case SIOCSIFDSTADDR: /* - * Do not pass this ioctl to driver handler since it is not + * Do not pass those ioctl to driver handler since they are not * properly setup. Instead just error out. */ return (EOPNOTSUPP); Are any of our driver ioctl handlers handling SIOCSIFDSTADDR? I don't think so but I think this could be just extra safety and should therefor be commited. Only tun(4) seems to have some code for it, but I don't think it is correct. And if it is, we should probably use SIOCGIFDSTADDR_IN6 anyway. Maybe a small cleanup can be done? So I'm also in favor of this supplementary safety check, and I would even add SIOCSIFBRDADDR to this list. And SIOCSIFNETMASK as well to be safe ? Index: in6.c === RCS file: /cvs/src/sys/netinet6/in6.c,v retrieving revision 1.118 diff -u -p -r1.118 in6.c --- in6.c 26 Aug 2013 07:15:58 - 1.118 +++ in6.c 18 Sep 2013 06:54:13 - @@ -426,8 +426,11 @@ in6_control(struct socket *so, u_long cm sa6 = ifr-ifr_addr; break; case SIOCSIFADDR: + case SIOCSIFDSTADDR: + case SIOCSIFBRDADDR: + case SIOCSIFNETMASK: /* -* Do not pass this ioctl to driver handler since it is not +* Do not pass those ioctl to driver handler since they are not * properly setup. Instead just error out. */ return (EOPNOTSUPP); M.
Re: openbsd ioctl fix (in6.c)
On 22/08/13(Thu) 23:31, Claudio Jeker wrote: On Wed, Aug 21, 2013 at 09:59:56AM -0700, Loganaden Velvindron wrote: I'm not sure if applies to OpenBSD as well, but NetBSD also disallowed SIOCSIFDSTADDR for ioctl. [...] Index: sys/netinet6/in6.c === RCS file: /cvs/src/sys/netinet6/in6.c,v retrieving revision 1.117 diff -u -p -r1.117 in6.c --- sys/netinet6/in6.c 13 Aug 2013 05:52:25 - 1.117 +++ sys/netinet6/in6.c 21 Aug 2013 15:50:00 - @@ -429,8 +429,9 @@ in6_control(struct socket *so, u_long cm sa6 = ifr-ifr_addr; break; case SIOCSIFADDR: + case SIOCSIFDSTADDR: /* -* Do not pass this ioctl to driver handler since it is not +* Do not pass those ioctl to driver handler since they are not * properly setup. Instead just error out. */ return (EOPNOTSUPP); Are any of our driver ioctl handlers handling SIOCSIFDSTADDR? I don't think so but I think this could be just extra safety and should therefor be commited. Only tun(4) seems to have some code for it, but I don't think it is correct. And if it is, we should probably use SIOCGIFDSTADDR_IN6 anyway. Maybe a small cleanup can be done? So I'm also in favor of this supplementary safety check, and I would even add SIOCSIFBRDADDR to this list. M.
Re: openbsd ioctl fix (in6.c)
On Tue, Aug 27, 2013 at 10:37:30AM +0200, Martin Pieuchot wrote: On 22/08/13(Thu) 23:31, Claudio Jeker wrote: On Wed, Aug 21, 2013 at 09:59:56AM -0700, Loganaden Velvindron wrote: I'm not sure if applies to OpenBSD as well, but NetBSD also disallowed SIOCSIFDSTADDR for ioctl. [...] Index: sys/netinet6/in6.c === RCS file: /cvs/src/sys/netinet6/in6.c,v retrieving revision 1.117 diff -u -p -r1.117 in6.c --- sys/netinet6/in6.c13 Aug 2013 05:52:25 - 1.117 +++ sys/netinet6/in6.c21 Aug 2013 15:50:00 - @@ -429,8 +429,9 @@ in6_control(struct socket *so, u_long cm sa6 = ifr-ifr_addr; break; case SIOCSIFADDR: + case SIOCSIFDSTADDR: /* - * Do not pass this ioctl to driver handler since it is not + * Do not pass those ioctl to driver handler since they are not * properly setup. Instead just error out. */ return (EOPNOTSUPP); Are any of our driver ioctl handlers handling SIOCSIFDSTADDR? I don't think so but I think this could be just extra safety and should therefor be commited. Only tun(4) seems to have some code for it, but I don't think it is correct. And if it is, we should probably use SIOCGIFDSTADDR_IN6 anyway. Maybe a small cleanup can be done? I haven't looked at tun(4) code yet, but I can give it a try :-) So I'm also in favor of this supplementary safety check, and I would even add SIOCSIFBRDADDR to this list. I'm updating the diff. M.
Re: openbsd ioctl fix (in6.c)
On Wed, Aug 21, 2013 at 09:59:56AM -0700, Loganaden Velvindron wrote:
I'm not sure if applies to OpenBSD as well, but NetBSD
also disallowed SIOCSIFDSTADDR for ioctl.
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/netinet6/in6.c?annotate=1.166only_with_tag=MAIN
1.2 itojun374:switch (cmd) {
1.104 christos 375:/*
1.105 christos 376: * XXX: Fix me, once we fix SIOCSIFADDR,
SIOCIFDSTADDR, etc.
1.104 christos 377: */
378:case SIOCSIFADDR:
1.105 christos 379:case SIOCSIFDSTADDR:
1.129 cube 380: #ifdef SIOCSIFCONF_X25
1.106 christos 381:case SIOCSIFCONF_X25:
1.110 matt 382: #endif
1.104 christos 383:return EOPNOTSUPP;
If it's indeed the case in OpenBSD, here's a diff:
Index: sys/netinet6/in6.c
===
RCS file: /cvs/src/sys/netinet6/in6.c,v
retrieving revision 1.117
diff -u -p -r1.117 in6.c
--- sys/netinet6/in6.c13 Aug 2013 05:52:25 - 1.117
+++ sys/netinet6/in6.c21 Aug 2013 15:50:00 -
@@ -429,8 +429,9 @@ in6_control(struct socket *so, u_long cm
sa6 = ifr-ifr_addr;
break;
case SIOCSIFADDR:
+ case SIOCSIFDSTADDR:
/*
- * Do not pass this ioctl to driver handler since it is not
+ * Do not pass those ioctl to driver handler since they are not
* properly setup. Instead just error out.
*/
return (EOPNOTSUPP);
Are any of our driver ioctl handlers handling SIOCSIFDSTADDR? I don't
think so but I think this could be just extra safety and should therefor
be commited.
--
:wq Claudio
openbsd ioctl fix (in6.c)
I'm not sure if applies to OpenBSD as well, but NetBSD
also disallowed SIOCSIFDSTADDR for ioctl.
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/netinet6/in6.c?annotate=1.166only_with_tag=MAIN
1.2 itojun374:switch (cmd) {
1.104 christos 375:/*
1.105 christos 376: * XXX: Fix me, once we fix SIOCSIFADDR,
SIOCIFDSTADDR, etc.
1.104 christos 377: */
378:case SIOCSIFADDR:
1.105 christos 379:case SIOCSIFDSTADDR:
1.129 cube 380: #ifdef SIOCSIFCONF_X25
1.106 christos 381:case SIOCSIFCONF_X25:
1.110 matt 382: #endif
1.104 christos 383:return EOPNOTSUPP;
If it's indeed the case in OpenBSD, here's a diff:
Index: sys/netinet6/in6.c
===
RCS file: /cvs/src/sys/netinet6/in6.c,v
retrieving revision 1.117
diff -u -p -r1.117 in6.c
--- sys/netinet6/in6.c 13 Aug 2013 05:52:25 - 1.117
+++ sys/netinet6/in6.c 21 Aug 2013 15:50:00 -
@@ -429,8 +429,9 @@ in6_control(struct socket *so, u_long cm
sa6 = ifr-ifr_addr;
break;
case SIOCSIFADDR:
+ case SIOCSIFDSTADDR:
/*
-* Do not pass this ioctl to driver handler since it is not
+* Do not pass those ioctl to driver handler since they are not
* properly setup. Instead just error out.
*/
return (EOPNOTSUPP);
