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

[Philippe Cerfon <philcerf@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1434 bytes --]

Hey Corinna, et al.

On Mon, Jun 5, 2023 at 9:05 PM Corinna Vinschen
<corinna-cygwin@cygwin.com> 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

[-- Attachment #2: 0002-Cygwin-use-new-XATTR_-NAME-SIZE-_MAX-instead-of-MAX_.patch --]
[-- Type: text/x-patch, Size: 3022 bytes --]

From a860212533b2c438832ea419fc23537d05ea2210 Mon Sep 17 00:00:00 2001
From: Philippe Cerfon <philcerf@gmail.com>
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 <philcerf@gmail.com>
---
 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 <stdlib.h>
 #include <attr/xattr.h>
-
-#define MAX_EA_NAME_LEN    256
-#define MAX_EA_VALUE_LEN 65536
+#include <cygwin/limits.h>
 
 /* 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 all EA names in uppercase unfortunately.  To
@@ -297,7 +295,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)
 	{
 	  set_errno (EINVAL);
 	  __leave;
-- 
2.40.1


[-- Attachment #3: 0001-Cygwin-export-XATTR_-NAME-SIZE-LIST-_MAX.patch --]
[-- Type: text/x-patch, Size: 1099 bytes --]

From b64b9a48c77326ed2544e51422adbe1f1c631542 Mon Sep 17 00:00:00 2001
From: Philippe Cerfon <philcerf@gmail.com>
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 <philcerf@gmail.com>
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
---
 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