From bfade9b8bc76b5deaf153c97a1ba27286fa66af1 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 28 Aug 2023 22:47:44 +0200 Subject: [PATCH] msys2-runtime: restore fast path for current user primary group A recent commit removed an optimization from the Cygwin runtime that fetches the default group from the current user token: At times, this is not accurate such as when groups like the builtin Administrators group is the primary group. However, removing this optimization causes extremely poor performance when connected to some Active Directory environments. Reflecting https://github.com/git-for-windows/msys2-runtime/pull/57, this update restores this optimization as the default behaviour. To still allow the user to opt into the correct behavior, a `group: db-accurate` option is provided for `nsswitch.conf`. This fixes https://github.com/git-for-windows/git/issues/4459 Signed-off-by: Johannes Schindelin --- ...store-fast-path-for-current-user-pri.patch | 156 ++++++++++++++++++ msys2-runtime/PKGBUILD | 13 +- msys2-runtime/msys2-runtime.commit | 2 +- 3 files changed, 165 insertions(+), 6 deletions(-) create mode 100644 msys2-runtime/0063-msys2-runtime-restore-fast-path-for-current-user-pri.patch diff --git a/msys2-runtime/0063-msys2-runtime-restore-fast-path-for-current-user-pri.patch b/msys2-runtime/0063-msys2-runtime-restore-fast-path-for-current-user-pri.patch new file mode 100644 index 00000000000..e7f03301b59 --- /dev/null +++ b/msys2-runtime/0063-msys2-runtime-restore-fast-path-for-current-user-pri.patch @@ -0,0 +1,156 @@ +From eb7ffd4130fc5ddb655e0eb8e5375052cd96d69b Mon Sep 17 00:00:00 2001 +From: Richard Glidden +Date: Thu, 24 Aug 2023 13:36:10 -0400 +Subject: [PATCH 63/N] msys2-runtime: restore fast path for current user + primary group + +Commit a5bcfe616c7e removed an optimization that fetches the +default group from the current user token, as it is sometimes +not accurate such as when groups like the builtin +Administrators group is the primary group. + +However, removing this optimization causes extremely poor +performance when connected to some Active Directory +environments. + +Restored this optimization as the default behaviour, and +added a `group: db-accurate` option to `nsswitch.conf` that +can be used to disable the optimization in cases where +accurate group information is required. + +This fixes https://github.com/git-for-windows/git/issues/4459 + +Signed-off-by: Richard Glidden +--- + winsup/cygwin/include/sys/cygwin.h | 3 ++- + winsup/cygwin/local_includes/cygheap.h | 1 + + winsup/cygwin/uinfo.cc | 30 ++++++++++++++++++++------ + winsup/doc/ntsec.xml | 20 ++++++++++++++++- + 4 files changed, 46 insertions(+), 8 deletions(-) + +diff --git a/winsup/cygwin/include/sys/cygwin.h b/winsup/cygwin/include/sys/cygwin.h +index 6383646..6b2f647 100644 +--- a/winsup/cygwin/include/sys/cygwin.h ++++ b/winsup/cygwin/include/sys/cygwin.h +@@ -219,7 +219,8 @@ enum + enum + { + NSS_SRC_FILES = 1, +- NSS_SRC_DB = 2 ++ NSS_SRC_DB = 2, ++ NSS_SRC_DB_ACCURATE = 4 + }; + + /* Enumeration source constants for CW_SETENT called from mkpasswd/mkgroup. */ +diff --git a/winsup/cygwin/local_includes/cygheap.h b/winsup/cygwin/local_includes/cygheap.h +index f5aecba..c33f378 100644 +--- a/winsup/cygwin/local_includes/cygheap.h ++++ b/winsup/cygwin/local_includes/cygheap.h +@@ -454,6 +454,7 @@ public: + inline int nss_pwd_src () const { return pwd_src; } /* CW_GETNSS_PWD_SRC */ + inline bool nss_grp_files () const { return !!(grp_src & NSS_SRC_FILES); } + inline bool nss_grp_db () const { return !!(grp_src & NSS_SRC_DB); } ++ inline bool nss_grp_db_accurate () const { return !!(grp_src & NSS_SRC_DB_ACCURATE); } + inline int nss_grp_src () const { return grp_src; } /* CW_GETNSS_GRP_SRC */ + inline bool nss_cygserver_caching () const { return caching; } + inline void nss_disable_cygserver_caching () { caching = false; } +diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc +index a25f39f..fefd984 100644 +--- a/winsup/cygwin/uinfo.cc ++++ b/winsup/cygwin/uinfo.cc +@@ -637,6 +637,11 @@ cygheap_pwdgrp::nss_init_line (const char *line) + *src |= NSS_SRC_DB; + c += 2; + } ++ else if (NSS_CMP ("db-accurate")) ++ { ++ *src |= NSS_SRC_DB | NSS_SRC_DB_ACCURATE; ++ c += 11; ++ } + else + { + c += strcspn (c, " \t"); +@@ -1906,6 +1911,7 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap) + gid_t gid = ILLEGAL_GID; + bool is_domain_account = true; + PCWSTR domain = NULL; ++ bool get_default_group_from_current_user_token = false; + char *shell = NULL; + char *home = NULL; + char *gecos = NULL; +@@ -2371,9 +2377,19 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap) + uid = posix_offset + sid_sub_auth_rid (sid); + if (!is_group () && acc_type == SidTypeUser) + { +- /* Default primary group. Make the educated guess that the user +- is in group "Domain Users" or "None". */ +- gid = posix_offset + DOMAIN_GROUP_RID_USERS; ++ /* Default primary group. If the sid is the current user, and ++ we are not configured for accurate mode, fetch ++ the default group from the current user token, otherwise make ++ the educated guess that the user is in group "Domain Users" ++ or "None". */ ++ if (!cygheap->pg.nss_grp_db_accurate() && sid == cygheap->user.sid ()) ++ { ++ get_default_group_from_current_user_token = true; ++ gid = posix_offset ++ + sid_sub_auth_rid (cygheap->user.groups.pgsid); ++ } ++ else ++ gid = posix_offset + DOMAIN_GROUP_RID_USERS; + } + + if (is_domain_account) +@@ -2384,9 +2400,11 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap) + /* On AD machines, use LDAP to fetch domain account infos. */ + if (cygheap->dom.primary_dns_name ()) + { +- /* Fetch primary group from AD and overwrite the one we +- just guessed above. */ +- if (cldap->fetch_ad_account (sid, false, domain)) ++ /* For the current user we got correctly cased username and ++ the primary group via process token. For any other user ++ we fetch it from AD and overwrite it. */ ++ if (!get_default_group_from_current_user_token ++ && cldap->fetch_ad_account (sid, false, domain)) + { + if ((val = cldap->get_account_name ())) + wcscpy (name, val); +diff --git a/winsup/doc/ntsec.xml b/winsup/doc/ntsec.xml +index c6871ec..c95207e 100644 +--- a/winsup/doc/ntsec.xml ++++ b/winsup/doc/ntsec.xml +@@ -930,7 +930,16 @@ The two lines starting with the keywords passwd: and + information from. files means, fetch the information + from the corresponding file in the /etc directory. db + means, fetch the information from the Windows account databases, the SAM +-for local accounts, Active Directory for domain account. Examples: ++for local accounts, Active Directory for domain account. For the current ++user, the default group is obtained from the current user token to avoid ++additional lookups to the group database. db-accuarte ++is only valid on group: line, and performs the same ++lookups as the db option, but disables using the ++current user token to retrieve the default group as this optimization ++is not accurate in all cases. For example, if you run a native process ++with the primary group set to the Administrators builtin group, the ++db option will return a non-existent group as primary ++group. Examples: + + + +@@ -949,6 +958,15 @@ Read passwd entries only from /etc/passwd. + Read group entries only from SAM/AD. + + ++ ++ group: db-accurate ++ ++ ++ ++Read group entries only from SAM/AD. Force the use of the group database ++for the current user. ++ ++ + + group: files # db + diff --git a/msys2-runtime/PKGBUILD b/msys2-runtime/PKGBUILD index 8a24f0a4cd6..5d9ac396561 100644 --- a/msys2-runtime/PKGBUILD +++ b/msys2-runtime/PKGBUILD @@ -4,7 +4,7 @@ pkgbase=msys2-runtime pkgname=('msys2-runtime' 'msys2-runtime-devel') pkgver=3.4.7 -pkgrel=2 +pkgrel=3 pkgdesc="Cygwin POSIX emulation engine" arch=('x86_64') url="https://www.cygwin.com/" @@ -88,9 +88,10 @@ source=('msys2-runtime'::git+https://github.com/cygwin/cygwin#tag=cygwin-${pkgve 0059-uname-report-msys2-runtime-commit-hash-too.patch 0060-Revert-Cygwin-pipe-Restore-blocking-mode-for-cygwin-.patch 0061-fixup-uname-report-msys2-runtime-commit-hash-too.patch - 0062-fchmodat-fstatat-fix-regression-with-empty-pathname.patch) + 0062-fchmodat-fstatat-fix-regression-with-empty-pathname.patch + 0063-msys2-runtime-restore-fast-path-for-current-user-pri.patch) sha256sums=('SKIP' - '21e4c6cd8959a964f0c2a39831a9ce569e8695818578ea5d393b8efefeae0c85' + '7b8477f768f5e07d89ff513c812a755d5db06e50ed65f598363e15f3d554d117' 'c092f44f33e526c93282de5f1b12628665b38d607c2e7628d8437634ce9d133f' 'a19b83098b577f64b7df42197ada140b58601f0acb20feba304f4dcdce8563c7' '8c71c0477544aec58d4c213c59e273f6faf94fd8691248276af2f77ffec1b481' @@ -152,7 +153,8 @@ sha256sums=('SKIP' 'b1cc58ff353fa74bca23359c62687a19cf9067b16c123ecfe016496a7d276b4b' '1f9a2cfd1ad42e3b101028d10555a8fe4782ba2b0df931d8e23784d2c76df6ad' 'abc44c284023fd58aa94ce4c3fdbcde7f690fd3dbfd05b7dca05c70a14f07ef6' - '257a8347537ace85c1491fb48ea51a65b83dca4bb99b7a7ab2146afb828538ef') + '257a8347537ace85c1491fb48ea51a65b83dca4bb99b7a7ab2146afb828538ef' + '6f1fc4f57622a984704d2df652d647fc960cb9c83f53422a69c97b0fac4deab5') # Helper macros to help make tasks easier # apply_patch_with_msg() { @@ -268,7 +270,8 @@ prepare() { 0059-uname-report-msys2-runtime-commit-hash-too.patch \ 0060-Revert-Cygwin-pipe-Restore-blocking-mode-for-cygwin-.patch \ 0061-fixup-uname-report-msys2-runtime-commit-hash-too.patch \ - 0062-fchmodat-fstatat-fix-regression-with-empty-pathname.patch + 0062-fchmodat-fstatat-fix-regression-with-empty-pathname.patch \ + 0063-msys2-runtime-restore-fast-path-for-current-user-pri.patch } build() { diff --git a/msys2-runtime/msys2-runtime.commit b/msys2-runtime/msys2-runtime.commit index ca93ac57d6c..3727875fba9 100644 --- a/msys2-runtime/msys2-runtime.commit +++ b/msys2-runtime/msys2-runtime.commit @@ -1 +1 @@ -ea78182979f94b592f6344f3fd26f5e129f9e819 +25de8b84ec4bc30622b5c70babe0eea6fad3ea8a