Page MenuHomeFreeBSD

remove %n support from printf(9)
ClosedPublic

Authored by emaste on May 8 2020, 2:15 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 20, 9:33 AM
Unknown Object (File)
Wed, Nov 20, 9:17 AM
Unknown Object (File)
Wed, Nov 20, 7:13 AM
Unknown Object (File)
Sun, Nov 17, 2:45 PM
Unknown Object (File)
Fri, Oct 25, 3:51 PM
Unknown Object (File)
Sep 18 2024, 6:32 AM
Unknown Object (File)
Sep 17 2024, 4:45 PM
Unknown Object (File)
Sep 17 2024, 4:45 PM

Details

Summary

It can be dangerous and there is no need for it in the kernel. Inspired by Kees Cook's change in Linux, and later OpenBSD.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

emaste requested review of this revision.May 8 2020, 2:15 PM
emaste created this revision.
This revision is now accepted and ready to land.May 8 2020, 3:25 PM

Have we checked to see how often this is used in tree?

Have we checked to see how often this is used in tree?

It's not. grepping sys turns up a couple of sscanfs and a bunch of unrelated %ns.

philip added a subscriber: philip.

I double-checked @emaste's grep and likewise see no consumer of %n in callers of kernel printf(9).

Looks good to me. I haven't tested it, but seeing as it is the same patch as OpenBSD's it should do what is expected.

This revision was automatically updated to reflect the committed changes.

Can we hack clang to warn? Or enable a clang option?

Or noisily complain / panic?

In D24760#545321, @imp wrote:

Or noisily complain / panic?

I think this should do it:

--- a/contrib/llvm-project/clang/lib/AST/PrintfFormatString.cpp
+++ b/contrib/llvm-project/clang/lib/AST/PrintfFormatString.cpp
@@ -316,8 +316,8 @@ static PrintfSpecifierResult ParsePrintfSpecifier(FormatStringHandler &H,
     case 'g': k = ConversionSpecifier::gArg; break;
     case 'i': k = ConversionSpecifier::iArg; break;
     case 'n':
-      // Not handled, but reserved in OpenCL.
-      if (!LO.OpenCL)
+      // Not handled, but reserved in OpenCL and FreeBSD kernel.
+      if (!LO.OpenCL && !isFreeBSDKPrintf)
         k = ConversionSpecifier::nArg;
       break;
     case 'o': k = ConversionSpecifier::oArg; break;