From owner-svn-src-all@FreeBSD.ORG Fri Dec 28 14:25:41 2012 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id C4193684; Fri, 28 Dec 2012 14:25:41 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail03.syd.optusnet.com.au (mail03.syd.optusnet.com.au [211.29.132.184]) by mx1.freebsd.org (Postfix) with ESMTP id 5524B8FC0C; Fri, 28 Dec 2012 14:25:40 +0000 (UTC) Received: from c122-106-175-26.carlnfd1.nsw.optusnet.com.au (c122-106-175-26.carlnfd1.nsw.optusnet.com.au [122.106.175.26]) by mail03.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id qBSEPVXi016509 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 29 Dec 2012 01:25:33 +1100 Date: Sat, 29 Dec 2012 01:25:31 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Juli Mallett Subject: Re: svn commit: r244765 - in head/sys: kern tools In-Reply-To: Message-ID: <20121229010402.K1464@besplex.bde.org> References: <201212280652.qBS6qrj7072246@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.0 cv=e5de0tV/ c=1 sm=1 a=uCOecJ8eGE4A:10 a=kj9zAlcOel0A:10 a=PO7r1zJSAAAA:8 a=JzwRw_2MAAAA:8 a=LNCWVHfIY9kA:10 a=PbejLz-6Xlbcm8SuJw8A:9 a=CjuIK1q_8ugA:10 a=bxQHXO5Py4tHmhUgaywp5w==:117 Cc: svn-src-head@freebsd.org, Oleksandr Tymoshenko , svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 28 Dec 2012 14:25:41 -0000 On Thu, 27 Dec 2012, Juli Mallett wrote: > Seems like uintmax_t and %ju might be better? At least, truncating a > syscall arg to a pointer seems like it could be misleading (e.g. on > MIPS n32, even though it's an abomination.) Seems unlikely. At least the old KTR calls only support u_long args, the type mismatches for uintmax_t and %ju are more fatal than the type mismatches for int and %d -- uintmax_t is just too large for KTR on all 32-bit arches. >> Modified: head/sys/kern/subr_syscall.c >> ============================================================================== >> --- head/sys/kern/subr_syscall.c Fri Dec 28 05:48:44 2012 (r244764) >> +++ head/sys/kern/subr_syscall.c Fri Dec 28 06:52:53 2012 (r244765) >> @@ -78,7 +78,7 @@ syscallenter(struct thread *td, struct s >> ktrsyscall(sa->code, sa->narg, sa->args); >> #endif >> KTR_START4(KTR_SYSC, "syscall", syscallname(p, sa->code), >> - td, "pid:%d", td->td_proc->p_pid, "arg0:%p", sa->args[0], >> + (uintptr_t)td, "pid:%d", td->td_proc->p_pid, "arg0:%p", sa->args[0], >> "arg1:%p", sa->args[1], "arg2:%p", sa->args[2]); >> >> if (error == 0) { I don't see where the format td is. Anything except %lx would be wrong. Note that all the visible formats are wrong: - %p for pointers is never compatible with u_long for the args - %d for pids is normally compatible with pid_t = int32_t for the original args, but becomes incompatible when the args are converted to u_long. I think the format string is only used in userland. ktrdump uses bugs like the following to print it: % fprintf(out, desc, parms[0], parms[1], parms[2], parms[3], % parms[4], parms[5]); Here the format 'desc' is the original string from the kernel, so it usually has %p's and %d's in it, while parms[N] is a u_long containing a copy of the u_long parameter in the kernel, so it never matches %p's and %d's in the format string. Correct code would parse `desc' and either change the parameters to match the format or change the format to match the parameters. fmtcheck(3) should notice these bugs. -Wformat in the kernel doesn't notice these bugs (or the more fatal ones for %ju formats) because CTR*() is not variadic -- it just converts all args to u_long. Bruce