diff options
author | Evan Green <evgreen@chromium.org> | 2019-10-01 14:26:29 -0700 |
---|---|---|
committer | Commit Bot <commit-bot@chromium.org> | 2019-10-05 00:47:59 +0000 |
commit | 47b264c6eb5074f74dac2ed3d88e7b9e4e6d3139 (patch) | |
tree | 188246705a36af80ba1ba466914c5f20f9bdb3c5 /common | |
parent | e77ccb89c1b74a9ba3be47072d35e3e148d2a469 (diff) | |
download | chrome-ec-47b264c6eb5074f74dac2ed3d88e7b9e4e6d3139.tar.gz |
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 <evgreen@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1834604
Reviewed-by: Jack Rosenthal <jrosenth@chromium.org>
Diffstat (limited to 'common')
-rw-r--r-- | common/printf.c | 18 |
1 files changed, 17 insertions, 1 deletions
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; |