diff mbox series

debug: Fix fortified realpath C++ ODR violation (BZ 30516)

Message ID 20230703154833.3529850-1-adhemerval.zanella@linaro.org
State New
Headers show
Series debug: Fix fortified realpath C++ ODR violation (BZ 30516) | expand

Commit Message

Adhemerval Zanella Netto July 3, 2023, 3:48 p.m. UTC 
Although fortify wrappers are built with always_inline attribute,
C++ modules imports a (precompiled) header module foo.pcm generated from
foo.h which may differ if the translation unit includes or not limit.h.

Although GCC does not fully support C++ modules yet, this is a
problem when using C++ modules with clang.

The build check is removed and the runtime check is improved with
a better error message.

Checked on x86_64-linux-gnu.
---
 debug/realpath_chk.c |  5 +++--
 stdlib/bits/stdlib.h | 10 ----------
 2 files changed, 3 insertions(+), 12 deletions(-)

Comments

Florian Weimer July 3, 2023, 4:03 p.m. UTC | #1 
* Adhemerval Zanella via Libc-alpha:

> @@ -41,10 +35,6 @@ __NTH (realpath (const char *__restrict __name, char *__restrict __resolved))
>    if (sz == (size_t) -1)
>      return __realpath_alias (__name, __resolved);
>  
> -#if defined _LIBC_LIMITS_H_ && defined PATH_MAX
> -  if (__glibc_unsafe_len (PATH_MAX, sizeof (char), sz))
> -    return __realpath_chk_warn (__name, __resolved, sz);
> -#endif
>    return __realpath_chk (__name, __resolved, sz);
>  }

The issue is tje _LIBC_LIMITS_H_ conditional, right?

I suggest we just define PATH_MAX to the correct value in a Linux header
(in a namespace-clean fashion), and add a check of that constant against
the Linux UAPI value.  This constant is ABI anyway, and it's good to
know if the kernel decides to change it underneath us.

Regressing C diagnostics for experimental C++ module support does not
seem the right trade-off, sorry.

Thanks,
Florian
Adhemerval Zanella Netto July 3, 2023, 4:50 p.m. UTC | #2 
On 03/07/23 13:03, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> @@ -41,10 +35,6 @@ __NTH (realpath (const char *__restrict __name, char *__restrict __resolved))
>>    if (sz == (size_t) -1)
>>      return __realpath_alias (__name, __resolved);
>>  
>> -#if defined _LIBC_LIMITS_H_ && defined PATH_MAX
>> -  if (__glibc_unsafe_len (PATH_MAX, sizeof (char), sz))
>> -    return __realpath_chk_warn (__name, __resolved, sz);
>> -#endif
>>    return __realpath_chk (__name, __resolved, sz);
>>  }
> 
> The issue is tje _LIBC_LIMITS_H_ conditional, right?
> 
> I suggest we just define PATH_MAX to the correct value in a Linux header
> (in a namespace-clean fashion), and add a check of that constant against
> the Linux UAPI value.  This constant is ABI anyway, and it's good to
> know if the kernel decides to change it underneath us.
> 
> Regressing C diagnostics for experimental C++ module support does not
> seem the right trade-off, sorry.

Ok, this makes sense.   I will send an updated version.
diff mbox series

Patch

diff --git a/debug/realpath_chk.c b/debug/realpath_chk.c
index adfc09237c..71a7e3ded3 100644
--- a/debug/realpath_chk.c
+++ b/debug/realpath_chk.c
@@ -16,17 +16,18 @@ 
    <https://www.gnu.org/licenses/>.  */
 
 #include <limits.h>
+#include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 
-
 char *
 __realpath_chk (const char *buf, char *resolved, size_t resolvedlen)
 {
 #ifdef PATH_MAX
   if (resolvedlen < PATH_MAX)
-    __chk_fail ();
+    __fortify_fail ("second argument of realpah must be either NULL or at "
+		    "least PATH_MAX bytes long buffer");
 
   return __realpath (buf, resolved);
 #else
diff --git a/stdlib/bits/stdlib.h b/stdlib/bits/stdlib.h
index c6c0082ad5..6c8854cab0 100644
--- a/stdlib/bits/stdlib.h
+++ b/stdlib/bits/stdlib.h
@@ -26,12 +26,6 @@  extern char *__realpath_chk (const char *__restrict __name,
 extern char *__REDIRECT_NTH (__realpath_alias,
 			     (const char *__restrict __name,
 			      char *__restrict __resolved), realpath) __wur;
-extern char *__REDIRECT_NTH (__realpath_chk_warn,
-			     (const char *__restrict __name,
-			      char *__restrict __resolved,
-			      size_t __resolvedlen), __realpath_chk) __wur
-     __warnattr ("second argument of realpath must be either NULL or at "
-		 "least PATH_MAX bytes long buffer");
 
 __fortify_function __wur char *
 __NTH (realpath (const char *__restrict __name, char *__restrict __resolved))
@@ -41,10 +35,6 @@  __NTH (realpath (const char *__restrict __name, char *__restrict __resolved))
   if (sz == (size_t) -1)
     return __realpath_alias (__name, __resolved);
 
-#if defined _LIBC_LIMITS_H_ && defined PATH_MAX
-  if (__glibc_unsafe_len (PATH_MAX, sizeof (char), sz))
-    return __realpath_chk_warn (__name, __resolved, sz);
-#endif
   return __realpath_chk (__name, __resolved, sz);
 }