* explicit_bzero and -std=c99 [not found] ` <87tu2kps44.fsf@latte> @ 2022-11-27 11:27 ` Simon Josefsson via Gnulib discussion list 2022-11-27 17:48 ` Bruno Haible 2022-11-27 18:05 ` Paul Eggert 0 siblings, 2 replies; 7+ messages in thread From: Simon Josefsson via Gnulib discussion list @ 2022-11-27 11:27 UTC (permalink / raw) To: bug-gnulib [-- Attachment #1: Type: text/plain, Size: 2250 bytes --] Hi. We got a bug report about a build failure (see below), and I'm wondering: 1) Does gnulib support building with gcc -std=c99? I think we should, but it could have documented missing functionality or breakage. 2) It seems explicit_bzero.c in gnulib fall backs to using 'asm' for GCC, which isn't working in non-GNU modes of gcc. Further wondering: 1) The reason for having explicit_bzero is read_file, which needs it for reading sensitive files, a feature we don't use. Uncoupling this unnecessary dependency would have been nice. 2) Is there no other way to implement explicit_bzero without 'asm'? There is a another fallback code using volatile pointers, but I'm not sure it really has the same semantics. 3) Is there a way to detect if the compiler supports 'asm'? The current test 'defined __GNUC__ && !defined __clang__' is what is really failing here. 3) Is the idiom of using separate functions bzero() vs explicit_bzero() to avoid security-problematic compiler optimization still a good one? 1) If yes, I think we should have read_sensitive_file() rather than extending read_file() with a flag for this purpose. 2) If no, what is the better idiom to use here instead of explicit_bzero? Other thoughts? /Simon Simon Josefsson via Discussion list for GNU Libtasn1 <help-libtasn1@gnu.org> writes: > Vincent Fortier <th0ma7@gmail.com> writes: > >> While preparing a gnutls update I ended-up updating libtasn1 from >> 4.16. Going to 4.17 works but anything after that fails with: > > Thanks for the report! I can reproduce this using: > > ./configure ac_cv_func_explicit_bzero=no CPPFLAGS="-std=c99" > > In other words, the problem is due to a combination of a platform > without explicit_bzero and forcing GCC into C99 mode where it doesn't > support 'asm', so it is not possible to implement 'explicit_bzero'. Why > do you hard code -std=c99? Try just removing that. Or use -std=gnu99' > to allow GCC to use 'asm'. > > Analysing further, the 'explicit_bzero' function is only used by > 'read_file' in a mode that we never use (for reading sensitive files), > so it is never needed by libtasn1. This is not ideal, but I'm not sure > what a maintainable solution is. > > /Simon > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 255 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: explicit_bzero and -std=c99 2022-11-27 11:27 ` explicit_bzero and -std=c99 Simon Josefsson via Gnulib discussion list @ 2022-11-27 17:48 ` Bruno Haible 2022-11-28 10:32 ` Simon Josefsson via Gnulib discussion list 2022-11-27 18:05 ` Paul Eggert 1 sibling, 1 reply; 7+ messages in thread From: Bruno Haible @ 2022-11-27 17:48 UTC (permalink / raw) To: bug-gnulib; +Cc: Simon Josefsson, Vincent Fortier Hi Simon, > 1) Does gnulib support building with gcc -std=c99? I think we should, > but it could have documented missing functionality or breakage. No, Gnulib does not support this. We freely use GCC extensions, such as ({...}) or asm, usually conditionalized with defined __GNUC__ || defined __clang__ Only in math.in.h and xalloc-oversized.h we also test __STRICT_ANSI__. We could test __STRICT_ANSI__ also in more places, but what really is the point? So that people then complain "the asyncsafe-spin and simple-atomic tests fail for me"? The point of '-std=c99' is to verify that the source code is pure ISO C without any extensions. Gnulib is not in that category. > 2) It seems explicit_bzero.c in gnulib fall backs to using 'asm' for > GCC, which isn't working in non-GNU modes of gcc. In lib/explicit_bzero.c you see that we need to defeat compiler optimizations. 'asm' lets us formulate a memory barrier. Without 'asm' we have a fallback code that defeated compiler optimizations at the time it was written, but that will break in the long run as compilers are becoming better. > Further wondering: > > 1) The reason for having explicit_bzero is read_file, which needs it > for reading sensitive files, a feature we don't use. Uncoupling this > unnecessary dependency would have been nice. No, we have explicit_bzero because it's a glibc function that we think should be available to programs on all OSes. <https://www.gnu.org/software/gnulib/manual/html_node/explicit_005fbzero.html> > 2) Is there no other way to implement explicit_bzero without 'asm'? > There is a another fallback code using volatile pointers, but I'm not > sure it really has the same semantics. That's the point, exactly: Here 'asm' is necessary. > 3) Is there a way to detect if the compiler supports 'asm'? The > current test 'defined __GNUC__ && !defined __clang__' is what is > really failing here. Probably something like (defined __GNUC__ || defined __clang__) && !defined __STRICT_ANSI__ > 3) Is the idiom of using separate functions bzero() vs explicit_bzero() > to avoid security-problematic compiler optimization still a good one? > > 1) If yes, I think we should have read_sensitive_file() rather than > extending read_file() with a flag for this purpose. > > 2) If no, what is the better idiom to use here instead of > explicit_bzero? When the code for average contexts and the code for secure contexts differ only by a few lines of code, we would like to avoid code duplication. As code duplication means twice the maintenance effort in the future. Things would be different when you want the code for secure contexts to use exactly the same instructions (just with different data), regardless of the input data.[1] Since code for average contexts is typically written to minimize the number of instructions depending on the input data, this would imply different algorithms and different code. But we are not in this case here. Bruno [1] https://en.wikipedia.org/wiki/Timing_attack ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: explicit_bzero and -std=c99 2022-11-27 17:48 ` Bruno Haible @ 2022-11-28 10:32 ` Simon Josefsson via Gnulib discussion list 2022-11-29 14:49 ` Bruno Haible 0 siblings, 1 reply; 7+ messages in thread From: Simon Josefsson via Gnulib discussion list @ 2022-11-28 10:32 UTC (permalink / raw) To: Bruno Haible, Paul Eggert; +Cc: bug-gnulib [-- Attachment #1: Type: text/plain, Size: 4136 bytes --] Paul Eggert <eggert@cs.ucla.edu> writes: >> 2) It seems explicit_bzero.c in gnulib fall backs to using 'asm' for >> GCC, which isn't working in non-GNU modes of gcc. Further wondering: > > I hope I fixed this particular problem by installing the attached. Thank you! > Perhaps Gnulib's other uses of asm should also be changed? Yes I think we should '__asm__' instead of 'asm' for the reason explained by the gcc manual that Bruno linked to. >> 3) Is the idiom of using separate functions bzero() vs explicit_bzero() >> to avoid security-problematic compiler optimization still a good one? > > Yes If so, I would prefer a read_sensitive_file() API instead of read_file() with a flag to enable the security-sensitive functionality. I'll leave it for the future, as this the immediate problem is resolved. Bruno Haible <bruno@clisp.org> writes: >> 1) Does gnulib support building with gcc -std=c99? I think we should, >> but it could have documented missing functionality or breakage. > > No, Gnulib does not support this. We freely use GCC extensions, > such as ({...}) or asm, usually conditionalized with > defined __GNUC__ || defined __clang__ > Only in math.in.h and xalloc-oversized.h we also test __STRICT_ANSI__. > > We could test __STRICT_ANSI__ also in more places, but what really is the > point? So that people then complain "the asyncsafe-spin and simple-atomic > tests fail for me"? > > The point of '-std=c99' is to verify that the source code is pure ISO C > without any extensions. Gnulib is not in that category. Your answer is a bit different from Paul's, and both seems like reasonable approaches to me. This may be a situation where sometimes we make a small effort of being compatible with -std=c99 and sometimes decide against it. I think what could help is a bit more documentation about this problem. Building gnulib with -std=c99 and fixing some of the minor issues will likely help future compatibility of code, so I think we should make small efforts to comply. I agree that there is likely some parts of gnulib that simply don't work in C99-mode -- documenting what they are would be useful. In libtasn1, we want to support C89 environments since it is such a low-level and bootstrap-relevant library. At least for the library, the command-line tool doesn't have to be C89-compatible IMHO. >> 1) The reason for having explicit_bzero is read_file, which needs it >> for reading sensitive files, a feature we don't use. Uncoupling this >> unnecessary dependency would have been nice. > > No, we have explicit_bzero because it's a glibc function that we think > should be available to programs on all OSes. > <https://www.gnu.org/software/gnulib/manual/html_node/explicit_005fbzero.html> Sorry I was unclear: the reason for LIBTASN1 to have explicit_bzero is read_file. But libtasn1 never uses the sensitive flag, and thus never really excercise the explicit_bzero code path. >> 3) Is there a way to detect if the compiler supports 'asm'? The >> current test 'defined __GNUC__ && !defined __clang__' is what is >> really failing here. > > Probably something like > (defined __GNUC__ || defined __clang__) && !defined __STRICT_ANSI__ Using __asm__ instead seems more elegant, and even aligned with gcc manual. >> 3) Is the idiom of using separate functions bzero() vs explicit_bzero() >> to avoid security-problematic compiler optimization still a good one? >> >> 1) If yes, I think we should have read_sensitive_file() rather than >> extending read_file() with a flag for this purpose. >> >> 2) If no, what is the better idiom to use here instead of >> explicit_bzero? > > When the code for average contexts and the code for secure contexts differ > only by a few lines of code, we would like to avoid code duplication. As > code duplication means twice the maintenance effort in the future. Sure -- although it would be possible to implement the essence of read_file in a way where support for the sensitive flag is a compile-time option. /Simon [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 255 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: explicit_bzero and -std=c99 2022-11-28 10:32 ` Simon Josefsson via Gnulib discussion list @ 2022-11-29 14:49 ` Bruno Haible 0 siblings, 0 replies; 7+ messages in thread From: Bruno Haible @ 2022-11-29 14:49 UTC (permalink / raw) To: Simon Josefsson; +Cc: Paul Eggert, bug-gnulib Simon Josefsson wrote: > > Perhaps Gnulib's other uses of asm should also be changed? > > Yes I think we should '__asm__' instead of 'asm' for the reason > explained by the gcc manual that Bruno linked to. The remaining uses of 'asm' (as opposed to '__asm__') are in lib/simple-atomic.c lib/asyncsafe-spin.c and are for GCC versions < 4.7 (hardly in use nowadays) and for non-GCC compilers (for which I don't want to spend time to see whether they support '__asm__'). > If so, I would prefer a read_sensitive_file() API instead of read_file() > with a flag to enable the security-sensitive functionality. I'll leave > it for the future, as this the immediate problem is resolved. Minimizing module dependencies is a goal when that dependency is large or otherwise cumbersome. However, the dependency to 'memset_explicit' is small (produces a tiny .o file, even no .o file at all in the long term) and does not require any link options. Therefore the dependency 'read-file' -> 'memset_explicit' doesn't bother me a lot. Bruno ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: explicit_bzero and -std=c99 2022-11-27 11:27 ` explicit_bzero and -std=c99 Simon Josefsson via Gnulib discussion list 2022-11-27 17:48 ` Bruno Haible @ 2022-11-27 18:05 ` Paul Eggert 2022-11-27 18:08 ` Paul Eggert 2022-11-27 18:24 ` Bruno Haible 1 sibling, 2 replies; 7+ messages in thread From: Paul Eggert @ 2022-11-27 18:05 UTC (permalink / raw) To: Simon Josefsson; +Cc: Gnulib bugs [-- Attachment #1: Type: text/plain, Size: 1701 bytes --] On 2022-11-27 03:27, Simon Josefsson via Gnulib discussion list wrote: > 1) Does gnulib support building with gcc -std=c99? I think we should, > but it could have documented missing functionality or breakage. It should, yes. That's a reasonable portability test, so long as Gnulib continues to support C99. > 2) It seems explicit_bzero.c in gnulib fall backs to using 'asm' for > GCC, which isn't working in non-GNU modes of gcc. Further wondering: I hope I fixed this particular problem by installing the attached. Perhaps Gnulib's other uses of asm should also be changed? > 1) The reason for having explicit_bzero is read_file, which needs it > for reading sensitive files, a feature we don't use. Uncoupling this > unnecessary dependency would have been nice. In the long run it should be OK; see below. > 2) Is there no other way to implement explicit_bzero without 'asm'? > There is a another fallback code using volatile pointers, but I'm not > sure it really has the same semantics. That fallback should work, though it's a bit slower. > 3) Is there a way to detect if the compiler supports 'asm'? The > current test 'defined __GNUC__ && !defined __clang__' is what is > really failing here. We could add a configure-time test. Not sure it's worth the hassle. > 3) Is the idiom of using separate functions bzero() vs explicit_bzero() > to avoid security-problematic compiler optimization still a good one? Yes, though we should switch to memset_explicit as that's the name C23 has standardized on. I.e., create a memset_explicit module, have other modules use that instead of explicit_bzero. No rush, but that's the way to proceed. [-- Attachment #2: 0001-explicit_bzero-work-with-gcc-std-c99.patch --] [-- Type: text/x-patch, Size: 1286 bytes --] From 04191d1b325186fcd788a4a0a89274f8b9a9943b Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Sun, 27 Nov 2022 09:59:32 -0800 Subject: [PATCH] explicit_bzero: work with gcc -std=c99 * lib/explicit_bzero.c (explicit_bzero) [__GNUC__ && !__clang__]: Use __asm__ instead of asm. --- ChangeLog | 6 ++++++ lib/explicit_bzero.c | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 36825874d2..eedab2ae83 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2022-11-27 Paul Eggert <eggert@cs.ucla.edu> + + explicit_bzero: work with gcc -std=c99 + * lib/explicit_bzero.c (explicit_bzero) [__GNUC__ && !__clang__]: + Use __asm__ instead of asm. + 2022-11-26 Paul Eggert <eggert@cs.ucla.edu> Prefer "kill -INT" to killing with a number diff --git a/lib/explicit_bzero.c b/lib/explicit_bzero.c index ad0bfd170c..584f982924 100644 --- a/lib/explicit_bzero.c +++ b/lib/explicit_bzero.c @@ -57,7 +57,7 @@ explicit_bzero (void *s, size_t len) #elif defined __GNUC__ && !defined __clang__ memset (s, '\0', len); /* Compiler barrier. */ - asm volatile ("" ::: "memory"); + __asm__ volatile ("" ::: "memory"); #elif defined __clang__ memset (s, '\0', len); /* Compiler barrier. */ -- 2.37.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: explicit_bzero and -std=c99 2022-11-27 18:05 ` Paul Eggert @ 2022-11-27 18:08 ` Paul Eggert 2022-11-27 18:24 ` Bruno Haible 1 sibling, 0 replies; 7+ messages in thread From: Paul Eggert @ 2022-11-27 18:08 UTC (permalink / raw) To: bug-gnulib On 2022-11-27 10:05, Paul Eggert wrote: > It should, yes. That's a reasonable portability test, so long as Gnulib > continues to support C99. Ah, Bruno has corrected me on this. Although compiling with -std=c99 is OK for some parts of Gnulib, it's not OK for all. In this particular case the fix is easy so I guess it's OK. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: explicit_bzero and -std=c99 2022-11-27 18:05 ` Paul Eggert 2022-11-27 18:08 ` Paul Eggert @ 2022-11-27 18:24 ` Bruno Haible 1 sibling, 0 replies; 7+ messages in thread From: Bruno Haible @ 2022-11-27 18:24 UTC (permalink / raw) To: Paul Eggert; +Cc: Simon Josefsson, bug-gnulib Paul Eggert wrote: > > 2) Is there no other way to implement explicit_bzero without 'asm'? > > There is a another fallback code using volatile pointers, but I'm not > > sure it really has the same semantics. > > That fallback should work, though it's a bit slower. I'm afraid that the fallback code /* Invoke memset through a volatile function pointer. This defeats compiler optimizations. */ void * (* const volatile volatile_memset) (void *, int, size_t) = memset; (void) volatile_memset (s, '\0', len); will stop working, as compilers get "smarter". The other code based on asm is not so likely to break, since it is documented that "Using the "memory" clobber effectively forms a read/write memory barrier for the compiler." [1] Bruno [1] https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gcc/Extended-Asm.html ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-11-29 14:50 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CALAySuJi-gG53FDojZtMwZ9e254o3LbdAxnjZA6esN3724ZXLg@mail.gmail.com> [not found] ` <87tu2kps44.fsf@latte> 2022-11-27 11:27 ` explicit_bzero and -std=c99 Simon Josefsson via Gnulib discussion list 2022-11-27 17:48 ` Bruno Haible 2022-11-28 10:32 ` Simon Josefsson via Gnulib discussion list 2022-11-29 14:49 ` Bruno Haible 2022-11-27 18:05 ` Paul Eggert 2022-11-27 18:08 ` Paul Eggert 2022-11-27 18:24 ` Bruno Haible
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).