From nobody Tue Jul 1 13:18:28 2025 X-Original-To: dev-commits-src-main@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 4bWkB063ysz60NXc; Tue, 01 Jul 2025 13:18:32 +0000 (UTC) (envelope-from avg@freebsd.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (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 "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4bWkB04Gyrz4M8b; Tue, 01 Jul 2025 13:18:32 +0000 (UTC) (envelope-from avg@freebsd.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1751375912; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:autocrypt:autocrypt; bh=KQjZo7m/Fboka2h/Cm6t4xKsS0+nhrdwVFzIoN7cT/A=; b=ScHz8Ek64oNux7fudojHZXXTFusQcwCZxCRRmgI3XHjEh6jfFH8Ms52amchJx44DWjT/Ap +nyqGcHQst0jM8HP6yc5K70lNMMSwAIcIHX/IH2GKM5XfkSc5Pgpw9I5wedpL7Ak4WpW7D jEs7kIaDo2Nm2GD/GCoNJ1Ns2x5rZqDUd5EkCeGqRdGPQ6aMVt/2uUj+KolzJSIGTkN7rQ 3dLEmJDcXBM8Ke7rQOBVFkQN1IJoaemy57RzENTDlykN0FSNV+MqXw+Kzm/KjlGRbM8XkP JkcW+FwHjxgMF5TwNhW/MpqFMY4AZcbgGOJ+3fQOY6V5UaGASDUPKh2q9Ah38w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1751375912; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:autocrypt:autocrypt; bh=KQjZo7m/Fboka2h/Cm6t4xKsS0+nhrdwVFzIoN7cT/A=; b=SZFps3CLQx46gLfRAg5FB3GYKT7yMx1YnRNHMpU1J9/5RelTO/t5VPoOMktKJPGobyLW2i aDyG26d8GAqyKvxoDQz0zhwKx3IHAZFeCQcpOQuk8gFzY70hLjN3DM0nqm4MJPVF5VnIfY SqnoO852nEgVM1Iqi743WKXGellzDo1E4DsS4rEAfEIrg4eW32pM50NcRqpTno883FPFUR rxTyg4BaRnt7kwaTHuFaO2RoKUvTLJmeZ/O5Emr1Ik80CDQYa9dmbROyNIEjNxKgv2AAMD pMogtxj9Ue7xaibC7fuIhzyxsW04q998a73jzrXf2hlnfIdupa25oMgz6oAJMA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1751375912; a=rsa-sha256; cv=none; b=k53l1JgaLaY24qFwntafjxVijzbpU3ZLLlGvzt5HwSiq2VkT82FRt609hALxX9PjYVYDXy SP/Do8JDzXjdM8k6me0a1G8o6oTjfUdMDLtdqqzqQaeMzyOdTQMa5ZdX82uN62SqGz1ETr 33+L/DahtZdrkankAT5AAXGQ5iGsY0FZjS7KJ8DD+jit1eztoJiKZtsFHExo8EbEhwirMl GFHRUp5RPrmmUePESUlwBDpF20sCSjq2/abKJ2KfJ4ZDzG+9mTCpPtVFiNQmGC7eS1M4mJ sRr86xVVeiVffolkAdG62Iv/ysLFUzM5jXCwt46pcj+MNLM1G9hLAYf9SWXbeg== Received: from [192.168.0.88] (unknown [93.188.39.137]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) (Authenticated sender: avg/mail) by smtp.freebsd.org (Postfix) with ESMTPSA id 4bWk9z5bBgz1Cd4; Tue, 01 Jul 2025 13:18:31 +0000 (UTC) (envelope-from avg@freebsd.org) Message-ID: <04134894-a3a4-43b1-ab92-f8947e046d30@freebsd.org> Date: Tue, 1 Jul 2025 16:18:28 +0300 List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: Andriy Gapon Subject: Re: git: ad8d33679999 - main - mmc_xpt: use strlcpy instead of strncpy To: Warner Losh Cc: John Baldwin , src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org References: <202506270721.55R7LlB1067221@gitrepo.freebsd.org> <69858708-8e73-4b70-9f33-5176fceb1b36@FreeBSD.org> <89e23d9e-885c-4811-886a-6c1011715679@FreeBSD.org> Content-Language: en-US Autocrypt: addr=avg@freebsd.org; keydata= xsDNBGcKrHEBDADRvwQOK0b/yo4ys5cs6bOQMhEh4xtfbaZ/CU00cpPgUip3sOZCdrtMWlRC g25z97prxE9pKueZi+HXDhIPpa9xl14ghqF4oYScuJ1i18HyiOH2y5Q3Vv/TtFiSzicd3EAu QgS3jVidpgDSPDdj2Yz3UxYpZ+PuFl6nOnvCvqOFcjUlzKCyPaiN2b86l1Nscmhnc+zQ/faB erUOEFEDQbWMA5YfXi8HrbeR16hfRfGt7E0aMDlIj9FIPIq71UWMN9CimPgs4+rbNr1MAlLa z4GxSDhVYZEY5rqtCzr+PLXboRQWnaUwXl0/biw9enf17NHdYv1SNAFTX2eC4dZ3qBVI74dS PgNprm+PMfz+6Hhs/dAv+Nan5nVhg3EFIjYTiy0MnjMSq8uI0v0ykpAGAcJJ5xl6d23aLxgN 6f0z6pJRCO0hGPgU7UzvFD0MxJxmbzqdT1R51KDan1oD41b+tjl2LMBuCDCoB0U44Pu0zLdp xMfFTxCXtwIYKIUxwd28jwMAEQEAAc0eQW5kcml5IEdhcG9uIDxhdmdARnJlZUJTRC5vcmc+ wsENBBMBCAA3FiEEmXvSmjiQFHPVOpLnzDOt5NLj67sFAmcKrHEFCQeEzgACGwMECwkIBwUV CAkKCwUWAgMBAAAKCRDMM63k0uPru5tSDACFK15LLbq89RSQ6QMnjiIm1t/wYJyumb519MHu Dhzxx1lbr8oghf0RHtF6kYRLQPaW2VdToi74pRobd3CN4bhZKDLSL6WfTn17RfavDjL6Njwp KBo30CkOeYKWq1mDmo0xEoQj8cc7ybEZnus+YScZOpj8Ti4EFwhRt6SHer7YDb161IHKL8m4 MsCxpFSGEjbKj8Iul3Ri/fTOO8w14ivcuEEQIvJt4/+4YV5Az8G23wKzL/3aJ7SOT3oYGmR9 atBTmVO3DlODjM+rZLegd8SfLSPTcBTHspWE5duemIzZbEX3BP77r3Qx4Fo5Tkit3bG1XVar yPQato+sFGFEGifdE9USBQoAoOaaeZevwAWjDU0TIuCT0CUe0sKtQuNP4LRq0n9EEHOXBu9a CfdMhFUSkAZnuE7miSVwgPvoVNJ1stA37EXLN/sVsWik7wslTQ5vF81VpdGFiwoQPOe2XEKh ogcwGSnXbwv1gD4x+Gz/7Y+kFyr1NY+4/nSaeXVcS2fOwM0EZwqscgEMAMQTe6ypAmQe/TFO HqKD2hfFKdksTptKi6uEh8xIwct8G/0FBldDWXo9eu8CGr/ZrDg0/bAwJxbaLRQCMH19Gq2Y hLvZ1QK5GQJVzZKcqfxbF2LiDUTs6WkdOBIhGpdDy7p1xFrvqCGCtNFYHuGYm067EozibBSF BWAPstKu2FQuVHZNMOfs7p3OIz3Yfqu9woXDeg3/8G2qVQJINe+8EwXKlhgh4CyDbq7nAZoA kIu1SE9z9u3WI5mcNy/0dFmVUsFxBqRC3ewbvzie8tKyZ9yFOlaZPT0Y4nRBXQTI3mLZ8zQ8 mtrWK5OOmrJ02kdeO9RBXe+OMaUUWMf92ZIoBFb4HP6N+B+4N1y1OwULousfl7JRoYxA4MRL ls7E2sSoJvrEBTJB3Pc34xu8rsJ1A5V3NgN6djX8yEZYpTRkcmrBeWy/ofDqZPVqneAx0LRm eldDS9msXDW4KXODyPZ+9unvmHAcoH0xaBYaSH44CDZDQDg4LNcmbOvuu1TEXBJhjQARAQAB wsD8BBgBCAAmFiEEmXvSmjiQFHPVOpLnzDOt5NLj67sFAmcKrHMFCQeEzgACGwwACgkQzDOt 5NLj67sUCAv5AXqgWnYN9EblapMbZjkiqL8pZQ0GNqh+Pg9FwbyULxjtRTO6rD4D0IxizByb ef+neeUNyYlagt5nfKMysEr0SU/gHKCi8vyTF/63ukMrGUNGmJJxrndl5ZYKC6j6eX7twrZF L1Uvlmn6FnQ22red5kHO93fDjG4zaDIZvHfwj7kzjZ4tpC7Byinf88s14mdZeScc0PnU2hj4 UGYju/wg2FF4YxaZYhcmdTiRYY0Wx85XSMZv19pnn78sadEuRvfRd4JTmw++j1xGXeqQGWzz /CTG5/Ex9GAkQ02hZbmi236byDXoet4G8TEyOph9QFVkV9bNd0jQZaFZPGEj4PSPUYGAF7s5 xJaNGgctC3aZ7WjEv1FBoo44XCU4xcjJ1wZQUrHxRhx6TW0Jtcl0U9qfKFW30TSPo6RyiXuj X4ltWKAtjoXB8nUmEJckaz7IRu2b4pXDeazZuz5JBygUs10yJjDxh2vFQZo0KaBAPx9MZlPn gpPTjT15L8xGftEjQXF6 In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 30/06/2025 18:19, Warner Losh wrote: > You should really get changes like this reviewed. "seems like" is not > a good enough reason to do this. In fact, it's a terrible reason in > the CAM code. There are places that the strncpy use is intentional and > you will screw things up if you use strlcpy. Too many fixed width > fields in old-school storage devices that reflected into different > APIs, sadly. That's a very good advice. I did forget that CAM and CAM-related code is a bit more magic and special than most of FreeBSD code. strncpy -> strlcpy looked like a "no brainer" as I did not spot anything special about those fields. Lesson learned. > A better reason that it's 'right' is that all but 3 other places in > the tree use strlcpy for this field. The field will be NUL padded, in > practice, since CCBs tend to be zero'd before use (though path_inq > often is done with stack storage without the usual init). And a quick > look shows that we don't use dev_name, hba_vid or sim_vid as a fixed > width field anywhere but the 'compat' code that copies the whole thing > and doesn't care. We never use any of these three fields in the kernel > for anything, apart from filling them in. We do use them in camcontrol > (and similar 3rd party programs) to report them to the user as > strings. There's other places it looks like they get injected into > geom attributes, also C strings. > > A look at history shows 4195c7de2471d intentionally changed them, and > that change was reviewed. It also has the reasons why it's OK in it. > So from that perspective, this is a good change. The reasons listed > there are sound (though it's too equivocal: these fields were always > NUL terminated except when the string was the exact length of the > field, which we didn't do anywhere, but that's not important). > > An even better question is: shouldn't a change like this have checked > the other 4 places, and changed them as well? If it's wrong in one > place, it might be wrong elsewhere. Or looking elsewhere would confirm > or deny that this was intentional for some reason. > > But there's another consideration that 4195c7de2471d didn't think > through that I have just thought of... -- Andriy Gapon