Re: %z for printf on mingw

[Bruno Haible]

Hi Reuben,

> gnulib can #define __USE_MINGW_ANSI_STDIO so that %z is implemented, but
> warnings are still generated for xasprintf (not for printf).
> 
> As far as I can tell, this is because the
> _GL_ATTRIBUTE_FORMAT_PRINTF_SYSTEM machinery to choose the correct
> attribute (__gnu_printf__ or __printf__) for printf-like functions needs to
> be used in xvasprintf.h and hence vasnprintf.h as well as in stdio.h.
> 
> I guess this applies to all printf-like functions in gnulib that ultimately
> use the system printf, that is, in verror.h, xprintf.h and argp.h (which
> depend on xvasprintf or stdio).

Indeed, we did not really pay attention to the distinction between
__gnu_printf__ (which denotes standards-compliant format strings) and
__printf__ (which denotes the MSVCRT format strings).

The interfaces in Gnulib fall in three categories:

* Those that take standards-compliant format strings:
  c_snprintf -> c_vasnprintf -> vasnprintf
  c_vasnprintf -> vasnprintf
  c_asprintf -> c_vasprintf -> c_vasnprintf -> vasnprintf
  c_vasprintf -> c_vasnprintf -> vasnprintf
  c_vsnprintf -> c_vasnprintf -> vasnprintf
  c_xasprintf -> c_xvasprintf -> c_vasprintf -> c_vasnprintf -> vasnprintf
  c_xvasprintf -> c_vasprintf -> c_vasnprintf -> vasnprintf
  verror, verror_at_line -> xvasprintf -> vasprintf -> vasnprintf
  asnprintf -> vasnprintf
  vasnprintf
  xasprintf -> xvasprintf -> vasprintf -> vasnprintf
  xvasprintf -> vasprintf -> vasnprintf
  ostream_printf -> vasprintf -> vasnprintf

* Those that use vfprintf and therefore take whatever the vfprintf function
  takes:
  argp_error, argp_failure -> vfprintf
  error, error_at_line -> vfprintf
  xfprintf -> xvfprintf -> vfprintf
  xvfprintf -> vfprintf

* Those that use vprintf and therefore take whatever the vprintf function
  takes:
  xprintf -> xvprintf -> vprintf
  xvprintf -> vprintf

This patch corrects the attributes.

I'm not really happy that the API does not use standards-compliant format
strings everywhere. But I'm not sure it's worth fixing.

Note: You may get warnings when you use format strings that contain the
PRI* macros from . This is because the mingw headers do ugly
things with these macros, and there are no two PRIdMAX macros (one for
__gnu_printf__ and one for __printf__) but only one.


2020-11-23  Bruno Haible  

Use the correct printf format attribute for mingw.
Reported by Reuben Thomas  in
.

* modules/vfprintf-posix (configure.ac): Define GNULIB_VFPRINTF_POSIX.
* modules/vprintf-posix (configure.ac): Define GNULIB_VPRINTF_POSIX.

* lib/stdio.in.h (_GL_ATTRIBUTE_SPEC_PRINTF_STANDARD,
_GL_ATTRIBUTE_SPEC_PRINTF_SYSTEM): New macros.
(_GL_ATTRIBUTE_FORMAT_PRINTF_STANDARD): Renamed from
_GL_ATTRIBUTE_FORMAT_PRINTF. Use _GL_ATTRIBUTE_SPEC_PRINTF_STANDARD.
(_GL_ATTRIBUTE_FORMAT_PRINTF_SYSTEM): Use
_GL_ATTRIBUTE_SPEC_PRINTF_SYSTEM.

* modules/vasnprintf (Depends-on): Add stdio.
* lib/vasnprintf.h: Include .
(asnprintf, vasnprintf): Use the standard printf format attribute.

* modules/xvasprintf (Depends-on): Add stdio.
* lib/xvasprintf.h: Include .
(xasprintf, xvasprintf): Use the standard printf format attribute.

* modules/xprintf (Depends-on): List stdio first.
* lib/xprintf.h (xprintf, xvprintf): Use a printf format attribute that
depends on GNULIB_VPRINTF_POSIX.
(xfprintf, xvfprintf): Use a printf format attribute that depends on
GNULIB_VFPRINTF_POSIX.

* modules/c-vasnprintf (Depends-on): Add stdio.
* lib/c-vasnprintf.h: Include .
(c_vasnprintf): Use the standard printf format attribute.

* modules/c-vasprintf (Depends-on): Add stdio.
* lib/c-vasprintf.h: Include .
(c_asprintf, c_vasprintf): Use the standard printf format attribute.

* modules/c-vsnprintf (Depends-on): Add stdio.
* lib/c-vsnprintf.h: Include .
(c_vsnprintf): Use the standard printf format attribute.

* modules/c-snprintf (Depends-on): Add stdio.
* lib/c-snprintf.h: Include .
(c_snprintf): Use the standard printf format attribute.

* modules/c-xvasprintf (Depends-on): Add stdio.
* lib/c-xvasprintf.h: Include .
(c_xasprintf, c_xvasprintf): Use the standard printf format attribute.

* modules/error (Depends-on): Depend on stdio always.
* lib/error.h: Include .
(_GL_ATTRIBUTE_SPEC_PRINTF): Remove macro.
(error, error_at_line): Use a printf format attribute that depends on
GNULIB_VFPRINTF_POSIX.
* lib/error.c (_GL_ATTRIBUTE_FORMAT_PRINTF_STANDARD): Renamed from
_GL_ATTRIBUTE_FORMAT_PRINTF.

* modules/verror (Depends-on): Add stdio.
* lib/verror.h: Include . Don't include "error.h".
(verror, verror_at_line): Use the standard printf format 

Re: Signals on Mingw

[Reuben Thomas]

On Mon, 23 Nov 2020 at 17:08, Reuben Thomas  wrote:

> Good question! Looks like the code is relying on the definitions matching.
> (I used one of the sample debugger stubs, which seems not to have been
> changed for many years; meanwhile, gdb seems to have introduced its own
> enumeration. I guess I'll look into this again at some point.) It doesn't
> seem to matter much anyway, as GDB's remote stub communicator only seems to
> ever use two concrete values. I guess the rest are just indicative. TBH
> it's several months since I worked on the code and it was my first time
> hacking inside GDB!
>

Yes, looks like the example code is outdated, and I should be using
include/gdb/signals.h rather than native signal.h. Thanks for raising this
question, Eli! This means in any case that I don't have a use for these
signal numbers on Windows.

-- 
https://rrt.sc3d.org

Re: Signals on Mingw

[Reuben Thomas]

On Mon, 23 Nov 2020 at 16:47, Eli Zaretskii  wrote:

> > From: Reuben Thomas 
> > Date: Mon, 23 Nov 2020 16:38:22 +
> > Cc: Bruno Haible , bug-gnulib 
> >
> >  Can you elaborate what are you using this module for in the MinGW
> >  build?  AFAIK, Posix signals can never work well enough on Windows to
> >  care about them.  Maybe I'm missing something.
> >
> > Signal numbers for use in a GDB debugger stub: stubs report
> errors/breaks etc to GDB as signal numbers.
>
> So all you need is signal numbers?
>

That's right.

But why is it useful to report signals when none were actually
> delivered to the program?


Because that's how GDB works: it uses signal numbers to represent CPU traps
such as page faults and breakpoints.


>   And how do you make sure the numbers you
> report will be identical to what GDB itself uses?
>

Good question! Looks like the code is relying on the definitions matching.
(I used one of the sample debugger stubs, which seems not to have been
changed for many years; meanwhile, gdb seems to have introduced its own
enumeration. I guess I'll look into this again at some point.) It doesn't
seem to matter much anyway, as GDB's remote stub communicator only seems to
ever use two concrete values. I guess the rest are just indicative. TBH
it's several months since I worked on the code and it was my first time
hacking inside GDB!

-- 
https://rrt.sc3d.org

Re: Signals on Mingw

[Eli Zaretskii]

> From: Reuben Thomas 
> Date: Mon, 23 Nov 2020 16:38:22 +
> Cc: Bruno Haible , bug-gnulib 
> 
>  Can you elaborate what are you using this module for in the MinGW
>  build?  AFAIK, Posix signals can never work well enough on Windows to
>  care about them.  Maybe I'm missing something.
> 
> Signal numbers for use in a GDB debugger stub: stubs report errors/breaks etc 
> to GDB as signal numbers.

So all you need is signal numbers?

But why is it useful to report signals when none were actually
delivered to the program?  And how do you make sure the numbers you
report will be identical to what GDB itself uses?

Re: Signals on Mingw

[Reuben Thomas]

On Mon, 23 Nov 2020 at 16:09, Eli Zaretskii  wrote:

>
> Can you elaborate what are you using this module for in the MinGW
> build?  AFAIK, Posix signals can never work well enough on Windows to
> care about them.  Maybe I'm missing something.
>

Signal numbers for use in a GDB debugger stub: stubs report errors/breaks
etc to GDB as signal numbers.

-- 
https://rrt.sc3d.org

Re: Signals on Mingw

[Eli Zaretskii]

> From: Reuben Thomas 
> Date: Sun, 22 Nov 2020 23:46:37 +
> Cc: bug-gnulib 
> 
>- Is it useful to have these signal names defined at all? If they can never
>  occur on native Windows, it does not necessarily make sense to define 
> them.
> 
> If I wanted native (non-POSIX) functionality, I would not have used the 
> signal-h module.
> 
> However, it seems _POSIX, when it was used in MSVC, may refer to Windows's 
> old POSIX subsystem:
> https://sourceforge.net/p/mingw-w64/mailman/message/33014416/
>  
>  Also consider the workarounds that Gnulib already does, in
>  doc/posix-headers/signal.texi .
> 
> I'm already using this, of course! It defines SIGPIPE (but not other missing 
> signals).
> 
> signal.texi claims that sigset_t will be defined on Windows, but it does this 
> just by #including ,
> which does not define sigset_t unless _POSIX is defined.
> 
> The only mention I can find of _POSIX in gnulib is in 
> doc/posix-functions/getlogin.texi, which mentions that
> getlogin is only defined on mingw if _POSIX is defined (as I noted above). 
> The solution of the getlogin module
> is to use the unistd module to declare getlogin, if I understand correctly.

Can you elaborate what are you using this module for in the MinGW
build?  AFAIK, Posix signals can never work well enough on Windows to
care about them.  Maybe I'm missing something.

[PATCH] selinux-at, selinux-h: use const correct declarations

[Pádraig Brady]

* lib/se-selinux.in.h: Use const for "set" functions,
to match current selinux, and support cleaner user code.
* lib/selinux-at.c: Likewise.
* lib/selinux-at.h: Likewise.
---
 ChangeLog   |  8 
 lib/se-selinux.in.h | 18 +-
 lib/selinux-at.c|  4 ++--
 lib/selinux-at.h|  4 ++--
 4 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 044b12d8c..a17f2b208 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2020-11-23  Pádraig Brady  
+
+   selinux-at, selinux-h: use const correct declarations
+   * lib/se-selinux.in.h: Use const for "set" functions,
+   to match current selinux, and support cleaner user code.
+   * lib/selinux-at.c: Likewise.
+   * lib/selinux-at.h: Likewise.
+
 2020-11-22  Paul Eggert  
 
canonicalize-lgpl: fix memory leak
diff --git a/lib/se-selinux.in.h b/lib/se-selinux.in.h
index a6c194aa0..67d034d0f 100644
--- a/lib/se-selinux.in.h
+++ b/lib/se-selinux.in.h
@@ -56,7 +56,7 @@ SE_SELINUX_INLINE int
 getfscreatecon (char **con _GL_UNUSED_PARAMETER)
   { errno = ENOTSUP; return -1; }
 SE_SELINUX_INLINE int
-setfscreatecon (char *con _GL_UNUSED_PARAMETER)
+setfscreatecon (char const *con _GL_UNUSED_PARAMETER)
   { errno = ENOTSUP; return -1; }
 SE_SELINUX_INLINE int
 matchpathcon (char const *file _GL_UNUSED_PARAMETER,
@@ -76,29 +76,29 @@ fgetfilecon (int fd, char **con _GL_UNUSED_PARAMETER)
   { errno = ENOTSUP; return -1; }
 SE_SELINUX_INLINE int
 setfilecon (char const *file _GL_UNUSED_PARAMETER,
-char *con _GL_UNUSED_PARAMETER)
+char const *con _GL_UNUSED_PARAMETER)
   { errno = ENOTSUP; return -1; }
 SE_SELINUX_INLINE int
 lsetfilecon (char const *file _GL_UNUSED_PARAMETER,
- char *con _GL_UNUSED_PARAMETER)
+ char const *con _GL_UNUSED_PARAMETER)
   { errno = ENOTSUP; return -1; }
 SE_SELINUX_INLINE int
 fsetfilecon (int fd _GL_UNUSED_PARAMETER,
- char *con _GL_UNUSED_PARAMETER)
+ char const *con _GL_UNUSED_PARAMETER)
   { errno = ENOTSUP; return -1; }
 
 SE_SELINUX_INLINE int
-security_check_context (char *con _GL_UNUSED_PARAMETER)
+security_check_context (char const *con _GL_UNUSED_PARAMETER)
   { errno = ENOTSUP; return -1; }
 SE_SELINUX_INLINE int
-security_check_context_raw (char *con _GL_UNUSED_PARAMETER)
+security_check_context_raw (char const *con _GL_UNUSED_PARAMETER)
   { errno = ENOTSUP; return -1; }
 SE_SELINUX_INLINE int
-setexeccon (char *con _GL_UNUSED_PARAMETER)
+setexeccon (char const *con _GL_UNUSED_PARAMETER)
   { errno = ENOTSUP; return -1; }
 SE_SELINUX_INLINE int
-security_compute_create (char *scon _GL_UNUSED_PARAMETER,
- char *tcon _GL_UNUSED_PARAMETER,
+security_compute_create (char const *scon _GL_UNUSED_PARAMETER,
+ char const *tcon _GL_UNUSED_PARAMETER,
  security_class_t tclass _GL_UNUSED_PARAMETER,
  char **newcon _GL_UNUSED_PARAMETER)
   { errno = ENOTSUP; return -1; }
diff --git a/lib/selinux-at.c b/lib/selinux-at.c
index 105a9f9d5..e1d214c2a 100644
--- a/lib/selinux-at.c
+++ b/lib/selinux-at.c
@@ -52,7 +52,7 @@
 
 #define AT_FUNC_NAME setfileconat
 #define AT_FUNC_F1 setfilecon
-#define AT_FUNC_POST_FILE_PARAM_DECLS , char *con
+#define AT_FUNC_POST_FILE_PARAM_DECLS , char const *con
 #define AT_FUNC_POST_FILE_ARGS, con
 #include "at-func.c"
 #undef AT_FUNC_NAME
@@ -62,7 +62,7 @@
 
 #define AT_FUNC_NAME lsetfileconat
 #define AT_FUNC_F1 lsetfilecon
-#define AT_FUNC_POST_FILE_PARAM_DECLS , char *con
+#define AT_FUNC_POST_FILE_PARAM_DECLS , char const *con
 #define AT_FUNC_POST_FILE_ARGS, con
 #include "at-func.c"
 #undef AT_FUNC_NAME
diff --git a/lib/selinux-at.h b/lib/selinux-at.h
index 50537f80f..9b331cb18 100644
--- a/lib/selinux-at.h
+++ b/lib/selinux-at.h
@@ -42,11 +42,11 @@ int lgetfileconat (int dir_fd, char const *file, char 
**con);
the file specified by DIR_FD and FILE to CON.  DIR_FD and FILE are
interpreted as for fstatat[*].  Upon success, return 0.
Otherwise, return -1 and set errno.  */
-int  setfileconat (int dir_fd, char const *file, char *con);
+int  setfileconat (int dir_fd, char const *file, char const *con);
 
 /* dir-fd-relative lsetfilecon.  This function is just like setfileconat,
except that rather than dereferencing a symlink, this function affects it. 
*/
 /* dir-fd-relative lsetfilecon.  This function is just like setfileconat,
except when DIR_FD and FILE specify a symlink:  lsetfileconat operates on
the symlink, while setfileconat operates on the referent of the symlink.  */
-int lsetfileconat (int dir_fd, char const *file, char *con);
+int lsetfileconat (int dir_fd, char const *file, char const *con);
-- 
2.26.2

Re: [PATCH] selinux-h: add stubs for selabel_open etc.

[Bernhard Voelker]

On 11/23/20 3:26 AM, Paul Eggert wrote:
> On 11/22/20 10:59 AM, Bernhard Voelker wrote:
>> selinux.c:257 has a superfluous semicolon after a jump label,
>> and a strange indentation:
> 
> The semicolon is required by the C standard, which does not allow a label 
> before 
> a declaration. Emacs indented it that way.

ah, I missed that, thanks!

Have a nice day,
Berny

Re: [PATCH] selinux-h: add stubs for selabel_open etc.

[Kamil Dudka]

On Monday, November 23, 2020 10:49:57 AM CET Paul Eggert wrote:
> Thanks, I think I see the problem. I installed the attached to try to fix it.

Yes, this made the test-suite green again.  Thanks!

Kamil

Re: [PATCH] selinux-h: add stubs for selabel_open etc.

[Paul Eggert]

On 11/23/20 1:15 AM, Kamil Dudka wrote:

 ...: context lookup failed: Operation not supported


Thanks, I think I see the problem. I installed the attached to try to fix it.
From e3a96eb14e8834f046d8370db80dfdc561ef5550 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Mon, 23 Nov 2020 01:48:15 -0800
Subject: [PATCH] install: suppress "Operation not supported" false alarms
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

At least, I *think* they are false alarms.  An SELinux expert eye
would be welcome.
* src/install.c (setdefaultfilecon): If selabel_lookup fails
due to either ENOTSUP or ENODATA, don’t diagnose the issue.
Problem reported by Kamil Dudka in:
https://lists.gnu.org/r/coreutils/2020-11/msg00050.html
---
 src/install.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/install.c b/src/install.c
index eb6e403e7..dce29dbe1 100644
--- a/src/install.c
+++ b/src/install.c
@@ -339,7 +339,7 @@ setdefaultfilecon (char const *file)
 return;
   if (selabel_lookup (hnd, , file, st.st_mode) != 0)
 {
-  if (errno != ENOENT)
+  if (errno != ENOENT && ! ignorable_ctx_err (errno))
 error (0, errno, _("warning: %s: context lookup failed"),
quotef (file));
   return;
-- 
2.27.0

Re: [PATCH] selinux-h: add stubs for selabel_open etc.

[Kamil Dudka]

On Sunday, November 22, 2020 7:08:44 PM CET Pádraig Brady wrote:
> > It seems selabel_lookup requires absolute paths.
> > Reinstating that code with the attached,
> > gets all tests to pass here on Fedora 32
> > with selinux enabled.
> 
> Non leaky version attached.
> 
> cheers,
> Pádraig

Thanks for the patch!

It fixes tests/install/install-Z-selinux and tests/mkdir/restorecon for me.  
However tests/install/basic-1 still fails with the following warnings in the 
log:

...: context lookup failed: Operation not supported

My test-suite.log from Fedora 32 is attached.

If I go back to coreutils commit 5c8e2716b3f206bc063dcc0b7eb509ecd2804cf5,
all tests pass again on the same Fedora 32 machine.

Kamil

test-suite.log.gz
Description: application/gzip

Re: [PATCH] selinux-h: add stubs for selabel_open etc.

[Paul Eggert]

On 11/22/20 10:08 AM, Pádraig Brady wrote:


Non leaky version attached.


Thanks, I installed that, along with the attached further coreutils patch to fix 
some bugs in the nearby errno handling. Most likely there are other issues in 
the SELinux area but I ran out of time to look into this right now.


Is there an SELinux crew at Red Hat that can look at coreutils as a sanity 
check?
From 6ac67d13567f4ab10722c612d3ef05d0f2ad80ed Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Mon, 23 Nov 2020 00:52:00 -0800
Subject: [PATCH] maint: propagate errno better in selinux.c
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* src/selinux.c: Don’t include die.h; no longer needed.
(computecon, defaultcon, restorecon): Propagate errno.
(defaultcon, restorecon): Don’t diagnose errors or exit, as that’s
the caller’s responsibility.
---
 src/selinux.c | 21 ++---
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/src/selinux.c b/src/selinux.c
index 50efb0aec..92b6b6b52 100644
--- a/src/selinux.c
+++ b/src/selinux.c
@@ -21,7 +21,6 @@
 #include 
 #include 
 
-#include "die.h"
 #include "system.h"
 #include "canonicalize.h"
 #include "xfts.h"
@@ -89,10 +88,12 @@ computecon (char const *path, mode_t mode, char **con)
 goto quit;
   rc = security_compute_create (scon, tcon, tclass, con);
 
-quit:
+ quit:;
+  int err = errno;
   free (dir);
   freecon (scon);
   freecon (tcon);
+  errno = err;
   return rc;
 }
 
@@ -119,10 +120,10 @@ defaultcon (struct selabel_handle *selabel_handle,
 
   if (! IS_ABSOLUTE_FILE_NAME (path))
 {
+  /* Generate absolute name as required by subsequent selabel_lookup.  */
   newpath = canonicalize_filename_mode (path, CAN_MISSING);
   if (! newpath)
-die (EXIT_FAILURE, errno, _("error canonicalizing %s"),
- quoteaf (path));
+goto quit;
   path = newpath;
 }
 
@@ -153,12 +154,14 @@ defaultcon (struct selabel_handle *selabel_handle,
 
   rc = setfscreatecon (constr);
 
-quit:
+ quit:;
+  int err = errno;
   context_free (scontext);
   context_free (tcontext);
   freecon (scon);
   freecon (tcon);
   free (newpath);
+  errno = err;
   return rc;
 }
 
@@ -286,17 +289,21 @@ restorecon (struct selabel_handle *selabel_handle,
 
   if (! IS_ABSOLUTE_FILE_NAME (path))
 {
+  /* Generate absolute name as required by subsequent selabel_lookup.
+ When RECURSE, this also generates absolute names in the
+ fts entries, which may be quicker to process in any case.  */
   newpath = canonicalize_filename_mode (path, CAN_MISSING);
   if (! newpath)
-die (EXIT_FAILURE, errno, _("error canonicalizing %s"),
- quoteaf (path));
+return false;
   path = newpath;
 }
 
   if (! recurse)
 {
   bool ok = restorecon_private (selabel_handle, path) != -1;
+  int err = errno;
   free (newpath);
+  errno = err;
   return ok;
 }
 
-- 
2.27.0