Index | Thread | Search

From:
Vitaliy Makkoveev <mvs@openbsd.org>
Subject:
Re: Don't take solock() in soreceive() for tcp(4) sockets
To:
Alexander Bluhm <alexander.bluhm@gmx.net>
Cc:
tech@openbsd.org
Date:
Sat, 20 Apr 2024 01:15:32 +0300

Download raw body.

Thread
> On 20 Apr 2024, at 00:51, Alexander Bluhm <alexander.bluhm@gmx.net> wrote:
> 
> On Wed, Apr 17, 2024 at 06:39:14PM +0300, Vitaliy Makkoveev wrote:
>> This is the updated diff. Witness kernel passed regress/usr.sbin/syslogd
>> without issues.
> 
> I see a witness error with this diff.  Maybe it was there before
> and I missed it due to the regular findings.
> 
> witness: lock order reversal:
> 1st 0xf9853a0c vmmaplk (&map->lock)
> 2nd 0xf93fcd74 sbufrcv (&so->so_rcv.sb_lock)
> lock order "&so->so_rcv.sb_lock"(rwlock) -> "&map->lock"(rwlock) first seen at:
> #0  witness_checkorder+0x291
> #1  rw_enter_read+0x32
> #2  vm_map_lock_read_ln+0x15
> #3  uvmfault_lookup+0x90
> #4  uvm_fault_check+0x14
> #5  uvm_fault+0xe4
> #6  kpageflttrap+0xe5
> #7  trap+0x25f
> #8  calltrap+0xc
> #9  copyout+0x42
> #10 uiomove+0x1c0
> #11 soreceive+0x841
> #12 soo_read+0x37
> #13 dofilereadv+0xb5
> #14 sys_read+0x49
> #15 syscall+0x41d
> #16 Xsyscall_untramp+0xa9
> lock order data w1 -> w2 missing
> 

This is something new, never seen this before. It was
regress/usr.sbin/syslogd?

Please keep in mind, the original sblock() is the lock too, but it has
no witness(4) support. If this witness report is valid, this means we
already have lock order problem in this area.

>> tcp(4) is very complicated. Isn't it better to convert AF_UNIX, AF_KEY
>> and AF_ROUTE sockets before?
> 
> I doubt that we will unlock TCP soon.  But if we manage to make
> progress in some parts of TCP that is fine.  We have to start
> somewhere.
> 

sosend() looks much simpler to unlock tcp(4). May be it will be
unlocked first. At least is should make clean locking around somove().

However, AF_UNIX, AF_KEY and AF_ROUTE sockets use the old locking in
soreceive(). To unify sockets layer i propose to convert them in the
nearest future.