From 47b264c6eb5074f74dac2ed3d88e7b9e4e6d3139 Mon Sep 17 00:00:00 2001 From: Evan Green Date: Tue, 1 Oct 2019 14:26:29 -0700 Subject: printf: Deprecate %l The semantics of %l changed during the enabling of compile-time printf format checking. Old firmware branches will treat something like %lx as a 64-bit value, but new code on master will enforce at compile-time that a long (32-bits on our ECs) is passed in as the argument. This creates a dangerous and difficult to notice situation if the following code is cherry-picked from master into an old firmware branch: printf("%lx %s", myval32, mystr); On master, this behaves correctly. On the old firmware branch, this would swallow myval32 and mystr for %lx, and then %s would grab a random stack pointer and print a string from it. Deprecating %l is our mechanism for keeping such a printf from creeping into master in the future. Obviously we can't protect against someone that checks in code that's never tested, but anyone who tests a printf with %l in it will notice their printf comes out with ERROR instead of what they want. BUG=chromium:984041 TEST=make -j buildall BRANCH=None Change-Id: I0267430363af7954c2ec5d2c45222759fe0ec2c1 Signed-off-by: Evan Green Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1834604 Reviewed-by: Jack Rosenthal --- common/printf.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) (limited to 'common/printf.c') diff --git a/common/printf.c b/common/printf.c index 1f6733e552..dd66d3630a 100644 --- a/common/printf.c +++ b/common/printf.c @@ -225,7 +225,7 @@ int vfnprintf(int (*addchar)(void *context, int c), void *context, /* * Handle length: - * %l - long + * %l - DEPRECATED (see below) * %ll - long long * %z - size_t */ @@ -239,6 +239,22 @@ int vfnprintf(int (*addchar)(void *context, int c), void *context, c = *format++; } + /* + * %l on 32-bit systems is deliberately + * deprecated. It was originally used as + * shorthand for 64-bit values. When + * compile-time printf format checking was + * enabled, it had to be cleaned up to be + * sizeof(long), which is 32 bits on today's + * ECs. This presents a mismatch which can be + * dangerous if a new-style printf call is + * cherry-picked into an old firmware branch. + * See crbug.com/984041 for more context. + */ + if (!(flags & PF_64BIT)) { + format = error_str; + continue; + } } else if (c == 'z') { if (sizeof(size_t) == sizeof(uint64_t)) flags |= PF_64BIT; -- cgit v1.2.1