Re: sosetstate/soclrstate
On Tue, Jan 09, 2018 at 11:23:32AM +0100, Martin Pieuchot wrote:
> --- netinet6/ip6_output.c 1 Sep 2017 15:05:31 - 1.232
> +++ netinet6/ip6_output.c 9 Jan 2018 10:16:10 -
> @@ -1033,12 +1033,12 @@ int
> ip6_ctloutput(int op, struct socket *so, int level, int optname,
> struct mbuf *m)
> {
> - int privileged, optdatalen, uproto;
> + int optdatalen, uproto;
> void *optdata;
> struct inpcb *inp = sotoinpcb(so);
> int error, optval;
> struct proc *p = curproc; /* For IPSec and rdomain */
> - u_int rtid = 0;
> + u_int privileged, rtid = 0;
>
> error = optval = 0;
>
"privileged" is passed down as an boolean signed int. So it does
not matter where it is converted.
anyway OK bluhm@
Re: sosetstate/soclrstate
On 09/01/18(Tue) 00:42, Alexander Bluhm wrote:
> On Mon, Jan 08, 2018 at 03:50:14PM +0100, Martin Pieuchot wrote:
> > ok?
>
> OK bluhm@ with a few remarks.
visa@ pointed out that since all the writes are done under the socket
lock it is enough to use an aligned int32 variable. So here's an
updated diff that address your comments.
Index: kern/kern_pledge.c
===
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.227
diff -u -p -r1.227 kern_pledge.c
--- kern/kern_pledge.c 8 Jan 2018 11:54:28 - 1.227
+++ kern/kern_pledge.c 9 Jan 2018 10:17:28 -
@@ -1337,7 +1337,7 @@ pledge_sockopt(struct proc *p, int set,
}
int
-pledge_socket(struct proc *p, int domain, int state)
+pledge_socket(struct proc *p, int domain, unsigned int state)
{
if (! ISSET(p->p_p->ps_flags, PS_PLEDGE))
return 0;
Index: kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.213
diff -u -p -r1.213 uipc_socket.c
--- kern/uipc_socket.c 2 Jan 2018 12:54:07 - 1.213
+++ kern/uipc_socket.c 9 Jan 2018 10:14:21 -
@@ -1248,7 +1248,7 @@ somove(struct socket *so, int wait)
u_long len, off, oobmark;
long space;
int error = 0, maxreached = 0;
- shortstate;
+ unsigned int state;
soassertlocked(so);
Index: kern/uipc_syscalls.c
===
RCS file: /cvs/src/sys/kern/uipc_syscalls.c,v
retrieving revision 1.161
diff -u -p -r1.161 uipc_syscalls.c
--- kern/uipc_syscalls.c2 Jan 2018 06:38:45 - 1.161
+++ kern/uipc_syscalls.c9 Jan 2018 10:17:39 -
@@ -83,7 +83,8 @@ sys_socket(struct proc *p, void *v, regi
struct file *fp;
int type = SCARG(uap, type);
int domain = SCARG(uap, domain);
- int fd, error, ss = 0;
+ int fd, error;
+ unsigned int ss = 0;
if ((type & SOCK_DNS) && !(domain == AF_INET || domain == AF_INET6))
return (EINVAL);
Index: netinet/tcp_usrreq.c
===
RCS file: /cvs/src/sys/netinet/tcp_usrreq.c,v
retrieving revision 1.162
diff -u -p -r1.162 tcp_usrreq.c
--- netinet/tcp_usrreq.c1 Dec 2017 10:33:33 - 1.162
+++ netinet/tcp_usrreq.c9 Jan 2018 10:15:30 -
@@ -583,7 +583,7 @@ tcp_attach(struct socket *so, int proto)
inp = sotoinpcb(so);
tp = tcp_newtcpcb(inp);
if (tp == NULL) {
- int nofd = so->so_state & SS_NOFDREF; /* XXX */
+ unsigned int nofd = so->so_state & SS_NOFDREF; /* XXX */
so->so_state &= ~SS_NOFDREF;/* don't free the socket yet */
in_pcbdetach(inp);
Index: netinet6/ip6_output.c
===
RCS file: /cvs/src/sys/netinet6/ip6_output.c,v
retrieving revision 1.232
diff -u -p -r1.232 ip6_output.c
--- netinet6/ip6_output.c 1 Sep 2017 15:05:31 - 1.232
+++ netinet6/ip6_output.c 9 Jan 2018 10:16:10 -
@@ -1033,12 +1033,12 @@ int
ip6_ctloutput(int op, struct socket *so, int level, int optname,
struct mbuf *m)
{
- int privileged, optdatalen, uproto;
+ int optdatalen, uproto;
void *optdata;
struct inpcb *inp = sotoinpcb(so);
int error, optval;
struct proc *p = curproc; /* For IPSec and rdomain */
- u_int rtid = 0;
+ u_int privileged, rtid = 0;
error = optval = 0;
Index: sys/pledge.h
===
RCS file: /cvs/src/sys/sys/pledge.h,v
retrieving revision 1.33
diff -u -p -r1.33 pledge.h
--- sys/pledge.h12 Dec 2017 01:12:34 - 1.33
+++ sys/pledge.h9 Jan 2018 10:17:45 -
@@ -126,7 +126,7 @@ int pledge_chown(struct proc *p, uid_t,
intpledge_adjtime(struct proc *p, const void *v);
intpledge_sendit(struct proc *p, const void *to);
intpledge_sockopt(struct proc *p, int set, int level, int optname);
-intpledge_socket(struct proc *p, int domain, int state);
+intpledge_socket(struct proc *p, int domain, unsigned int state);
intpledge_ioctl(struct proc *p, long com, struct file *);
intpledge_ioctl_drm(struct proc *p, long com, dev_t device);
intpledge_ioctl_vmm(struct proc *p, long com);
Index: sys/socketvar.h
===
RCS file: /cvs/src/sys/sys/socketvar.h,v
retrieving revision 1.81
diff -u -p -r1.81 socketvar.h
--- sys/socketvar.h 2 Jan 2018 12:54:07 - 1.81
+++ sys/socketvar.h 9 Jan 2018 10:08:53 -
@@ -51,12 +51,12 @@ TAILQ_HEAD(soqhead, socket);
* private data and error information.
*/
struct socket {
+ const struct protosw *so_proto; /* pro
Re: sosetstate/soclrstate
On Mon, Jan 08, 2018 at 03:50:14PM +0100, Martin Pieuchot wrote:
> ok?
OK bluhm@ with a few remarks.
> @@ -1422,12 +1422,12 @@ somove(struct socket *so, int wait)
>
> /* Receive buffer did shrink by len bytes, adjust oob. */
> state = so->so_state;
> - so->so_state &= ~SS_RCVATMARK;
> + soclrstate(so, SS_RCVATMARK);
> oobmark = so->so_oobmark;
> so->so_oobmark = oobmark > len ? oobmark - len : 0;
> if (oobmark) {
Can we make the "state" also u_int?
> @@ -198,7 +198,7 @@ sonewconn(struct socket *head, int conns
> if (connstatus) {
> sorwakeup(head);
> wakeup(&head->so_timeo);
> - so->so_state |= connstatus;
> + sosetstate(so, connstatus);
> }
> return (so);
> }
"connstatus" should be u_int.
> @@ -112,8 +112,8 @@ sys_socket(struct proc *p, void *v, regi
> fdpunlock(fdp);
> } else {
> if (type & SOCK_NONBLOCK)
> - so->so_state |= SS_NBIO;
> - so->so_state |= ss;
> + sosetstate(so, SS_NBIO);
> + sosetstate(so, ss);
> fp->f_data = so;
> FILE_SET_MATURE(fp, p);
> *retval = fd;
"ss" should be u_int for consistency. And pledge_socket() still
takes the state as signed int.
> @@ -181,7 +181,7 @@ fifo_open(void *v)
> goto bad;
> }
> if (fip->fi_writers == 1) {
> - rso->so_state &= ~(SS_CANTRCVMORE|SS_ISDISCONNECTED);
> + soclrstate(rso, (SS_CANTRCVMORE|SS_ISDISCONNECTED));
> if (fip->fi_readers > 0)
> wakeup(&fip->fi_readers);
> }
There are more () than needed.
> @@ -585,9 +585,9 @@ tcp_attach(struct socket *so, int proto)
> if (tp == NULL) {
> int nofd = so->so_state & SS_NOFDREF; /* XXX */
>
> - so->so_state &= ~SS_NOFDREF;/* don't free the socket yet */
> + soclrstate(so, SS_NOFDREF); /* don't free the socket yet */
> in_pcbdetach(inp);
> - so->so_state |= nofd;
> + sosetstate(so, nofd);
> return (ENOBUFS);
> }
> tp->t_state = TCPS_CLOSED;
nofd should be u_int.
sosetstate/soclrstate
Diff below changes the type of `so_state' and `so_error' to int. It
also introduces two new wrappers to set & clear state bits atomically.
My goal is to prevent a second CPU from reading garbage when looking
at these two fields in filt_soread() and filt_sowrite(). The values
might be outdated though.
This should be the last requirement before unlocking the receiving path.
ok?
Index: kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.213
diff -u -p -r1.213 uipc_socket.c
--- kern/uipc_socket.c 2 Jan 2018 12:54:07 - 1.213
+++ kern/uipc_socket.c 8 Jan 2018 14:32:33 -
@@ -139,7 +139,7 @@ socreate(int dom, struct socket **aso, i
s = solock(so);
error = (*prp->pr_attach)(so, proto);
if (error) {
- so->so_state |= SS_NOFDREF;
+ sosetstate(so, SS_NOFDREF);
sofree(so);
sounlock(s);
return (error);
@@ -275,7 +275,7 @@ drop:
discard:
if (so->so_state & SS_NOFDREF)
panic("soclose NOFDREF: so %p, so_type %d", so, so->so_type);
- so->so_state |= SS_NOFDREF;
+ sosetstate(so, SS_NOFDREF);
sofree(so);
sounlock(s);
return (error);
@@ -299,7 +299,7 @@ soaccept(struct socket *so, struct mbuf
if ((so->so_state & SS_NOFDREF) == 0)
panic("soaccept !NOFDREF: so %p, so_type %d", so, so->so_type);
- so->so_state &= ~SS_NOFDREF;
+ soclrstate(so, SS_NOFDREF);
if ((so->so_state & SS_ISDISCONNECTED) == 0 ||
(so->so_proto->pr_flags & PR_ABRTACPTDIS) == 0)
error = (*so->so_proto->pr_usrreq)(so, PRU_ACCEPT, NULL,
@@ -425,7 +425,7 @@ sosend(struct socket *so, struct mbuf *a
restart:
if ((error = sblock(so, &so->so_snd, SBLOCKWAIT(flags))) != 0)
goto out;
- so->so_state |= SS_ISSENDING;
+ sosetstate(so, SS_ISSENDING);
do {
if (so->so_state & SS_CANTSENDMORE)
snderr(EPIPE);
@@ -455,7 +455,7 @@ restart:
snderr(EWOULDBLOCK);
sbunlock(so, &so->so_snd);
error = sbwait(so, &so->so_snd);
- so->so_state &= ~SS_ISSENDING;
+ soclrstate(so, SS_ISSENDING);
if (error)
goto out;
goto restart;
@@ -481,7 +481,7 @@ restart:
top->m_flags |= M_EOR;
}
if (resid == 0)
- so->so_state &= ~SS_ISSENDING;
+ soclrstate(so, SS_ISSENDING);
if (top && so->so_options & SO_ZEROIZE)
top->m_flags |= M_ZEROIZE;
error = (*so->so_proto->pr_usrreq)(so,
@@ -496,7 +496,7 @@ restart:
} while (resid);
release:
- so->so_state &= ~SS_ISSENDING;
+ soclrstate(so, SS_ISSENDING);
sbunlock(so, &so->so_snd);
out:
sounlock(s);
@@ -857,7 +857,7 @@ dontblock:
panic("receive 3: so %p, so_type %d, m %p, m_type %d",
so, so->so_type, m, m->m_type);
#endif
- so->so_state &= ~SS_RCVATMARK;
+ soclrstate(so, SS_RCVATMARK);
len = uio->uio_resid;
if (so->so_oobmark && len > so->so_oobmark - offset)
len = so->so_oobmark - offset;
@@ -932,7 +932,7 @@ dontblock:
if ((flags & MSG_PEEK) == 0) {
so->so_oobmark -= len;
if (so->so_oobmark == 0) {
- so->so_state |= SS_RCVATMARK;
+ sosetstate(so, SS_RCVATMARK);
break;
}
} else {
@@ -1292,7 +1292,7 @@ somove(struct socket *so, int wait)
goto release;
len = space;
}
- sosp->so_state |= SS_ISSENDING;
+ sosetstate(sosp, SS_ISSENDING);
SBLASTRECORDCHK(&so->so_rcv, "somove 1");
SBLASTMBUFCHK(&so->so_rcv, "somove 1");
@@ -1422,12 +1422,12 @@ somove(struct socket *so, int wait)
/* Receive buffer did shrink by len bytes, adjust oob. */
state = so->so_state;
- so->so_state &= ~SS_RCVATMARK;
+ soclrstate(so, SS_RCVATMARK);
oobmark = so->so_oobmark;
so->so_oobmark = oobmark > len ? oobmark - len : 0;
if (oobmark) {
if (oobmark == len)
- so->so_state |= SS_RCVATMARK;
+ sosetstate(so, SS_RCVATMARK);
if (oobmark >= len)
oobmark = 0;
}
@@ -1485,7 +1485,7
