Re: recvmsg() "short receive" after FIONREAD

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