Re: [PATCH v3 3/3] fhandler/proc.cc: use wincap.has_user_shstk

2023-06-16 Thread Corinna Vinschen
Hi Brain,


thanks for the patch. Sorry to hassle you again, but I forget to mention
this yesterday and I'm still only partially available.  So, here goes:

It would be really great if you could resend your patchset with three
changes in the commit message:

- For obvious reasons, the message text in your cover message won't make
  it into the git repo.  However, the commit messages in git should
  reflect why the change was made, so a future interested reader has
  a chance to understand why a change was made.

- As I already mentioned a couple of times on this list (but not
  lately), it would be great if you could always add a "Signed-off-by:"
  line.

- Also, given this patch fixes an earlier patch, it should contain
  a line

Fixes: <12-digit-SHA1> ("commit headline")

  In this case, patch 3 of the series should contain

Fixes: 41fdb869f998 ("fhandler/proc.cc(format_proc_cpuinfo): Add Linux 6.3 
cpuinfo")


Thanks,
Corinna


On Jun 15 18:16, Brian Inglis wrote:
> ---
>  winsup/cygwin/fhandler/proc.cc | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/winsup/cygwin/fhandler/proc.cc b/winsup/cygwin/fhandler/proc.cc
> index 3c79762e0fbd..2eaf436dc122 100644
> --- a/winsup/cygwin/fhandler/proc.cc
> +++ b/winsup/cygwin/fhandler/proc.cc
> @@ -1486,12 +1486,12 @@ format_proc_cpuinfo (void *, char *&destbuf)
>  
>  /* ftcprint (features1,  6, "split_lock_detect");*//* MSR_TEST_CTRL 
> split lock */
>  
> -  /* cpuid 0x0007 ecx & Windows [20]20H1/[20]2004+ */
> -  if (maxf >= 0x0007 && wincap.osname () >= "10.0"
> -  && wincap.build_number () >= 19041)
> +  /* Windows [20]20H1/[20]2004/19041 user shadow stack */
> +  if (maxf >= 0x0007 && wincap.has_user_shstk ())
>  {
> +   /* cpuid 0x0007 ecx CET shadow stack */
> cpuid (&unused, &unused, &features1, &unused, 0x0007, 0);
> -   ftcprint (features1,  7, "user_shstk");   /* "user shadow stack" 
> */
> +   ftcprint (features1,  7, "user_shstk");   /* user shadow stack */
>   }
>  
>/* cpuid 0x0007:1 eax */
> -- 
> 2.39.0


Re: [PATCH v3 3/3] fhandler/proc.cc: use wincap.has_user_shstk

2023-06-16 Thread Corinna Vinschen
On Jun 16 10:28, Corinna Vinschen wrote:
> Hi Brain,
> 
> 
> thanks for the patch. Sorry to hassle you again, but I forget to mention
> this yesterday and I'm still only partially available.  So, here goes:
> 
> It would be really great if you could resend your patchset with three
> changes in the commit message:
> 
> - For obvious reasons, the message text in your cover message won't make
>   it into the git repo.  However, the commit messages in git should
>   reflect why the change was made, so a future interested reader has
>   a chance to understand why a change was made.

Along these lines, given this patch fixes another one, the message text
should ideally outline what was wrong with the original patch and that
the new method doing things fixes it.

> 
> - As I already mentioned a couple of times on this list (but not
>   lately), it would be great if you could always add a "Signed-off-by:"
>   line.
> 
> - Also, given this patch fixes an earlier patch, it should contain
>   a line
> 
> Fixes: <12-digit-SHA1> ("commit headline")
> 
>   In this case, patch 3 of the series should contain
> 
> Fixes: 41fdb869f998 ("fhandler/proc.cc(format_proc_cpuinfo): Add Linux 
> 6.3 cpuinfo")
> 
> 
> Thanks,
> Corinna
> 
> 
> On Jun 15 18:16, Brian Inglis wrote:
> > ---
> >  winsup/cygwin/fhandler/proc.cc | 8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/winsup/cygwin/fhandler/proc.cc b/winsup/cygwin/fhandler/proc.cc
> > index 3c79762e0fbd..2eaf436dc122 100644
> > --- a/winsup/cygwin/fhandler/proc.cc
> > +++ b/winsup/cygwin/fhandler/proc.cc
> > @@ -1486,12 +1486,12 @@ format_proc_cpuinfo (void *, char *&destbuf)
> >  
> >  /*   ftcprint (features1,  6, "split_lock_detect");*//* MSR_TEST_CTRL 
> > split lock */
> >  
> > -  /* cpuid 0x0007 ecx & Windows [20]20H1/[20]2004+ */
> > -  if (maxf >= 0x0007 && wincap.osname () >= "10.0"
> > -&& wincap.build_number () >= 19041)
> > +  /* Windows [20]20H1/[20]2004/19041 user shadow stack */
> > +  if (maxf >= 0x0007 && wincap.has_user_shstk ())
> >  {
> > + /* cpuid 0x0007 ecx CET shadow stack */
> >   cpuid (&unused, &unused, &features1, &unused, 0x0007, 0);
> > - ftcprint (features1,  7, "user_shstk");   /* "user shadow stack" 
> > */
> > + ftcprint (features1,  7, "user_shstk");   /* user shadow stack */
> > }
> >  
> >/* cpuid 0x0007:1 eax */
> > -- 
> > 2.39.0


Re: [PATCH] include/cygwin/limits.h: add XATTR_{NAME,SIZE,LIST}_MAX

2023-06-16 Thread Philippe Cerfon
Hey Corinna.

On Wed, Jun 7, 2023 at 12:06 PM Corinna Vinschen
 wrote:
> Hmm, the comparisons would have to check for XATTR_NAME_MAX anyway,
> so maybe inlining is cleaner in this case.

Please have a look at the updated and attached patches.

Thanks,
Philippe.
From 82e2ff6f52d7401210247ae396ce3f1f115d93f0 Mon Sep 17 00:00:00 2001
From: Philippe Cerfon 
Date: Tue, 30 May 2023 13:16:18 +0200
Subject: [PATCH 1/2] Cygwin: export XATTR_{NAME,SIZE,LIST}_MAX

These are used for example by CPython.

Signed-off-by: Philippe Cerfon 
Signed-off-by: Corinna Vinschen 
---
 winsup/cygwin/include/cygwin/limits.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/winsup/cygwin/include/cygwin/limits.h b/winsup/cygwin/include/cygwin/limits.h
index aefc7c7bd..ea3e2836a 100644
--- a/winsup/cygwin/include/cygwin/limits.h
+++ b/winsup/cygwin/include/cygwin/limits.h
@@ -56,4 +56,11 @@ details. */
 #define __PATH_MAX 4096
 #define __PIPE_BUF 4096
 
+/* XATTR_NAME_MAX is the maximum XATTR name length excluding the null
+ * terminator. Since only XATTRs in the `user' namespace are allowed and the
+ * `user.' prefix is not stored, the maximum is increased by 5. */
+#define XATTR_NAME_MAX 260
+#define XATTR_SIZE_MAX 65536
+#define XATTR_LIST_MAX 65536
+
 #endif /* _CYGWIN_LIMITS_H__ */
-- 
2.40.1

From 34a2e210082ddbf87d2c9569b6b14775a6cc5b95 Mon Sep 17 00:00:00 2001
From: Philippe Cerfon 
Date: Tue, 6 Jun 2023 02:52:49 +0200
Subject: [PATCH 2/2] Cygwin: use new XATTR_{NAME,SIZE}_MAX instead of
 MAX_EA_{NAME,VALUE}_LEN

Signed-off-by: Philippe Cerfon 
---
 winsup/cygwin/ntea.cc | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/winsup/cygwin/ntea.cc b/winsup/cygwin/ntea.cc
index a400fcb2b..ef1fdf4cc 100644
--- a/winsup/cygwin/ntea.cc
+++ b/winsup/cygwin/ntea.cc
@@ -17,9 +17,7 @@ details. */
 #include "tls_pbuf.h"
 #include 
 #include 
-
-#define MAX_EA_NAME_LEN256
-#define MAX_EA_VALUE_LEN 65536
+#include 
 
 /* At least one maximum sized entry fits.
CV 2014-04-04: NtQueryEaFile function chokes on buffers bigger than 64K
@@ -27,13 +25,14 @@ details. */
 		  on a remote share, at least on Windows 7 and later.
 		  In theory the buffer should have a size of
 
-		sizeof (FILE_FULL_EA_INFORMATION) + MAX_EA_NAME_LEN
-		+ MAX_EA_VALUE_LEN
+		sizeof (FILE_FULL_EA_INFORMATION)
+		+ (XATTR_NAME_MAX + 1 - strlen("user."))
+		+ XATTR_SIZE_MAX
 
 		  (65804 bytes), but we're opting for simplicity here, and
 		  a 64K buffer has the advantage that we can use a tmp_pathbuf
 		  buffer, rather than having to alloca 64K from stack. */
-#define EA_BUFSIZ MAX_EA_VALUE_LEN
+#define EA_BUFSIZ XATTR_SIZE_MAX
 
 #define NEXT_FEA(p) ((PFILE_FULL_EA_INFORMATION) (p->NextEntryOffset \
 		 ? (char *) p + p->NextEntryOffset : NULL))
@@ -55,7 +54,7 @@ read_ea (HANDLE hdl, path_conv &pc, const char *name, char *value, size_t size)
  returns the last EA entry of the file infinitely.  Even utilizing the
  optional EaIndex only helps marginally.  If you use that, the last
  EA in the file is returned twice. */
-  char lastname[MAX_EA_NAME_LEN];
+  char lastname[(XATTR_NAME_MAX + 1 - strlen("user."))];
 
   __try
 {
@@ -95,7 +94,7 @@ read_ea (HANDLE hdl, path_conv &pc, const char *name, char *value, size_t size)
 	  __leave;
 	}
 
-	  if ((nlen = strlen (name)) >= MAX_EA_NAME_LEN)
+	  if ((nlen = strlen (name)) >= (XATTR_NAME_MAX + 1 - strlen("user.")))
 	{
 	  set_errno (EINVAL);
 	  __leave;
@@ -197,7 +196,7 @@ read_ea (HANDLE hdl, path_conv &pc, const char *name, char *value, size_t size)
 		  /* For compatibility with Linux, we always prepend "user." to
 		 the attribute name, so effectively we only support user
 		 attributes from a application point of view. */
-		  char tmpbuf[MAX_EA_NAME_LEN * 2];
+		  char tmpbuf[(XATTR_NAME_MAX + 1 - strlen("user.")) * 2];
 		  char *tp = stpcpy (tmpbuf, "user.");
 		  stpcpy (tp, fea->EaName);
 		  /* NTFS stores all EA names in uppercase unfortunately.  To
@@ -297,7 +296,7 @@ write_ea (HANDLE hdl, path_conv &pc, const char *name, const char *value,
   /* Skip "user." prefix. */
   name += 5;
 
-  if ((nlen = strlen (name)) >= MAX_EA_NAME_LEN)
+  if ((nlen = strlen (name)) >= (XATTR_NAME_MAX + 1 - strlen("user.")))
 	{
 	  set_errno (EINVAL);
 	  __leave;
-- 
2.40.1



Re: [PATCH] include/cygwin/limits.h: add XATTR_{NAME,SIZE,LIST}_MAX

2023-06-16 Thread Corinna Vinschen
Hi Philippe,

On Jun 16 16:09, Philippe Cerfon wrote:
> Hey Corinna.
> 
> On Wed, Jun 7, 2023 at 12:06 PM Corinna Vinschen
>  wrote:
> > Hmm, the comparisons would have to check for XATTR_NAME_MAX anyway,
> > so maybe inlining is cleaner in this case.
> 
> Please have a look at the updated and attached patches.
> 
> Thanks,
> Philippe.

Oh well. Now that I see it in real life, my idea to use the entire
expression inline wasn't that great, it seems... 

I didn't want to keep MAX_EA_NAME_LEN because now that we have an
official name for the value, having an unofficial name using a different
naming convention is a bit weird.

On the other hand, having a macro for the expression certainly looks
much cleaner.  Also, only one place to change (should a change ever be
necessary).

Sorry about that.

What do you think about something like _XATTR_NAME_MAX_ONDISK_?

I can also just push the patches and we discuss this further afterwards,
your call.


Thanks,
Corinna



> @@ -55,7 +54,7 @@ read_ea (HANDLE hdl, path_conv &pc, const char *name, char 
> *value, size_t size)
>   returns the last EA entry of the file infinitely.  Even utilizing the
>   optional EaIndex only helps marginally.  If you use that, the last
>   EA in the file is returned twice. */
> -  char lastname[MAX_EA_NAME_LEN];
> +  char lastname[(XATTR_NAME_MAX + 1 - strlen("user."))];
>  
>__try
>  {
> @@ -95,7 +94,7 @@ read_ea (HANDLE hdl, path_conv &pc, const char *name, char 
> *value, size_t size)
> __leave;
>   }
>  
> -   if ((nlen = strlen (name)) >= MAX_EA_NAME_LEN)
> +   if ((nlen = strlen (name)) >= (XATTR_NAME_MAX + 1 - strlen("user.")))
>   {
> set_errno (EINVAL);
> __leave;
> @@ -197,7 +196,7 @@ read_ea (HANDLE hdl, path_conv &pc, const char *name, 
> char *value, size_t size)
> /* For compatibility with Linux, we always prepend "user." to
>the attribute name, so effectively we only support user
>attributes from a application point of view. */
> -   char tmpbuf[MAX_EA_NAME_LEN * 2];
> +   char tmpbuf[(XATTR_NAME_MAX + 1 - strlen("user.")) * 2];
> char *tp = stpcpy (tmpbuf, "user.");
> stpcpy (tp, fea->EaName);
> /* NTFS stores all EA names in uppercase unfortunately.  To
> @@ -297,7 +296,7 @@ write_ea (HANDLE hdl, path_conv &pc, const char *name, 
> const char *value,
>/* Skip "user." prefix. */
>name += 5;
>  
> -  if ((nlen = strlen (name)) >= MAX_EA_NAME_LEN)
> +  if ((nlen = strlen (name)) >= (XATTR_NAME_MAX + 1 - strlen("user.")))
>   {
> set_errno (EINVAL);
> __leave;
> -- 
> 2.40.1
> 



Re: [PATCH] include/cygwin/limits.h: add XATTR_{NAME,SIZE,LIST}_MAX

2023-06-16 Thread Philippe Cerfon
Hey.

On Fri, Jun 16, 2023 at 5:04 PM Corinna Vinschen
 wrote:
> Oh well. Now that I see it in real life, my idea to use the entire
> expression inline wasn't that great, it seems...

^^


> I didn't want to keep MAX_EA_NAME_LEN because now that we have an
> official name for the value, having an unofficial name using a different
> naming convention is a bit weird.
>
> On the other hand, having a macro for the expression certainly looks
> much cleaner.  Also, only one place to change (should a change ever be
> necessary).

Does both make sense.


> Sorry about that.

No worries :-)


> What do you think about something like _XATTR_NAME_MAX_ONDISK_?

Really with trailing/leading underscores? If you try to keep it out of
the "official namespace", then I'd would perhaps make more sense to
mark this as being cygwin specific like CYGWIN_XATTR_NAME_MAX_ONDISK
or so?
Also - may be nitpicking - but storage is not really guaranteed to be
a disk anymore. Maybe ONSTORAGE instead? But admittedly ONDISK sounds
more common ("on disk format", etc.).

> I can also just push the patches and we discuss this further afterwards,
> your call.

Well you know the naming convention used in your code much better than I do.

Attached patches use _XATTR_NAME_MAX_ONDISK_ as you proposed.

Just pick whichever name you like best, and either tell me and I
provide a new patch, or just sed 's/_XATTR_NAME_MAX_ONDISK_/foobar/g'
(+ maybe align text wrapping of comments if necessary).


Thanks,
Philippe
From 82e2ff6f52d7401210247ae396ce3f1f115d93f0 Mon Sep 17 00:00:00 2001
From: Philippe Cerfon 
Date: Tue, 30 May 2023 13:16:18 +0200
Subject: [PATCH 1/2] Cygwin: export XATTR_{NAME,SIZE,LIST}_MAX

These are used for example by CPython.

Signed-off-by: Philippe Cerfon 
Signed-off-by: Corinna Vinschen 
---
 winsup/cygwin/include/cygwin/limits.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/winsup/cygwin/include/cygwin/limits.h b/winsup/cygwin/include/cygwin/limits.h
index aefc7c7bd..ea3e2836a 100644
--- a/winsup/cygwin/include/cygwin/limits.h
+++ b/winsup/cygwin/include/cygwin/limits.h
@@ -56,4 +56,11 @@ details. */
 #define __PATH_MAX 4096
 #define __PIPE_BUF 4096
 
+/* XATTR_NAME_MAX is the maximum XATTR name length excluding the null
+ * terminator. Since only XATTRs in the `user' namespace are allowed and the
+ * `user.' prefix is not stored, the maximum is increased by 5. */
+#define XATTR_NAME_MAX 260
+#define XATTR_SIZE_MAX 65536
+#define XATTR_LIST_MAX 65536
+
 #endif /* _CYGWIN_LIMITS_H__ */
-- 
2.40.1

From 4c94f80fc817bee0e59da39397391cf2409cf029 Mon Sep 17 00:00:00 2001
From: Philippe Cerfon 
Date: Tue, 6 Jun 2023 02:52:49 +0200
Subject: [PATCH 2/2] Cygwin: use new XATTR_{NAME,SIZE}_MAX instead of
 MAX_EA_{NAME,VALUE}_LEN

Signed-off-by: Philippe Cerfon 
---
 winsup/cygwin/ntea.cc | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/winsup/cygwin/ntea.cc b/winsup/cygwin/ntea.cc
index a400fcb2b..70815649c 100644
--- a/winsup/cygwin/ntea.cc
+++ b/winsup/cygwin/ntea.cc
@@ -17,9 +17,11 @@ details. */
 #include "tls_pbuf.h"
 #include 
 #include 
+#include 
 
-#define MAX_EA_NAME_LEN256
-#define MAX_EA_VALUE_LEN 65536
+/* On storage the `user.` prefix is not included but the terminating null byte
+   is needed.*/
+#define _XATTR_NAME_MAX_ONDISK_ (XATTR_NAME_MAX - strlen("user.") + 1)
 
 /* At least one maximum sized entry fits.
CV 2014-04-04: NtQueryEaFile function chokes on buffers bigger than 64K
@@ -27,13 +29,13 @@ details. */
 		  on a remote share, at least on Windows 7 and later.
 		  In theory the buffer should have a size of
 
-		sizeof (FILE_FULL_EA_INFORMATION) + MAX_EA_NAME_LEN
-		+ MAX_EA_VALUE_LEN
+		sizeof (FILE_FULL_EA_INFORMATION) + _XATTR_NAME_MAX_ONDISK_
+		+ XATTR_SIZE_MAX
 
 		  (65804 bytes), but we're opting for simplicity here, and
 		  a 64K buffer has the advantage that we can use a tmp_pathbuf
 		  buffer, rather than having to alloca 64K from stack. */
-#define EA_BUFSIZ MAX_EA_VALUE_LEN
+#define EA_BUFSIZ XATTR_SIZE_MAX
 
 #define NEXT_FEA(p) ((PFILE_FULL_EA_INFORMATION) (p->NextEntryOffset \
 		 ? (char *) p + p->NextEntryOffset : NULL))
@@ -55,7 +57,7 @@ read_ea (HANDLE hdl, path_conv &pc, const char *name, char *value, size_t size)
  returns the last EA entry of the file infinitely.  Even utilizing the
  optional EaIndex only helps marginally.  If you use that, the last
  EA in the file is returned twice. */
-  char lastname[MAX_EA_NAME_LEN];
+  char lastname[_XATTR_NAME_MAX_ONDISK_];
 
   __try
 {
@@ -95,7 +97,7 @@ read_ea (HANDLE hdl, path_conv &pc, const char *name, char *value, size_t size)
 	  __leave;
 	}
 
-	  if ((nlen = strlen (name)) >= MAX_EA_NAME_LEN)
+	  if ((nlen = strlen (name)) >= _XATTR_NAME_MAX_ONDISK_)
 	{
 	  set_errno (EINVAL);
 	  __leave;
@@ -197,7 +199,7 @@ read_ea (HANDLE hdl, path_conv &pc, const char *name, char *value, size_t size)
 		  /* For compatibility with Linux, w

[PATCH v3 0/3] use wincap in format_proc_cpuinfo for user_shstk

2023-06-16 Thread Brian Inglis
Fixes: 41fdb869f998 "fhandler/proc.cc(format_proc_cpuinfo): Add Linux 6.3 
cpuinfo"

In test for for AMD/Intel Control flow Enforcement Technology user mode
shadow stack support replace Windows version tests with test of wincap
member addition has_user_shstk with Windows version dependent value

Signed-off-by: Brian Inglis 

Brian Inglis (3):
  wincap.h: add wincap member has_user_shstk
  wincap.cc: set wincap member has_user_shstk true for 2004+
  fhandler/proc.cc: use wincap.has_user_shstk

 winsup/cygwin/fhandler/proc.cc|  8 
 winsup/cygwin/local_includes/wincap.h |  2 ++
 winsup/cygwin/wincap.cc   | 10 ++
 3 files changed, 16 insertions(+), 4 deletions(-)

-- 
2.39.0



[PATCH v3 1/3] wincap.h: add wincap member has_user_shstk

2023-06-16 Thread Brian Inglis
Signed-off-by: Brian Inglis 
---
 winsup/cygwin/local_includes/wincap.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/winsup/cygwin/local_includes/wincap.h 
b/winsup/cygwin/local_includes/wincap.h
index 29a7a63de7f6..c14872787ca2 100644
--- a/winsup/cygwin/local_includes/wincap.h
+++ b/winsup/cygwin/local_includes/wincap.h
@@ -32,6 +32,7 @@ struct wincaps
 unsigned has_linux_tcp_keepalive_sockopts  : 1;
 unsigned has_tcp_maxrtms   : 1;
 unsigned has_con_broken_tabs   : 1;
+unsigned has_user_shstk: 1;
   };
 };
 
@@ -84,6 +85,7 @@ public:
   bool IMPLEMENT (has_linux_tcp_keepalive_sockopts)
   bool IMPLEMENT (has_tcp_maxrtms)
   bool IMPLEMENT (has_con_broken_tabs)
+  bool IMPLEMENT (has_user_shstk)
 
   void disable_case_sensitive_dirs ()
   {
-- 
2.39.0



[PATCH v3 2/3] wincap.cc: set wincap member has_user_shstk true for 2004+

2023-06-16 Thread Brian Inglis
Signed-off-by: Brian Inglis 
---
 winsup/cygwin/wincap.cc | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/winsup/cygwin/wincap.cc b/winsup/cygwin/wincap.cc
index 91d5d9df8889..30d9c14e8d3b 100644
--- a/winsup/cygwin/wincap.cc
+++ b/winsup/cygwin/wincap.cc
@@ -31,6 +31,7 @@ static const wincaps wincap_8_1 = {
 has_linux_tcp_keepalive_sockopts:false,
 has_tcp_maxrtms:false,
 has_con_broken_tabs:false,
+has_user_shstk:false,
   },
 };
 
@@ -52,6 +53,7 @@ static const wincaps  wincap_10_1507 = {
 has_linux_tcp_keepalive_sockopts:false,
 has_tcp_maxrtms:false,
 has_con_broken_tabs:false,
+has_user_shstk:false,
   },
 };
 
@@ -73,6 +75,7 @@ static const wincaps  wincap_10_1607 = {
 has_linux_tcp_keepalive_sockopts:false,
 has_tcp_maxrtms:true,
 has_con_broken_tabs:false,
+has_user_shstk:false,
   },
 };
 
@@ -94,6 +97,7 @@ static const wincaps wincap_10_1703 = {
 has_linux_tcp_keepalive_sockopts:false,
 has_tcp_maxrtms:true,
 has_con_broken_tabs:true,
+has_user_shstk:false,
   },
 };
 
@@ -115,6 +119,7 @@ static const wincaps wincap_10_1709 = {
 has_linux_tcp_keepalive_sockopts:true,
 has_tcp_maxrtms:true,
 has_con_broken_tabs:true,
+has_user_shstk:false,
   },
 };
 
@@ -136,6 +141,7 @@ static const wincaps wincap_10_1803 = {
 has_linux_tcp_keepalive_sockopts:true,
 has_tcp_maxrtms:true,
 has_con_broken_tabs:true,
+has_user_shstk:false,
   },
 };
 
@@ -157,6 +163,7 @@ static const wincaps wincap_10_1809 = {
 has_linux_tcp_keepalive_sockopts:true,
 has_tcp_maxrtms:true,
 has_con_broken_tabs:true,
+has_user_shstk:false,
   },
 };
 
@@ -178,6 +185,7 @@ static const wincaps wincap_10_1903 = {
 has_linux_tcp_keepalive_sockopts:true,
 has_tcp_maxrtms:true,
 has_con_broken_tabs:true,
+has_user_shstk:false,
   },
 };
 
@@ -199,6 +207,7 @@ static const wincaps wincap_10_2004 = {
 has_linux_tcp_keepalive_sockopts:true,
 has_tcp_maxrtms:true,
 has_con_broken_tabs:true,
+has_user_shstk:true,
   },
 };
 
@@ -220,6 +229,7 @@ static const wincaps wincap_11 = {
 has_linux_tcp_keepalive_sockopts:true,
 has_tcp_maxrtms:true,
 has_con_broken_tabs:false,
+has_user_shstk:true,
   },
 };
 
-- 
2.39.0



[PATCH v3 3/3] fhandler/proc.cc: use wincap.has_user_shstk

2023-06-16 Thread Brian Inglis
Signed-off-by: Brian Inglis 
---
 winsup/cygwin/fhandler/proc.cc | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/winsup/cygwin/fhandler/proc.cc b/winsup/cygwin/fhandler/proc.cc
index 3c79762e0fbd..cbc49a12a417 100644
--- a/winsup/cygwin/fhandler/proc.cc
+++ b/winsup/cygwin/fhandler/proc.cc
@@ -1486,12 +1486,12 @@ format_proc_cpuinfo (void *, char *&destbuf)
 
 /*   ftcprint (features1,  6, "split_lock_detect");*//* MSR_TEST_CTRL 
split lock */
 
-  /* cpuid 0x0007 ecx & Windows [20]20H1/[20]2004+ */
-  if (maxf >= 0x0007 && wincap.osname () >= "10.0"
-&& wincap.build_number () >= 19041)
+  /* Windows [20]20H1/[20]2004/19041 user shadow stack */
+  if (maxf >= 0x0007 && wincap.has_user_shstk ())
 {
+ /* cpuid 0x0007 ecx CET shadow stack */
  cpuid (&unused, &unused, &features1, &unused, 0x0007, 0);
- ftcprint (features1,  7, "user_shstk");   /* "user shadow stack" 
*/
+ ftcprint (features1,  7, "user_shstk");   /* user shadow stack */
}
 
   /* cpuid 0x0007:1 eax */
-- 
2.39.0



Re: [PATCH v3 0/3] use wincap in format_proc_cpuinfo for user_shstk

2023-06-16 Thread Corinna Vinschen
Hi Brian,

On Jun 16 11:17, Brian Inglis wrote:
> Fixes: 41fdb869f998 "fhandler/proc.cc(format_proc_cpuinfo): Add Linux 6.3 
> cpuinfo"
> 
> In test for for AMD/Intel Control flow Enforcement Technology user mode
> shadow stack support replace Windows version tests with test of wincap
> member addition has_user_shstk with Windows version dependent value
> 
> Signed-off-by: Brian Inglis 
> 
> Brian Inglis (3):
>   wincap.h: add wincap member has_user_shstk
>   wincap.cc: set wincap member has_user_shstk true for 2004+
>   fhandler/proc.cc: use wincap.has_user_shstk
> 
>  winsup/cygwin/fhandler/proc.cc|  8 
>  winsup/cygwin/local_includes/wincap.h |  2 ++
>  winsup/cygwin/wincap.cc   | 10 ++
>  3 files changed, 16 insertions(+), 4 deletions(-)
> 
> -- 
> 2.39.0

Is that actually the final version?  It's still missing the commit
message text explaining things and the "Fixes" line...


Thanks,
Corinna


Re: [PATCH] include/cygwin/limits.h: add XATTR_{NAME,SIZE,LIST}_MAX

2023-06-16 Thread Corinna Vinschen
On Jun 16 17:43, Philippe Cerfon wrote:
> Hey.
> 
> On Fri, Jun 16, 2023 at 5:04 PM Corinna Vinschen
>  wrote:
> > Oh well. Now that I see it in real life, my idea to use the entire
> > expression inline wasn't that great, it seems...
> 
> ^^
> 
> 
> > I didn't want to keep MAX_EA_NAME_LEN because now that we have an
> > official name for the value, having an unofficial name using a different
> > naming convention is a bit weird.
> >
> > On the other hand, having a macro for the expression certainly looks
> > much cleaner.  Also, only one place to change (should a change ever be
> > necessary).
> 
> Does both make sense.
> 
> 
> > Sorry about that.
> 
> No worries :-)
> 
> 
> > What do you think about something like _XATTR_NAME_MAX_ONDISK_?
> 
> Really with trailing/leading underscores? If you try to keep it out of
> the "official namespace", then I'd would perhaps make more sense to
> mark this as being cygwin specific like CYGWIN_XATTR_NAME_MAX_ONDISK
> or so?
> Also - may be nitpicking - but storage is not really guaranteed to be
> a disk anymore. Maybe ONSTORAGE instead? But admittedly ONDISK sounds
> more common ("on disk format", etc.).

You're right, of course.  Disk is just like everyone talks about it.
Even a SSD has "disk" in it's name :)

> > I can also just push the patches and we discuss this further afterwards,
> > your call.
> 
> Well you know the naming convention used in your code much better than I do.
> 
> Attached patches use _XATTR_NAME_MAX_ONDISK_ as you proposed.
> 
> Just pick whichever name you like best, and either tell me and I
> provide a new patch, or just sed 's/_XATTR_NAME_MAX_ONDISK_/foobar/g'
> (+ maybe align text wrapping of comments if necessary).

Let's keep it at that.  I pushed your patchset.


Thanks!
Corinna


Re: [PATCH] include/cygwin/limits.h: add XATTR_{NAME,SIZE,LIST}_MAX

2023-06-16 Thread Philippe Cerfon
On Fri, Jun 16, 2023 at 9:49 PM Corinna Vinschen
 wrote:
> Even a SSD has "disk" in it's name :)

That actually stands for drive :-)

> Let's keep it at that.  I pushed your patchset.

Thanks for merging! :-)

Any rough estimate when this will be in a live release?

Regards and best wishes,
Philippe


Re: [PATCH v3 0/3] use wincap in format_proc_cpuinfo for user_shstk

2023-06-16 Thread Brian Inglis

On 2023-06-16 13:51, Corinna Vinschen wrote:

Hi Brian,

On Jun 16 11:17, Brian Inglis wrote:


v

Fixes: 41fdb869f998 "fhandler/proc.cc(format_proc_cpuinfo): Add Linux 6.3 
cpuinfo"

^

v

In test for for AMD/Intel Control flow Enforcement Technology user mode
shadow stack support replace Windows version tests with test of wincap
member addition has_user_shstk with Windows version dependent value

^


Is that actually the final version?  It's still missing the commit
message text explaining things and the "Fixes" line...

Hi Corinna,

Is more required above?

--
Take care. Thanks, Brian Inglis  Calgary, Alberta, Canada

La perfection est atteinte   Perfection is achieved
non pas lorsqu'il n'y a plus rien à ajouter  not when there is no more to add
mais lorsqu'il n'y a plus rien à retirer but when there is no more to cut
-- Antoine de Saint-Exupéry