On Sun, Apr 03, 2016 at 10:20:30PM +0200, Vincent Gross wrote:
> On 03/31/16 14:07, Alexander Bluhm wrote:
> > At this point inp has already been modified. So when we bail out
> > with EACCES here, we have a partially successful system call.
> >
> > Move the assignments
> > inp->inp_laddr6 = sin6->sin6_addr;
> > inp->inp_laddr = sin->sin_addr;
> > down after the return (EACCES).
> >
> > Looks like that return (error) was wrong before.
Please remove the comment. It is not a question wether we want BSD
behavior.
Some cases may return EADDRINUSE instead of EACCES now, but I think
this is not an issue.
OK bluhm@
> diff --git a/sys/netinet/in_pcb.c b/sys/netinet/in_pcb.c
> index 1ff0056..63b3357 100644
> --- a/sys/netinet/in_pcb.c
> +++ b/sys/netinet/in_pcb.c
> @@ -343,9 +343,22 @@ in_pcbbind(struct inpcb *inp, struct mbuf *nam, struct
> proc *p)
> }
> }
>
> - if (lport == 0)
> + if (lport == 0) {
> if ((error = in_pcbpickport(&lport, laddr, wild, inp, p)))
> return (error);
> + } else {
> + /*
> + * Question: Do we wish to continue the Berkeley
> + * tradition of ports < IPPORT_RESERVED be only for
> + * root?
> + * Answer: For now yes, but IMHO, it should be REMOVED!
> + * OUCH: One other thing, is there no better way of
> + * finding a process for a socket instead of using
> + * curproc? (Marked with BSD's {in,}famous XXX ?
> + */
> + if (ntohs(lport) < IPPORT_RESERVED && (error = suser(p, 0)))
> + return (EACCES);
> + }
> if (nam) {
> switch (sotopf(so)) {
> #ifdef INET6
> @@ -371,7 +384,6 @@ in_pcbaddrisavail(struct inpcb *inp, struct sockaddr_in
> *sin, int wild,
> struct inpcbtable *table = inp->inp_table;
> u_int16_t lport = sin->sin_port;
> int reuseport = (so->so_options & SO_REUSEPORT);
> - int error;
>
> if (IN_MULTICAST(sin->sin_addr.s_addr)) {
> /*
> @@ -411,10 +423,6 @@ in_pcbaddrisavail(struct inpcb *inp, struct sockaddr_in
> *sin, int wild,
> if (lport) {
> struct inpcb *t;
>
> - /* GROSS */
> - if (ntohs(lport) < IPPORT_RESERVED &&
> - (error = suser(p, 0)))
> - return (EACCES);
> if (so->so_euid) {
> t = in_pcblookup(table, &zeroin_addr, 0,
> &sin->sin_addr, lport, INPLOOKUP_WILDCARD,
> diff --git a/sys/netinet6/in6_pcb.c b/sys/netinet6/in6_pcb.c
> index 4fde210..c11b936 100644
> --- a/sys/netinet6/in6_pcb.c
> +++ b/sys/netinet6/in6_pcb.c
> @@ -158,7 +158,6 @@ in6_pcbaddrisavail(struct inpcb *inp, struct sockaddr_in6
> *sin6, int wild,
> struct inpcbtable *table = inp->inp_table;
> u_short lport = sin6->sin6_port;
> int reuseport = (so->so_options & SO_REUSEPORT);
> - int error;
>
> wild |= INPLOOKUP_IPV6;
> /* KAME hack: embed scopeid */
> @@ -217,17 +216,6 @@ in6_pcbaddrisavail(struct inpcb *inp, struct
> sockaddr_in6 *sin6, int wild,
> if (lport) {
> struct inpcb *t;
>
> - /*
> - * Question: Do we wish to continue the Berkeley
> - * tradition of ports < IPPORT_RESERVED be only for
> - * root?
> - * Answer: For now yes, but IMHO, it should be REMOVED!
> - * OUCH: One other thing, is there no better way of
> - * finding a process for a socket instead of using
> - * curproc? (Marked with BSD's {in,}famous XXX ?
> - */
> - if (ntohs(lport) < IPPORT_RESERVED && (error = suser(p, 0)))
> - return error;
> if (so->so_euid) {
> t = in_pcblookup(table,
> (struct in_addr *)&zeroin6_addr, 0,
