On 03/31/16 14:07, Alexander Bluhm wrote:
> On Wed, Mar 30, 2016 at 10:44:14PM +0200, Vincent Gross wrote:
>> This diff moves the "are we binding to a privileged port while not being
>> root ?"
>> check from in(6)_pcbaddrisavail() to in_pcbbind().
>
>> --- sys/netinet/in_pcb.c 26 Mar 2016 21:56:04 -0000 1.198
>> +++ sys/netinet/in_pcb.c 30 Mar 2016 20:33:00 -0000
>> @@ -341,9 +341,14 @@ in_pcbbind(struct inpcb *inp, struct mbu
>> }
>> }
>>
>> - if (lport == 0)
>> + if (lport == 0) {
>> if ((error = in_pcbpickport(&lport, wild, inp, p)))
>> return (error);
>> + } else {
>> + if (ntohs(lport) < IPPORT_RESERVED &&
>> + (error = suser(p, 0)))
>> + return (EACCES);
>> + }
>> inp->inp_lport = lport;
>
> 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.
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,
