From nobody Wed Dec 29 00:58:29 2021 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 3D8A3190BDAA; Wed, 29 Dec 2021 00:58:31 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4JNtLt6gPGz4t4N; Wed, 29 Dec 2021 00:58:30 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from [10.0.1.4] (ralph.baldwin.cx [66.234.199.215]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id 69273D7B7; Wed, 29 Dec 2021 00:58:30 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Message-ID: <54437778-3a4a-d049-db33-47afd769d32c@FreeBSD.org> Date: Tue, 28 Dec 2021 16:58:29 -0800 List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:91.0) Gecko/20100101 Thunderbird/91.4.1 Subject: Re: git: c74ab5ce6f25 - main - iscsid: Always free the duplicated address in resolve_addr(). Content-Language: en-US From: John Baldwin To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org References: <202112290050.1BT0o78E003806@gitrepo.freebsd.org> In-Reply-To: <202112290050.1BT0o78E003806@gitrepo.freebsd.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1640739510; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=+sQlTL+l6mmwqQjhqgLo/5ht7Vtl/iE1Yrk54GOk/F0=; b=nsY/XiyUCgUkhq+7uiDOvrWg3H+T6MTQzcu8kXze0NqBxN0axXy3ae4LbYgAXFqd7ggzyw daW95Rc23mxqYhPcSVcTKO/rdbp/vbC3Zy7s1F1UsJ2MNSV99qOBlRDTuae3ds/QAD2XTl MUUl5XSoTITo1Ypcl1jS2HE+q86aHp1+xELgKAmDlozniapSHMeH+xanc3msrVLdSlWH5c pTI10FCCbJXVxcdVcvQcLOKJTPl6FnZOP3gyrrbyowOuUAJAwRT+mZSECgA6tfG40P+3Yl uOVkcIU7VAv4YS2XHxCZ02MC3GYhoxlx8j3Ba6JTuuOI9ip4vuFzNtHq6AEzuw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1640739510; a=rsa-sha256; cv=none; b=HSqUZRIftpffK4sfZSfhvFNNebcOeM3zJuKYYuJ9znivn54yTh/ftAHoSGJ0C3hySeuu1g m64cN3a2k2ZvUwvb2RLLwL+RYlHdbsX1mmKSLCTADcjqY84tH8b7tE+d3ZImkDN9YT3d07 /ntA8W7v3IOxse8UNpSPph2zcd9Iq+zKsWAg+nEWE5ZTFsSr7EiXUXwu8Ek8hKvJBeW1Cy yiZHzDnRhzVa6YA20yu4RxUg1H+59ipFG7bpN+8sAxx9th7GSCfiOydZ4x1FOFxdIUYleR T0JqSVXQSHHw/GcPMrysdb3bupvKC/5CTTCkDOYLDjRUYNFe5cLE8va/fu2VVQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N 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 > AuthorDate: 2021-12-29 00:40:04 +0000 > Commit: John Baldwin > 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 () 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