Re: recvmsg() "short receive" after FIONREAD

From: Andriy Gapon <avg_at_FreeBSD.org>
Date: Sat, 11 Sep 2021 21:25:42 +0300
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:
>> _at__at_ -210,7 +210,7 _at__at_ 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
_at__at_ -210,7 +210,12 _at__at_ 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
Received on Sat Sep 11 2021 - 18:25:42 UTC

Original text of this message