Date: Tue, 26 Jan 2016 17:45:35 +0000 From: bugzilla-noreply@freebsd.org To: freebsd-bugs@FreeBSD.org Subject: [Bug 206648] Fix double strlen in ktrstruct Message-ID: <bug-206648-8-KCzfgzcM8G@https.bugs.freebsd.org/bugzilla/> In-Reply-To: <bug-206648-8@https.bugs.freebsd.org/bugzilla/> References: <bug-206648-8@https.bugs.freebsd.org/bugzilla/>
next in thread | previous in thread | raw e-mail | index | archive | help
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D206648 Mateusz Guzik <mjg@FreeBSD.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mjg@FreeBSD.org --- Comment #2 from Mateusz Guzik <mjg@FreeBSD.org> --- 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.=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bug-206648-8-KCzfgzcM8G>