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>

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;