[PATCH] Sanitize printf functions

Alexander Potapenko glider at google.com
Mon Dec 30 00:43:40 PST 2013



================
Comment at: sanitizer_common/sanitizer_common_interceptors_scanf.inc:10
@@ -9,3 +9,3 @@
 //
 // Scanf implementation for use in *Sanitizer interceptors.
 // Follows http://pubs.opengroup.org/onlinepubs/9699919799/functions/fscanf.html
----------------
Please update this comment.
Also, how about renaming the file to something like "sanitizer_common_interceptors_printf_scanf.inc" or "sanitizer_common_interceptors_format.inc"?

================
Comment at: sanitizer_common/sanitizer_common_interceptors_scanf.inc:281
@@ -280,1 +280,3 @@
 
+  // TODO(y.gribov): check format?
+
----------------
Yury Gribov wrote:
> Kostya Serebryany wrote:
> > good idea. feel free to implement or to leave the TODO.
> > Actually this code base uses FIXME (w/o name) more often. 
> > Actually this code base uses FIXME (w/o name) more often.
> 
> Got it. Anyway, this was more a question for reviewers than real TODO. I'll add format check in next rev.
We sure need to check |format|. Since you've already touched scanf_common in this CL, why not just add
  COMMON_INTERCEPTOR_READ_RANGE(ctx, format, internal_strlen(format) + 1);

?

================
Comment at: sanitizer_common/sanitizer_common_interceptors_scanf.inc:362
@@ +361,3 @@
+      default:
+        break; // TODO(ygribov): issue warning?
+      }
----------------
Two spaces before the comment, please.

================
Comment at: sanitizer_common/sanitizer_platform_interceptors.h:76
@@ -75,3 +75,3 @@
 # define SANITIZER_INTERCEPT_SCANF SI_NOT_WINDOWS
 # define SANITIZER_INTERCEPT_ISOC99_SCANF SI_LINUX
 
----------------
I wonder why the lines above have this strange indentation, but let us fix that in a separate commit.


http://llvm-reviews.chandlerc.com/D2480



More information about the llvm-commits mailing list