On 11/09/2021 17:28, Andriy Gapon wrote:
On 11/09/2021 17:16, Andriy Gapon wrote:
On 11/09/2021 17:13, Mark Johnston wrote:
I think the semantic change is ok. Did you change FIONREAD to lock the
sockbuf? I think it would be necessary to avoid races with pulseaudio:
sb_acc is modified before sb_ctl, so there could be windows where
sbavail(sb) - sb->sb_ctl gives a larger.
And, it is not really safe to lock the sockbuf itself, since it may be
overwritten by a listen(2) call. SOCK_RECVBUF_LOCK(so) should be used
instead.
I didn't think about the locking, so I didn't add it.
My current patch is trivial:
@@ -210,7 +210,7 @@ soo_ioctl(struct file *fp, u_long cmd, void *data, struct
ucred *active_cred,
if (SOLISTENING(so)) {
error = EINVAL;
} else {
- *(int *)data = sbavail(&so->so_rcv);
+ *(int *)data = sbavail(&so->so_rcv) - so->so_rcv.sb_ctl;
}
break;
Let me try adding the lock.
By the way, soo_stat() seems to be another good example to follow.
So, this is what I've got:
diff --git a/sys/kern/sys_socket.c b/sys/kern/sys_socket.c
index e53b0367960b..11ee03703407 100644
--- a/sys/kern/sys_socket.c
+++ b/sys/kern/sys_socket.c
@@ -210,7 +210,12 @@ soo_ioctl(struct file *fp, u_long cmd, void *data, struct
ucred *active_cred,
if (SOLISTENING(so)) {
error = EINVAL;
} else {
- *(int *)data = sbavail(&so->so_rcv);
+ struct sockbuf *sb;
+
+ sb = &so->so_rcv;
+ SOCKBUF_LOCK(sb);
+ *(int *)data = sbavail(sb) - sb->sb_ctl;
+ SOCKBUF_UNLOCK(sb);
}
break;
--
Andriy Gapon
