Fix #332: prevent mutex leak in ipv4_connect when LocalIP is already set #401

2026-03-31T17:49:39Z

jasonL commented

2026-03-31 17:49:39 +00:00

Fixes #332.

This fixes a mutex leak in ipv4_connect when IP_SOCKET.LocalIP is already set.

The old code used:

cmp     [eax + IP_SOCKET.LocalIP], 0
jne     @f

after checking LocalIP.

In ipv4_connect, that anonymous forward jump could leave the normal tail path and skip the RemoteIP update together with mutex_unlock, so repeated connect calls on the same socket could leave SOCKET.mutex locked.

The fix is to replace that anonymous jump with an explicit local label, so both LocalIP == 0 and LocalIP != 0 paths continue through the same cleanup flow:

cmp     [eax + IP_SOCKET.LocalIP], 0
jne     .local_ip_ready
push    [IPv4_address + 4]                                   ; FIXME: use correct local IP
pop     [eax + IP_SOCKET.LocalIP]

  .local_ip_ready:
pushd   [edx + 4]
pop     [eax + IP_SOCKET.RemoteIP]

Verified with the reproducer attached to #332 (sock_dedlock.ASM), which calls connect repeatedly on the same socket.

On the baseline build, the first connect returned, the next call left the socket locked, and the following call blocked on the same mutex.

On the patched build, repeated connect calls returned normally and no stuck mutex was observed.

I also checked this with temporary debug instrumentation in ipv4_connect. The baseline trace was ELZRUX, then EL, then E; the patched trace was ELZRUX, then ELNRUX, then ELNRUX.

Here E means function entry, L means mutex_lock returned, Z means LocalIP == 0, N means LocalIP != 0, R means RemoteIP was written, U means mutex_unlock, and X means return.

This change only fixes the lock leak in ipv4_connect. It does not change the existing FIXME: use correct local IP behavior.

Fixes #332. This fixes a mutex leak in `ipv4_connect` when `IP_SOCKET.LocalIP` is already set. The old code used: ```asm cmp [eax + IP_SOCKET.LocalIP], 0 jne @f ``` after checking `LocalIP`. In `ipv4_connect`, that anonymous forward jump could leave the normal tail path and skip the `RemoteIP` update together with `mutex_unlock`, so repeated `connect` calls on the same socket could leave `SOCKET.mutex` locked. The fix is to replace that anonymous jump with an explicit local label, so both `LocalIP == 0` and `LocalIP != 0` paths continue through the same cleanup flow: ```asm cmp [eax + IP_SOCKET.LocalIP], 0 jne .local_ip_ready push [IPv4_address + 4] ; FIXME: use correct local IP pop [eax + IP_SOCKET.LocalIP] .local_ip_ready: pushd [edx + 4] pop [eax + IP_SOCKET.RemoteIP] ``` Verified with the reproducer attached to #332 (`sock_dedlock.ASM`), which calls `connect` repeatedly on the same socket. On the baseline build, the first `connect` returned, the next call left the socket locked, and the following call blocked on the same mutex. On the patched build, repeated `connect` calls returned normally and no stuck mutex was observed. I also checked this with temporary debug instrumentation in `ipv4_connect`. The baseline trace was `ELZRUX`, then `EL`, then `E`; the patched trace was `ELZRUX`, then `ELNRUX`, then `ELNRUX`. Here `E` means function entry, `L` means `mutex_lock` returned, `Z` means `LocalIP == 0`, `N` means `LocalIP != 0`, `R` means `RemoteIP` was written, `U` means `mutex_unlock`, and `X` means return. This change only fixes the lock leak in `ipv4_connect`. It does not change the existing `FIXME: use correct local IP` behavior.

👍 1

jasonL added 1 commit 2026-03-31 17:49:46 +00:00

net: fix mutex leak in ipv4_connect

Build system / Check kernel codestyle (pull_request) Successful in 1m42s

Details

Build system / Build (pull_request) Successful in 12m27s

Details

0b4562bac4

Doczom commented

2026-04-01 07:07:01 +00:00

align 4
ipv4_connect:

        push    eax edx
        lea     ecx, [eax + SOCKET.mutex]
        call    mutex_lock
        pop     edx eax

; Fill in local IP
        cmp     [eax + IP_SOCKET.LocalIP], 0
        jne     @f
        push    [IPv4_address + 4]                                   ; FIXME: use correct local IP
        pop     [eax + IP_SOCKET.LocalIP]

@@:
; Fill in remote IP
        pushd   [edx + 4]
        pop     [eax + IP_SOCKET.RemoteIP]

        lea     ecx, [eax + SOCKET.mutex]
        call    mutex_unlock

        xor     eax, eax
        ret

``` align 4 ipv4_connect: push eax edx lea ecx, [eax + SOCKET.mutex] call mutex_lock pop edx eax ; Fill in local IP cmp [eax + IP_SOCKET.LocalIP], 0 jne @f push [IPv4_address + 4] ; FIXME: use correct local IP pop [eax + IP_SOCKET.LocalIP] @@: ; Fill in remote IP pushd [edx + 4] pop [eax + IP_SOCKET.RemoteIP] lea ecx, [eax + SOCKET.mutex] call mutex_unlock xor eax, eax ret ```

Doczom approved these changes 2026-04-01 08:24:37 +00:00

Doczom requested review from dunkaist 2026-04-01 08:24:47 +00:00

dunkaist approved these changes 2026-04-01 09:57:46 +00:00

Doczom merged commit 0b4562bac4 into main

2026-04-02 16:28:11 +00:00

Sign in to join this conversation.

3 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KolibriOS/kolibrios#401

Allow edits from maintainers