linux-kernel - Re: [GIT PULL] ext4 changes for the 6.4 merge window

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKwvOdn3Unm94UCiXygWTM_KyhATNsy68b_CFbqBDFXshd+34Q@mail.gmail.com>
Date:   Wed, 26 Apr 2023 15:07:48 -0700
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     "Theodore Ts'o" <tytso@....edu>,
        Nathan Chancellor <nathan@...nel.org>,
        linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org
Subject: Re: [GIT PULL] ext4 changes for the 6.4 merge window

On Wed, Apr 26, 2023 at 11:33 AM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> On Wed, Apr 26, 2023 at 11:22 AM Nick Desaulniers
> <ndesaulniers@...gle.com> wrote:
> >
> > Ah, it does do something in the callee, not the caller:
>
> Ack, it does seem to have _some_ meaning for the return case, just not
> the one we'd be looking for as a way to find mistakes in the
> error-pointer case.

Is this what you had in mind?
```
$ cat linus.c
#define NULL ((void*)0)

void * _Nonnull foo (void) {
    return &foo;
}

void bar (void) {
    if (foo() == NULL) // maybe should warn that foo() returns _Nonnull?
        bar();
}
$ clang linus.c -fsyntax-only
linus.c:8:15: warning: comparison of _Nonnull function call 'foo'
equal to a null pointer is always false
[-Wtautological-pointer-compare]
    if (foo() == NULL) // maybe should warn that foo() returns _Nonnull?
              ^
linus.c:3:1: note: return type has '_Nonnull' nullability attribute
void * _Nonnull foo (void) {
^
1 warning generated.
```

Quick PoC, obviously incomplete.
```
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 18a0154b0041..10e405b1cf65 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3975,8 +3975,9 @@ def note_xor_used_as_pow_silence : Note<
   "replace expression with '%0' %select{|or use 'xor' instead of '^'
}1to silence this warning">;

 def warn_null_pointer_compare : Warning<
-    "comparison of %select{address of|function|array}0 '%1' %select{not |}2"
-    "equal to a null pointer is always %select{true|false}2">,
+    "comparison of %select{address of|function|array|_Nonnull function call}0 "
+    "'%1' %select{not |}2equal to a null pointer is always "
+    "%select{true|false}2">,
     InGroup<TautologicalPointerCompare>;
 def warn_nonnull_expr_compare : Warning<
     "comparison of nonnull %select{function call|parameter}0 '%1' "
@@ -3992,6 +3993,8 @@ def warn_address_of_reference_null_compare : Warning<
   "code; comparison may be assumed to always evaluate to "
   "%select{true|false}0">,
   InGroup<TautologicalUndefinedCompare>;
+def note_return_type_nonnull :
+  Note<"return type has '_Nonnull' nullability attribute">;
 def note_reference_is_return_value : Note<"%0 returns a reference">;

 def note_pointer_declared_here : Note<
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index f66eb9fcf13d..9f6d326f5b72 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -13176,6 +13176,22 @@ static void AnalyzeImpConvsInComparison(Sema
&S, BinaryOperator *E) {
 ///
 /// \param E the binary operator to check for warnings
 static void AnalyzeComparison(Sema &S, BinaryOperator *E) {
+  if (auto Call = dyn_cast<CallExpr>(E->getLHS())) {
+    QualType RetType = Call->getCallReturnType(S.Context);
+    if (std::optional<NullabilityKind> NK = RetType->getNullability()) {
+      if (*NK == NullabilityKind::NonNull &&
+        E->getRHS()->isNullPointerConstant(S.Context,
+
Expr::NPC_ValueDependentIsNotNull)) {
+        std::string result;
+        llvm::raw_string_ostream os(result);
+        Call->getDirectCallee()->getNameForDiagnostic(os,
S.getLangOpts(), true);
+        S.Diag(E->getExprLoc(), diag::warn_null_pointer_compare) << 3 <<
+          result << true;
+        S.Diag(Call->getDirectCallee()->getReturnTypeSourceRange().getBegin(),
+               diag::note_return_type_nonnull);
+      }
+    }
+  }
   // The type the comparison is being performed in.
   QualType T = E->getLHS()->getType();


```


-- 
Thanks,
~Nick Desaulniers