Changeset View
Standalone View
sys/kern/uipc_usrreq.c
Show First 20 Lines • Show All 2,761 Lines • ▼ Show 20 Lines | |||||||||
} | } | ||||||||
/* | /* | ||||||||
* Synchronize against unp_gc, which can trip over data as we are freeing it. | * Synchronize against unp_gc, which can trip over data as we are freeing it. | ||||||||
*/ | */ | ||||||||
static void | static void | ||||||||
unp_dispose(struct socket *so) | unp_dispose(struct socket *so) | ||||||||
{ | { | ||||||||
struct sockbuf *sb = &so->so_rcv; | |||||||||
struct unpcb *unp; | struct unpcb *unp; | ||||||||
struct mbuf *m; | |||||||||
MPASS(!SOLISTENING(so)); | MPASS(!SOLISTENING(so)); | ||||||||
unp = sotounpcb(so); | unp = sotounpcb(so); | ||||||||
UNP_LINK_WLOCK(); | UNP_LINK_WLOCK(); | ||||||||
unp->unp_gcflag |= UNPGC_IGNORE_RIGHTS; | unp->unp_gcflag |= UNPGC_IGNORE_RIGHTS; | ||||||||
UNP_LINK_WUNLOCK(); | UNP_LINK_WUNLOCK(); | ||||||||
unp_dispose_mbuf(so->so_rcv.sb_mb); | |||||||||
/* | |||||||||
* Grab our special mbufs before calling sbrelease(). | |||||||||
*/ | |||||||||
SOCK_RECVBUF_LOCK(so); | |||||||||
m = sbcut_locked(sb, sb->sb_ccc); | |||||||||
markj: Isn't this the same as `m = sbcut_locked(sb, sb->sb_ccc);`? | |||||||||
Done Inline Actionssbflush_internal() has a comment that suggests a problem exists here. It could be outdated, though. glebius: sbflush_internal() has a comment that suggests a problem exists here. It could be outdated… | |||||||||
Not Done Inline ActionsI suspect it is outdated. Right now the code doesn't even panic in that case, unless INVARIANTS is on. markj: I suspect it is outdated. Right now the code doesn't even panic in that case, unless INVARIANTS… | |||||||||
KASSERT(sb->sb_ccc == 0 && sb->sb_mb == 0 && sb->sb_mbcnt == 0, | |||||||||
Not Done Inline Actions
markj: | |||||||||
("%s: ccc %u mb %p mbcnt %u", __func__, | |||||||||
sb->sb_ccc, (void *)sb->sb_mb, sb->sb_mbcnt)); | |||||||||
Not Done Inline Actionssbrelease_locked() asserts this already. markj: sbrelease_locked() asserts this already. | |||||||||
Done Inline Actions
glebius: > sbrelease_locked() asserts this already.
| |||||||||
Done Inline Actions
It does so after calling sbcut(). We want to make sure socket buffer is empty after sbcut done by us. glebius: > sbrelease_locked() asserts this already.
It does so after calling sbcut(). We want to make… | |||||||||
sbrelease_locked(sb, so); | |||||||||
SOCK_RECVBUF_UNLOCK(so); | |||||||||
if (SOCK_IO_RECV_OWNED(so)) | |||||||||
SOCK_IO_RECV_UNLOCK(so); | |||||||||
Not Done Inline ActionsSo we still hold the IO lock here, and unp_dispose_mbuf() can close arbitrary descriptors, so a lot of code is reachable from this point (i.e., most implementations of fo_close). How can we be certain that there are no LORs possible here? markj: So we still hold the IO lock here, and unp_dispose_mbuf() can close arbitrary descriptors, so a… | |||||||||
Done Inline ActionsGood concern. I will update the revision. glebius: Good concern. I will update the revision. | |||||||||
Not Done Inline ActionsActually, I don't see why we need to hold the io lock here at all. Can't it be dropped before calling pru_dispose? We marked the buffer with CANTRCVMORE already. Hmm, no, it seems that soreceive_generic() removes mbufs from the recv socket buffer after copying them out to userspace, and to do that it needs to drop the socket buffer mutex. That's surprising to me, to be honest. markj: Actually, I don't see why we need to hold the io lock here at all. Can't it be dropped before… | |||||||||
Done Inline ActionsOther reason is to protect against a race between shutdown(2) and listen(2). Not something to be expected from a sane application though. glebius: Other reason is to protect against a race between shutdown(2) and listen(2). Not something to… | |||||||||
Not Done Inline ActionsAn application may be sane and may also have file descriptor reuse bugs. :) markj: An application may be sane and may also have file descriptor reuse bugs. :) | |||||||||
unp_dispose_mbuf(m); | |||||||||
} | } | ||||||||
static void | static void | ||||||||
unp_scan(struct mbuf *m0, void (*op)(struct filedescent **, int)) | unp_scan(struct mbuf *m0, void (*op)(struct filedescent **, int)) | ||||||||
{ | { | ||||||||
struct mbuf *m; | struct mbuf *m; | ||||||||
struct cmsghdr *cm; | struct cmsghdr *cm; | ||||||||
void *data; | void *data; | ||||||||
▲ Show 20 Lines • Show All 187 Lines • Show Last 20 Lines |
Isn't this the same as m = sbcut_locked(sb, sb->sb_ccc);?