On Fri, Nov 09, 2018 at 06:57:16PM +0100, Claudio Jeker wrote:
> On Fri, Nov 09, 2018 at 06:09:34PM +0100, Jan Klemkow wrote:
> > I printed the code path below to make it easier to review the diff.
> > While I was following the code path again, I found an inconsistency
> > between udp_output() and upd6_output(). Just upd_output() turns EACCES
> > to EHOSTUNREACH, udp6_output does not. The Diff below fixes udp6_output
> > and rounds up the sendto(2) manpage.
> >
> > Reviewer guide from sendto(2) to EHOSTUNREACH:
> >
> > sys_sendto() -> sendit() -> sosend() ---+
> > |
> > +--------------------------------------+
> > |
> > +-> udp_usrreq() -> udp_output():
> > | ...
> > | error = ip_output(m, inp->inp_options, &inp->inp_route,
> > | (inp->inp_socket->so_options & SO_BROADCAST), inp->inp_moptions,
> > | inp, ipsecflowinfo);
> > | if (error == EACCES) /* translate pf(4) error for userland */
> > | error = EHOSTUNREACH;
> > | ...
> > |
> > +-> udp_usrreq() -> udp6_output():
> > | ...
> > | error = ip6_output(m, optp, &in6p->inp_route6,
> > | flags, in6p->inp_moptions6, in6p);
> > | if (error == EACCES) /* translate pf(4) error for userland */
> > | error = EHOSTUNREACH;
> > | ...
> > |
> > +-> tcp_usrreq() -> tcp_output():
> > ...
> > error = ip_output(m, tp->t_inpcb->inp_options,
> > tp->t_inpcb->inp_route,
> > ip_mtudisc ? IP_MTUDISC : 0), NULL, tp->t_inpcb, 0);
> > ...
> > error = ip6_output(m, tp->t_inpcb->inp_outputopts6,
> > &tp->t_inpcb->inp_route6,
> > 0, NULL, tp->t_inpcb);
> > ...
> > if (error == EACCES) /* translate pf(4) error for userland */
> > error = EHOSTUNREACH;
> > ...
> >
>
> I really don't like this translation and therefor change of semantics of
> an old documented errno code.
>
> If I remember correctly this was added because of the fear that EACCES was
> not properly handled when used with write(2) since it is not documented
> there but neither is EHOSTUNREACH.
>
> I would prefer to remove this errno translation magic for pf(4) in the
> midlayer of the network code. As shown it is always prone for errors.
Hi Claudio,
Perfect, I also think its more intuitive to get a "permission denied"
in case of a pf(4) block then a "Host is unreachable". The diff below
corrects kernel and extents the manpage for pf(4) blocks.
Thanks,
Jan
Index: sys/netinet/tcp_output.c
===================================================================
RCS file: /cvs/src/sys/netinet/tcp_output.c,v
retrieving revision 1.127
diff -u -p -r1.127 tcp_output.c
--- sys/netinet/tcp_output.c 9 Nov 2018 14:14:31 -0000 1.127
+++ sys/netinet/tcp_output.c 9 Nov 2018 18:53:02 -0000
@@ -1084,8 +1084,6 @@ out:
tcp_mtudisc(tp->t_inpcb, -1);
return (0);
}
- if (error == EACCES) /* translate pf(4) error for userland */
- error = EHOSTUNREACH;
if ((error == EHOSTUNREACH || error == ENETDOWN) &&
TCPS_HAVERCVDSYN(tp->t_state)) {
tp->t_softerror = error;
Index: sys/netinet/udp_usrreq.c
===================================================================
RCS file: /cvs/src/sys/netinet/udp_usrreq.c,v
retrieving revision 1.253
diff -u -p -r1.253 udp_usrreq.c
--- sys/netinet/udp_usrreq.c 4 Oct 2018 17:33:41 -0000 1.253
+++ sys/netinet/udp_usrreq.c 9 Nov 2018 18:52:08 -0000
@@ -1004,8 +1004,6 @@ udp_output(struct inpcb *inp, struct mbu
error = ip_output(m, inp->inp_options, &inp->inp_route,
(inp->inp_socket->so_options & SO_BROADCAST), inp->inp_moptions,
inp, ipsecflowinfo);
- if (error == EACCES) /* translate pf(4) error for userland */
- error = EHOSTUNREACH;
bail:
m_freem(control);
Index: lib/libc/sys//send.2
===================================================================
RCS file: /cvs/src/lib/libc/sys/send.2,v
retrieving revision 1.32
diff -u -p -r1.32 send.2
--- lib/libc/sys//send.2 5 Oct 2017 12:30:16 -0000 1.32
+++ lib/libc/sys//send.2 9 Nov 2018 19:06:47 -0000
@@ -162,7 +162,9 @@ The output queue for a network interface
This generally indicates that the interface has stopped sending,
but may be caused by transient congestion.
.It Bq Er EACCES
-The
+The connection was blocked by
+.Xr pf 4 ,
+or
.Dv SO_BROADCAST
option is not set on the socket, and a broadcast address
was given as the destination.
