Re: git: c74ab5ce6f25 - main - iscsid: Always free the duplicated address in resolve_addr().

On 12/28/21 4:50 PM, John Baldwin wrote:
> The branch main has been updated by jhb:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=c74ab5ce6f259afe1720a326df7e77848cf4f00b
> 
> commit c74ab5ce6f259afe1720a326df7e77848cf4f00b
> Author:     John Baldwin <jhb@FreeBSD.org>
> AuthorDate: 2021-12-29 00:40:04 +0000
> Commit:     John Baldwin <jhb@FreeBSD.org>
> CommitDate: 2021-12-29 00:49:46 +0000
> 
>      iscsid: Always free the duplicated address in resolve_addr().
>      
>      If a "raw" IPv6 address (denoted by a leading '[') is used as a target
>      address, then 'arg' is incremented by one to skip over the '['.
>      However, this meant that at the end of the function the wrong address
>      was passed to free().  With malloc junking enabled and given suitably
>      small strings, malloc() would happily overwrite the correct number of
>      bytes with junk, but off by one byte overwriting the byte after the
>      allocation.
>      
>      This manifested as the first byte of the 'HeaderDigest' key being
>      overwritten causing the key name on the wire to be sent as
>      '\x5eaderDigest' which the target rejected.
>      
>      Reported by:    Jithesh Arakkan @ Chelsio
>      Found with:     ASAN (via WITH_ASAN=yes)
>      Sponsored by:   Chelsio Communications

WITH_ASAN=yes (and WITH_UBSAN=yes) did make using ASAN for this quite easy to
use (just rebuilt libiscsiutil and iscsid with those knobs).  The one oddity I
ran into is that I had expected some sort of core dump when the sanitizers had
an error.  Instead, the error message was written to stderr, which was promptly
lost since iscsid is a daemon.  I resorted to using ktrace and then stitching
the GIO writes to stderr back together.  The message itself was indeed quite
useful:

=================================================================

==4566==ERROR: AddressSanitizer: attempting free on address which was \
not malloc()-ed: 0x602000000031 in thread T0

     #0 0x10c96a2 in free /usr/src/contrib/llvm-project/compiler-rt/lib\
/asan/asan_malloc_linux.cpp:111:3
     #1 0x10f997c in resolve_addr /usr/src/usr.sbin/iscsid/iscsid.c:219\
:2
     #2 0x10f6ac4 in connection_new /usr/src/usr.sbin/iscsid/iscsid.c:2\
90:2
     #3 0x10f6ac4 in handle_request /usr/src/usr.sbin/iscsid/iscsid.c:5\
93:9
     #4 0x10f6ac4 in main /usr/src/usr.sbin/iscsid/iscsid.c:751:3


0x602000000031 is located 1 bytes inside of 15-byte region [0x60200000\
0030,0x60200000003f)

allocated by thread T0 here:

     #0 0x10c0584 in __interceptor_strdup /usr/src/contrib/llvm-project\
/compiler-rt/lib/asan/asan_interceptors.cpp:439:3
     #1 0x11079b8 in checked_strdup /usr/src/lib/libiscsiutil/utils.c:4\
0:6
     #2 0x10f952b in resolve_addr /usr/src/usr.sbin/iscsid/iscsid.c:157\
:8
     #3 0x10f6ac4 in connection_new /usr/src/usr.sbin/iscsid/iscsid.c:2\
90:2
     #4 0x10f6ac4 in handle_request /usr/src/usr.sbin/iscsid/iscsid.c:5\
93:9
     #5 0x10f6ac4 in main /usr/src/usr.sbin/iscsid/iscsid.c:751:3
     #6 0x1070f8c in _start /usr/src/lib/csu/amd64/crt1_c.c:73:7
     #7 0x80112e007  (<unknown module>)

SUMMARY: AddressSanitizer: bad-free /usr/src/contrib/llvm-project/comp\
iler-rt/lib/asan/asan_malloc_linux.cpp:111:3 in free

==4566==ABORTING

iscsid: child process 4566 terminated with exit status 1

-- 
John Baldwin