On 2025-03-29 Pali Rohár wrote:
> On Saturday 29 March 2025 12:19:19 Lasse Collin wrote:
> > On 2025-03-28 Pali Rohár wrote:
> > > On Saturday 22 March 2025 15:35:48 Lasse Collin wrote:
> > > > If I build with "clang -O2 -fsanitize=cfi -flto" then the
> > > > program terminates with an illegal instruction instead.
> > >
> > > Ah, that is bad :-( Is not there some attribute to mark the
> > > function to add either structure name alias or turn off the
> > > runtime check for particular function?
> >
> > I tried __attribute__((no_sanitize("cfi"))) and a few related
> > attributes. These work if used in the calling code, for example, on
> > the main() function in the above example. They don't help if
> > applied to the foo() declaration or definition. That is, the
> > attributes don't seem helpful here. Even if an attribute worked, it
> > would poke a small hole in the CFI protection.
>
> Would it help if we compile stat*.c mingw-w64 source files with
> -fno-lto option? Or this also does not work?
I think it wouldn't help. -fsanitize=cfi requires specifying also
-flto. On the other hand, -fsanitize=function or -fsanitize=undefined
don't require -flto.
> > The sanitizer issue should be easy enough to fix properly. Each
> > stat*.c would, if needed, define _USE_32BIT_TIME_T and
> > _FILE_OFFSET_BITS, and then use "struct stat". That is, a similar
> > method as my ftw patch used.
>
> Problem is that you cannot define _USE_32BIT_TIME_T on UCRT builds.
> There are header ifdef checks that combination of _USE_32BIT_TIME_T
> and _UCRT throw preprocessor #error. But UCRT has _stat32 and also
> _stat32i64 functions. So this would not work.
I might not fully understand how much this matters.
- Currently one doesn't need to build any stat*.c files for UCRT. The
.def file redirects statxxx to _statxxx.
- The way I see it, having the fstat/stat/wstat symbols matters mostly
for Autoconf checks. Normal applications rely on <sys/stat.h>,
and then the default symbol doesn't matter.
<ftw.h> uses "struct stat", not any underscore version. When apps
cannot get 32-bit time_t variants of "struct stat" from <sys/stat.h>,
then there should be no need to have such ftw/nftw functions either.
The silly exception is that one still needs ftw and nftw symbols so
that Autoconf can find them, but no real program should call ftw or nftw
without including <ftw.h>. So in this sense I suppose it doesn't matter
much what function is used to provide the default symbols; they could
even be stub functions.
I'm probably missing something important. Sorry.
> Strict aliasing issue on particular type could be probably fixed by
> the __attribute__((__may_alias__)).
It seems to work even with structs. The attribute *must* be somewhere
after the "struct" keyword like this:
struct __attribute__((__may_alias__)) foo { int x; };
Maybe this could be the way go to be sure that strict aliasing with the
struct pointer casts won't cause problems. (It doesn't make a difference
to the -fsanitize issue.)
> > There would also need to be fifth stat*.c file to provide a stat64()
> > function that uses "struct stat64". Other than the argument type, it
> > would be ABI compatible with regular stat64(). Assuming that
> > -fsanitize isn't a worry when directly calling MS CRT functions,
> > only non-UCRT builds would need the fifth stat*.c file.
> >
> > #ifdef _UCRT
> > // stat64 is redirected in the .def file.
> > int __cdecl stat64(const char *_Filename, struct stat64 *_Stat);
> > #else
> > // Wrapper that converts the argument type to keep
> > // -fsanitize=function and -fsanitize=cfi happy.
> > int __cdecl stat64(const char *_Filename, struct stat64 *_Stat)
> > __MINGW_ASM_CALL(__mingw_lfs_stat64);
> > #endif
>
> This is for sure wrong. UCRT and msvcrt has same ABI for stat*
> functions. So any such ifdef looks very suspicious. Why two libraries
> with same ABI requires differences in the header?
My thinking was that with UCRT one can cheat: stat64 is handled in a
.def file, and thus -fsanitize would not know that it actually takes a
pointer to "struct _stat64" instead of "struct stat64". A sanitizer
would only be able to use the information it sees in the header file.
With MSVCRT, stat64 comes from stat64.c, and if that is built with
some -fsanitize, then the type mismatch between <sys/stat.h> and
stat64.c would be detected. The problem would be stat() when it's
redirected to stat64() because stat64.c expects "struct stat64 *" while
stat() uses "struct stat *".
I only understand the high level concept. It's clear that things work
if the function argument typenames always match. I don't understand
well in which situations it's possible to cheat. It's still unclear how
much this even matters *in practice* with mingw-w64 at the moment.
Sorry, I'm not super helpful with this detail. :-/
--
Lasse Collin
_______________________________________________
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
