[PATCH] - improve sprintf buffer overflow detection (middle-end/49905)
Richard Biener
rguenther@suse.de
Mon Jul 4 10:59:00 GMT 2016
On Fri, 1 Jul 2016, Martin Sebor wrote:
> The attached patch enhances compile-time checking for buffer overflow
> and output truncation in non-trivial calls to the sprintf family of
> functions under a new option -Wformat-length=[12]. This initial
> patch handles printf directives with string, integer, and simple
> floating arguments but eventually I'd like to extend it all other
> functions and directives for which it makes sense.
>
> I made some choices in the implementation that resulted in trade-offs
> in the quality of the diagnostics. I would be grateful for comments
> and suggestions how to improve them. Besides the list I include
> Jakub who already gave me some feedback (thanks), Joseph who as
> I understand has deep knowledge of the c-format.c code, and Richard
> for his input on the LTO concern below.
>
> 1) Making use of -Wformat machinery in c-family/c-format.c. This
> seemed preferable to duplicating some of the same code elsewhere
> (I initially started implementing it in expand_builtin in
> builtins.c). It makes the implementation readily extensible
> to all the same formats as those already handled for -Wformat.
> One drawback is that unlike in expand_builtin, calls to these
> functions cannot readily be folded. Another drawback pointed
folded? You mean this -W option changes code generation?
> out by Jakub is that since the code is only available in the
> C and C++ compilers, it apparently may not be available with
> an LTO compiler (I don't completely understand this problem
> but I mention it in the interest of full disclosure). In light
> of the dependency in (2) below, I don't see a way to avoid it
> (moving c-format.c to the middle end was suggested but seemed
> like too much of a change to me).
Yes, lto1 is not linked with C_COMMON_OBJS (that could be changed
of course at the expense of dragging in some dead code). Moving
all the format stuff to the middle-end (or separated better so
the overhead in lto1 is lower) would be possible as well.
That said, a langhook as you add it highlights the issue with LTO.
Richard.
> 2) Optimization.
> In keeping with the other -Wformat options, the checking is
> enabled without optimization. Especially at level 2, the
> warnings can be useful even without it. But to make buffer
> sizes and non-constant argument values available in calls to
> functions like sprintf (via __builtin_object_size) better
> results are obtained with optimization.
>
> 3) Truncation warnings.
> Although calls to bounded functions like snprintf aren't subject
> to buffer overflow, they can be subject to accidental truncation
> when the destination buffer isn't sized appropriately. With the
> patch, such calls are diagnosed under the same option, but I
> wonder if have a separate warning option for them might be
> preferable (e.g., -Wformat-trunc=[01] or something like that).
> Independently, it might be useful to differentiate between
> truncating calls that check the return value and those that
> don't.
>
> Besides the usual testing I compiled several packages with the
> warning. If found a few bugs in boundary cases in Binutils that
> are being fixed.
>
> Thanks
> Martin
>
> PS There are a few FIXME notes in the patch that I will either
> fix or remove, depending on feedback, before committing the
> patch.
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
More information about the Gcc-patches
mailing list