Re: [PATCH] fhandler/proc.cc(format_proc_cpuinfo): Add Linux 6.3 cpuinfo

2023-06-05 Thread Corinna Vinschen
On May 12 16:36, Jon Turney wrote:
> On 08/05/2023 04:12, Brian Inglis wrote:
> > cpuid0x0007:0 ecx:7 shstk Shadow Stack support & Windows 
> > [20]20H1/[20]2004+
> > => user_shstk User mode program Shadow Stack support
> > AMD SVM  0x800a:0 edx:25 vnmi virtual Non-Maskable Interrrupts
> > Sync AMD 0x8008:0 ebx flags across two output locations
> 
> Thanks.  I applied this.
> 
> Does this need applying to the 3.4 branch as well?
> 
> > ---
> >   winsup/cygwin/fhandler/proc.cc | 29 ++---
> 
> > +  /* cpuid 0x0007 ecx & Windows [20]20H1/[20]2004+ */
> > +  if (maxf >= 0x0007 && wincap.osname () >= "10.0"
> > +&& wincap.build_number () >= 19041)

No problems checking for the OS versions, but not like this.

  wincap.osname () >= "10.0"   ?

That will not do what you expect it to do.  wincap.osname() is a char *
and the >= operator will not work as on cstring in C++, but compare the
pointer values of the two strings instead.

While changing this to

  strcmp (wincap.osname (), "10.0") >= 0

is possible, it doesn't make sense.  For all supported Windows versions,
the build number is unambiguously bumped with each new release.  So
there's no older OS version with a build number >= 19041.  As a result,
the check for osname() can simply go away.

But then again, this is a windows feature which would best served by
adding a bit flag to the wincaps array, *and* we already have a wincaps
array for windows versions starting with build number 19041
(wincap_10_2004).

So, Brian, would you mind to create a followup patch which rather defines
a new bitflag in the wincaps array, set it to false or true according
to the OS version, and check this flag instead?


Thanks,
Corinna


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

2023-06-05 Thread Corinna Vinschen
[dropping newlib from the recipient list]

Hi Philippe,

On May 30 14:04, Brian Inglis wrote:
> On Tue, 30 May 2023 13:25:38 +0200, Philippe Cerfon wrote:
> > Hey there.
> > 
> > Linux exports XATTR_{NAME,SIZE,LIST}_MAX in it's linux/limits.h and
> > e.g. the CPython interpreter uses them for it's XATTRs functions.
> > 
> > I made a corresponding PR at CPython
> > https://github.com/python/cpython/pull/105075 to get the code built
> > for Cygwin, but right now this would fail due to the missing
> > XATTR_*_MAX symbols.
> > 
> > The attached patch below would add them to cygwin/limits.h.
> 
> Patches for Cygwin under winsup are submitted to cygwin-patches@cygwin.com
> (forwarded there).
> 
> > But beware, I'm absolutely no Windows/Cygwin expert ^^ - so whether
> > the values I've chosen are actually correct, is more guesswork rather
> > than definite knowledge.
> > 
> > As written in the commit message, I think:
> > - XATTR_NAME_MAX corresponds to MAX_EA_NAME_LEN
> > and
> > - XATTR_SIZE_MAX to MAX_EA_VALUE_LEN
> > 
> > though I have no idea, whether these are just lower boundaries used by
> > Cygwin, while e.g. Windows itself might set longer names or value
> > lenghts, and thus - when Cygwin would try to read such - it might get
> > into troubles (or rather e.g. CPython, as it's buffers wouldn't
> > suffice to read the EA respectively XATTR.

As on Linux, these are upper bounds of the kernel API. In case of
MAX_EA_VALUE_LEN that's especially true, because the defined API
isn't overly consistent: the datastructure allows bigger values
than the function calls are able to handle.  However, file systems
may impose lower limits without them having matching macros.
IIRC, an EA on ext4 may be only 4K, but I'm not entirely sure.

Either way, we can do what you propose, but...

- The XATTR_NAME_MAX value is incorrect. The MAX_EA_NAME_LEN value
  is an internal definition for a name *including* the trailing \0,
  the XATTR_NAME_MAX value defines the max name length *excluding*
  the trailing \0.  Compare with NAME_MAX.

- Whatever that's good for, we actually allow bigger values right
  now.  For compat reasons we only allow attributes starting with
  the "user." prefix, and the *trailing* part after "user." is
  allowed to be 255 bytes long, because we don't store the "user."
  prefix in the EA name on disk.  So in fact, XATTR_NAME_MAX should
  be 255 + strlen("user.") == 260.

- If we actually define these values in limits.h, it would also be a
  good idea to use them in ntea.cc and to throw away the MAX_EA_*_LEN
  macros.

A patch along these lines is appreciated.


Thanks,
Corinna


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

2023-06-05 Thread Philippe Cerfon
Hey Corinna, et al.

On Mon, Jun 5, 2023 at 9:05 PM Corinna Vinschen
 wrote:
> - Whatever that's good for, we actually allow bigger values right
>   now.  For compat reasons we only allow attributes starting with
>   the "user." prefix, and the *trailing* part after "user." is
>   allowed to be 255 bytes long, because we don't store the "user."
>   prefix in the EA name on disk.  So in fact, XATTR_NAME_MAX should
>   be 255 + strlen("user.") == 260.

I haven't given to much though into that right now (just about to go
for 2 weeks on vacation), but if "we" (Cygwin) allow now names up to
260 bytes, because we don't store the "user." .. doesn't that mean
users could set XATTRs, that in the end couldn't be read by e.g. Linux
(should there be, or ever be in the future, support for reading
FAT/NTFS' EAs as XATTRs e.g. from the Linux FAT/NTFS fs drivers)?


> - If we actually define these values in limits.h, it would also be a
>   good idea to use them in ntea.cc and to throw away the MAX_EA_*_LEN
>   macros.

Done so in a 2nd commit.
But that commit, right now, really just replaces the name!
MAX_EA_NAME_LEN was set 256, so presumably with the null terminator...
while now it would be set to 260, which seems wrong.

Please just adapt if necessary,... or at least I won't likely be able
to update the patch until in about 2 weeks or so.


Thanks,
Philippe
From b64b9a48c77326ed2544e51422adbe1f1c631542 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 a860212533b2c438832ea419fc23537d05ea2210 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 | 18 --
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/winsup/cygwin/ntea.cc b/winsup/cygwin/ntea.cc
index a400fcb2b..aafecde59 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,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
+		+ 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 +53,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];
 
   __try
 {
@@ -95,7 +93,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)
 	{
 	  set_errno (EINVAL);
 	  __leave;
@@ -197,7 +195,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 * 2];
 		  char *tp = stpcpy (tmpbuf, "user.");
 		  stpcpy (tp, fea->EaName);
 		  /* NTFS stores al