Re: [PATCH v7 tip 4/8] tracing: allow BPF programs to call bpf_trace_printk()

On 3/19/15 8:29 AM, Steven Rostedt wrote:

quoted

+	/* check format string for allowed specifiers */
+	for (i = 0; i < fmt_size; i++)

Even though there's only a single "if" statement after the "for", it is
usually considered proper to add the brackets if the next line is
complex (more than one line). Which it is in this case.

ok.

quoted

+			} else if (fmt[i] == 'p') {
+				mod_l[fmt_cnt] = true;
+				fmt_cnt++;

So you also allow pointer conversions like "%pS" and "%pF"?

good catch. it's a bug. We shouldn't allow things like pV, pD, etc
Something like pK and pS may be ok, but pF is not because of arch
dependencies. So instead of analyzing all possibilities. I'll allow
%p only. bpf_trace_printk is debug only anyway.

quoted

+	return __trace_printk(1/* fake ip will not be printed */, fmt,
+			      mod_l[0] ? r3 : (u32) r3,
+			      mod_l[1] ? r4 : (u32) r4,
+			      mod_l[2] ? r5 : (u32) r5);

Does the above work on 32 bit machines? as "%ld" would be (u32), but
here you are passing in u64.

another great catch. it wouldn't crash, but would print junk. will fix.