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

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

[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@Shaw.ca>

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

[Brian Inglis]

Signed-off-by: Brian Inglis <Brian.Inglis@Shaw.ca>
---
 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+

[Brian Inglis]

Signed-off-by: Brian Inglis <Brian.Inglis@Shaw.ca>
---
 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

[Brian Inglis]

Signed-off-by: Brian Inglis <Brian.Inglis@Shaw.ca>
---
 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 0x00000007 ecx & Windows [20]20H1/[20]2004+ */
-      if (maxf >= 0x00000007 && wincap.osname () >= "10.0"
-					 && wincap.build_number () >= 19041)
+      /* Windows [20]20H1/[20]2004/19041 user shadow stack */
+      if (maxf >= 0x00000007 && wincap.has_user_shstk ())
         {
+	  /* cpuid 0x00000007 ecx CET shadow stack */
 	  cpuid (&unused, &unused, &features1, &unused, 0x00000007, 0);
-	  ftcprint (features1,  7, "user_shstk");	/* "user shadow stack" */
+	  ftcprint (features1,  7, "user_shstk");	/* user shadow stack */
 	}
 
       /* cpuid 0x00000007:1 eax */
-- 
2.39.0

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

[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@Shaw.ca>
> 
> 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 v3 0/3] use wincap in format_proc_cpuinfo for user_shstk

[Brian Inglis]

On 2023-06-16 13:51, Corinna Vinschen wrote:
> Hi Brian,
> 
> On Jun 16 11:17, Brian Inglis wrote:

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

vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
>> 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

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

[Corinna Vinschen]

On Jun 16 15:26, Brian Inglis wrote:
> On 2023-06-16 13:51, Corinna Vinschen wrote:
> > Hi Brian,
> > 
> > On Jun 16 11:17, Brian Inglis wrote:
> 
> vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
> > > Fixes: 41fdb869f998 "fhandler/proc.cc(format_proc_cpuinfo): Add Linux 6.3 cpuinfo"
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
> > > 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?

No, it's fine, albeit "Fixes:" is supposed to be kind of like a footer,
just where the "Signed-off-by:" is, too.

But it's still only in the cover letter.  As I wrote, it needs to go
into the actual patch, otherwise all the nice info doesn't make it into
the git repo.


Thanks,
Corinna

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

[Brian Inglis]

On 2023-06-19 02:55, Corinna Vinschen wrote:
> On Jun 16 15:26, Brian Inglis wrote:
>> On 2023-06-16 13:51, Corinna Vinschen wrote:
>>> Hi Brian,
>>>
>>> On Jun 16 11:17, Brian Inglis wrote:
>>
>> vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
>>>> Fixes: 41fdb869f998 "fhandler/proc.cc(format_proc_cpuinfo): Add Linux 6.3 cpuinfo"
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>
>> vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
>>>> 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?
> 
> No, it's fine, albeit "Fixes:" is supposed to be kind of like a footer,
> just where the "Signed-off-by:" is, too.
> 
> But it's still only in the cover letter.  As I wrote, it needs to go
> into the actual patch, otherwise all the nice info doesn't make it into
> the git repo.

Ahah - finally got the point, although I wondered about whether a cover could be 
added as a note to the patches, and found that since early 2022 git 2.35+ allows

	$ git ... am --empty=keep --allow-empty ...

to create an empty commit, with the contents of the e-mail message as its log.
As we are on 2.39 you may already know this and prefer to keep the log clear. YMMV
Emailed v4.

-- 
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

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

[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 0x00000007 ecx & Windows [20]20H1/[20]2004+ */
> > -      if (maxf >= 0x00000007 && wincap.osname () >= "10.0"
> > -					 && wincap.build_number () >= 19041)
> > +      /* Windows [20]20H1/[20]2004/19041 user shadow stack */
> > +      if (maxf >= 0x00000007 && wincap.has_user_shstk ())
> >          {
> > +	  /* cpuid 0x00000007 ecx CET shadow stack */
> >  	  cpuid (&unused, &unused, &features1, &unused, 0x00000007, 0);
> > -	  ftcprint (features1,  7, "user_shstk");	/* "user shadow stack" */
> > +	  ftcprint (features1,  7, "user_shstk");	/* user shadow stack */
> >  	}
> >  
> >        /* cpuid 0x00000007:1 eax */
> > -- 
> > 2.39.0

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

[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 0x00000007 ecx & Windows [20]20H1/[20]2004+ */
> -      if (maxf >= 0x00000007 && wincap.osname () >= "10.0"
> -					 && wincap.build_number () >= 19041)
> +      /* Windows [20]20H1/[20]2004/19041 user shadow stack */
> +      if (maxf >= 0x00000007 && wincap.has_user_shstk ())
>          {
> +	  /* cpuid 0x00000007 ecx CET shadow stack */
>  	  cpuid (&unused, &unused, &features1, &unused, 0x00000007, 0);
> -	  ftcprint (features1,  7, "user_shstk");	/* "user shadow stack" */
> +	  ftcprint (features1,  7, "user_shstk");	/* user shadow stack */
>  	}
>  
>        /* cpuid 0x00000007:1 eax */
> -- 
> 2.39.0

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

[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..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 0x00000007 ecx & Windows [20]20H1/[20]2004+ */
-      if (maxf >= 0x00000007 && wincap.osname () >= "10.0"
-					 && wincap.build_number () >= 19041)
+      /* Windows [20]20H1/[20]2004/19041 user shadow stack */
+      if (maxf >= 0x00000007 && wincap.has_user_shstk ())
         {
+	  /* cpuid 0x00000007 ecx CET shadow stack */
 	  cpuid (&unused, &unused, &features1, &unused, 0x00000007, 0);
-	  ftcprint (features1,  7, "user_shstk");	/* "user shadow stack" */
+	  ftcprint (features1,  7, "user_shstk");	/* user shadow stack */
 	}
 
       /* cpuid 0x00000007:1 eax */
-- 
2.39.0