Matt McCutchen wrote: > <http://savannah.gnu.org/bugs/?28113> > > Summary: chown silently fails to set uid/gid of ([ug]id_t) -1 > > Details: > > chown(2) and related system calls treat ([ug]id_t) -1 as a special value > meaning "don't change this field". If chown(1) is called with a uid or gid of > ([ug]id_t) -1 (typically 4294967295), it naively passes the value on to the > system call and, as a result, silently fails to do what the user asked. It > should issue an error message in this case. If the user had actually wanted > to not change a particular field, he/she would have used the appropriate > chown(1) syntax: "OWNER:" or ":GROUP". > > Steps to reproduce: > $ touch test > $ chown 4294967295:4294967295 test > No error, but "test" still has the same ownership as before. > > Tested with Fedora coreutils-7.2-4.fc11.x86_64 and with the latest coreutils > from the repository. > > The problem came up at: > https://sourceforge.net/mailarchive/forum.php?thread_name=1259202707.2009.382.camel%40mattlaptop2.local&forum_name=rsnapshot-discuss
Thanks for the bug report. I've begun addressing this. The first step is to make gnulib's userspec module do the right thing: [then I'll add unit tests in gnulib and coreutils] >From 2ead6c09f1bda238b1ccd923f9983b7744581c83 Mon Sep 17 00:00:00 2001 From: Jim Meyering <[email protected]> Date: Sat, 28 Nov 2009 07:33:16 +0100 Subject: [PATCH] userspec: disallow an ID that maps to (uid_t)-1 or (gid_t)-1 * lib/userspec.c (parse_with_separator): Do not accept a user ID number of MAXUID when it evaluates to (uid_t) -1. Likewise for group ID. Reported by Matt McCutchen in <http://savannah.gnu.org/bugs/?28113> --- ChangeLog | 6 ++++++ lib/userspec.c | 5 +++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index e274ef7..0f130f5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ 2009-11-28 Jim Meyering <[email protected]> + userspec: disallow an ID that maps to (uid_t)-1 or (gid_t)-1 + * lib/userspec.c (parse_with_separator): Do not accept a user ID + number of MAXUID when it evaluates to (uid_t) -1. + Likewise for group ID. Reported by Matt McCutchen in + <http://savannah.gnu.org/bugs/?28113> + userspec: reformat to use spaces, not TABs * lib/userspec.c: Expand TABs to spaces. Add Emacs' "indent-tabs-mode: nil" hint. diff --git a/lib/userspec.c b/lib/userspec.c index fa2f26f..0388cb1 100644 --- a/lib/userspec.c +++ b/lib/userspec.c @@ -169,7 +169,7 @@ parse_with_separator (char const *spec, char const *separator, { unsigned long int tmp; if (xstrtoul (u, NULL, 10, &tmp, "") == LONGINT_OK - && tmp <= MAXUID) + && tmp <= MAXUID && (uid_t) tmp != (uid_t) -1) unum = tmp; else error_msg = E_invalid_user; @@ -200,7 +200,8 @@ parse_with_separator (char const *spec, char const *separator, if (grp == NULL) { unsigned long int tmp; - if (xstrtoul (g, NULL, 10, &tmp, "") == LONGINT_OK && tmp <= MAXGID) + if (xstrtoul (g, NULL, 10, &tmp, "") == LONGINT_OK + && tmp <= MAXGID && (gid_t) tmp != (gid_t) -1) gnum = tmp; else error_msg = E_invalid_group; -- 1.6.6.rc0.285.g73651
