Date: Sat, 29 Dec 2012 01:25:31 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Juli Mallett <jmallett@freebsd.org> Cc: svn-src-head@freebsd.org, Oleksandr Tymoshenko <gonzo@freebsd.org>, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r244765 - in head/sys: kern tools Message-ID: <20121229010402.K1464@besplex.bde.org> In-Reply-To: <CACVs6=-D_z7609-6xDz4gKWVFmX1Bfv-%2BhueMviLbt0sPte5Rw@mail.gmail.com> References: <201212280652.qBS6qrj7072246@svn.freebsd.org> <CACVs6=-D_z7609-6xDz4gKWVFmX1Bfv-%2BhueMviLbt0sPte5Rw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121229010402.K1464>