[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [bug #28113] chown silently fails to set uid/gid of ([ug]id_t) -1
From: |
Jim Meyering |
Subject: |
Re: [bug #28113] chown silently fails to set uid/gid of ([ug]id_t) -1 |
Date: |
Sat, 28 Nov 2009 07:38:31 +0100 |
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 <address@hidden>
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 <address@hidden>
+ 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