From owner-freebsd-bugs@freebsd.org Tue Jan 26 17:45:35 2016 Return-Path: Delivered-To: freebsd-bugs@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 5FB04A6E69C for ; Tue, 26 Jan 2016 17:45:35 +0000 (UTC) (envelope-from bugzilla-noreply@freebsd.org) Received: from kenobi.freebsd.org (kenobi.freebsd.org [IPv6:2001:1900:2254:206a::16:76]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 339A7664 for ; Tue, 26 Jan 2016 17:45:35 +0000 (UTC) (envelope-from bugzilla-noreply@freebsd.org) Received: from bugs.freebsd.org ([127.0.1.118]) by kenobi.freebsd.org (8.15.2/8.15.2) with ESMTP id u0QHjZTD006775 for ; Tue, 26 Jan 2016 17:45:35 GMT (envelope-from bugzilla-noreply@freebsd.org) From: bugzilla-noreply@freebsd.org To: freebsd-bugs@FreeBSD.org Subject: [Bug 206648] Fix double strlen in ktrstruct Date: Tue, 26 Jan 2016 17:45:35 +0000 X-Bugzilla-Reason: AssignedTo X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: Base System X-Bugzilla-Component: kern X-Bugzilla-Version: 11.0-CURRENT X-Bugzilla-Keywords: X-Bugzilla-Severity: Affects Many People X-Bugzilla-Who: mjg@FreeBSD.org X-Bugzilla-Status: New X-Bugzilla-Resolution: X-Bugzilla-Priority: --- X-Bugzilla-Assigned-To: freebsd-bugs@FreeBSD.org X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: cc Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: https://bugs.freebsd.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Jan 2016 17:45:35 -0000 https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D206648 Mateusz Guzik changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mjg@FreeBSD.org --- Comment #2 from Mateusz Guzik --- The code is fine correctness-wise, although indeed could be improved by get= ting rid of double strlen, I'll maybe commit that later. If you make changes you would like to submit, please make actual patches (d= iff -u, svn diff, git diff, whatever). Now to the point: buflen =3D strlen(name) + 1 datalen;=20=20=20=20=20=20=20=20=20=20=20=20=20= =20=20=20=20=20=20 buf =3D malloc(buflen, M_KTRACE, M_WAITOK); strcpy(buf, name); bcopy(data, buf + strlen(name) + 1, datalen); The claim is that calling strlen twice would be problematic if the string c= ould change. First of all, if that was really the case, we would run into trouble with strcpy. Further, if it could change, maybe it also could be freed? Clearly, the kernel code needs to ensure the stability of the string. If it= is assumed the string is not going to change, the code is perfectly fine, alth= ough somewhat inefficient. If the string is in kernel memory and can change, appropriate locks need to= be held across the call. If it is in user memory, it has to be brought in with things like copyin(). Regardless, I see no bug nor an insecure practice here. If one was to not t= rust anything, it would be impossible to implement stuff. That said thanks for bringing this up, that strlen would really use getting= rid of, but there is nothing to fix correctness-wise or even in the spirit of defensive programming. One could consider adding an assertion the buffer belongs to the kernel, but then why would this function be special. --=20 You are receiving this mail because: You are the assignee for the bug.=