This issue tracker has been migrated to GitHub ,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2007年07月04日 14:21 by nbecker, last changed 2022年04月11日 14:56 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| posixmodule-chown-dynamic.patch | jafo, 2008年01月16日 08:37 | Dynamically figure out the size of uid_t/gid_t. | ||
| Messages (13) | |||
|---|---|---|---|
| msg32445 - (view) | Author: Neal D. Becker (nbecker) | Date: 2007年07月04日 14:21 | |
python-2.5 on fedora fc7 x86_64:
os.stat returns uid > 32bit:
os.stat ('shit')
(33204, 4420723, 64768L, 1, 4294967294, 500, 0, 1183558507, 1183558507, 1183558517)
os.chown doesn't like that:
os.chown ('shit', 4294967294, 4294967294)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
OverflowError: signed integer is greater than maximum
|
|||
| msg32446 - (view) | Author: Andrew Ferguson (owsla) | Date: 2007年07月04日 15:04 | |
Indeed, in Modules/posixmodule.c::posix_chown(), the uid and gid variables are defined as type 'int'. They should be of type 'uid_t' and 'gid_t' which are mapped to 'unsigned int' on at least some Unix platforms (I haven't checked extensively.) The wrinkle here is that chown() needs to be able to handle the case of uid/gid set to -1, which means to leave them as is. Therefore, os.chown(-1, -1) is valid, but so is os.chown(4294967294, 4294967294). |
|||
| msg32447 - (view) | Author: Martin v. Löwis (loewis) * (Python committer) | Date: 2007年07月08日 10:29 | |
Notice that the value really is -2. It's important to find out how uid_t is defined on the platform; it may be that the return value of stat is already incorrect. Merely changing the variables to uid_t and gid_t is not enough, since then ParseTuple would stop working correctly. Is anybody interested in providing a patch? |
|||
| msg32448 - (view) | Author: Andrew Ferguson (owsla) | Date: 2007年07月08日 13:51 | |
No, the return value of stat is correct. For that it does: PyInt_FromLong((long)st->st_uid) in _pystat_fromstructstat(STRUCT_STAT *st) (same file, posixmodule.c). Fedora has been defining the UID of the nfsnobody user on 32-bit to be 65534 (USHRT_MAX - 1) and on 64-bit to be 4294967294 (UINT_MAX - 1), assuming 32-bit ints. So, yes, this absurdly high UID is real. So that chown('foo', -1, -1) makes sense, the UID that would be "(uid_t) -1" is reserved. That's why Fedora went for "(uid_t) -2". I think a patch should look something like: $ diff -p posixmodule.c.orig posixmodule.c *** posixmodule.c.orig Sun Jul 8 09:43:50 2007 --- posixmodule.c Sun Jul 8 09:48:27 2007 *************** static PyObject * *** 1826,1834 **** posix_chown(PyObject *self, PyObject *args) { char *path = NULL; ! int uid, gid; int res; ! if (!PyArg_ParseTuple(args, "etii:chown", Py_FileSystemDefaultEncoding, &path, &uid, &gid)) return NULL; --- 1826,1834 ---- posix_chown(PyObject *self, PyObject *args) { char *path = NULL; ! unsigned int uid, gid; int res; ! if (!PyArg_ParseTuple(args, "etII:chown", Py_FileSystemDefaultEncoding, &path, &uid, &gid)) return NULL; |
|||
| msg32449 - (view) | Author: Andrew Ferguson (owsla) | Date: 2007年07月08日 13:58 | |
Reformatted sample patch:
--- posixmodule.c.orig 2007年07月08日 09:43:50.000000000 -0400
+++ posixmodule.c 2007年07月08日 09:48:27.000000000 -0400
@@ -1826,9 +1826,9 @@
posix_chown(PyObject *self, PyObject *args)
{
char *path = NULL;
- int uid, gid;
+ unsigned int uid, gid;
int res;
- if (!PyArg_ParseTuple(args, "etii:chown",
+ if (!PyArg_ParseTuple(args, "etII:chown",
Py_FileSystemDefaultEncoding, &path,
&uid, &gid))
return NULL;
|
|||
| msg59991 - (view) | Author: Martin v. Löwis (loewis) * (Python committer) | Date: 2008年01月16日 07:40 | |
I believe that patch would break on a system where uid_t is a 64-bit value, yet unsigned int is 32 bits. |
|||
| msg59992 - (view) | Author: Sean Reifschneider (jafo) * (Python committer) | Date: 2008年01月16日 08:37 | |
I've reviewed the chown system call under Linux and I think the included
patch will resolve the problem. With that patch on a Fedora 8 64-bit
system I'm getting:
>>> os.stat('/tmp/foo')
posix.stat_result(st_mode=33188, st_ino=5111823, st_dev=64769L,
st_nlink=1, st_uid=0, st_gid=0, st_size=0, st_atime=1200469633,
st_mtime=1200469633, st_ctime=1200469657)
>>> os.chown('/tmp/foo', 4294967294, -1)
>>> os.stat('/tmp/foo')
posix.stat_result(st_mode=33188, st_ino=5111823, st_dev=64769L,
st_nlink=1, st_uid=4294967294, st_gid=0, st_size=0, st_atime=1200469633,
st_mtime=1200469633, st_ctime=1200469683)
>>>
However, it's unclear to me whether there are any platforms that might
define uid_t as signed, and if so whether this code would do the right
thing on those platforms.
I don't know of a way to do it in C off hand, but perhaps we could check
the exact type of the uid_t and gid_t types and see if they're signed or
unsigned and use that combined with sizeof(uid_t) to use exactly the
correct ParseTuple format string.
I've attached a patch that dynamically tries to figure out the size to
use. Does this seem overly-heavy? If it seems appropriate, the same
would need to be applied to the other chown functions.
|
|||
| msg59995 - (view) | Author: Andrew Ferguson (owsla) | Date: 2008年01月16日 12:55 | |
The idea of dynamic typing it seems quite heavy to me, but I'm not a Python hacker, so I don't know what's the norm. Notice that os.stat() does "PyInt_FromLong((long)st->st_uid)" on the stat structure's st_uid field. On my platform (OS X 10.4), st_uid is defined as a uid_t type. So maybe os.stat() has the answer: ignore the signed vs. unsigned int problem and just use a long. The actual chown() call in posix_chown() casts the uid variable to a (uid_t) anyway. GCC doesn't seem to complain when we cast a long to an unsigned int, even. |
|||
| msg63770 - (view) | Author: Gregory P. Smith (gregory.p.smith) * (Python committer) | Date: 2008年03月17日 20:58 | |
i'll take a look at this during the sprint. |
|||
| msg63964 - (view) | Author: Gregory P. Smith (gregory.p.smith) * (Python committer) | Date: 2008年03月18日 19:06 | |
fixed in trunk r61540. I'm leaving this open until i backport it to release25-maint. |
|||
| msg63971 - (view) | Author: Gregory P. Smith (gregory.p.smith) * (Python committer) | Date: 2008年03月18日 19:26 | |
backported to 2.5 in r61542 and r61544. it'll go into py3k via the regular merges from trunk. The fix just changed the int -> long and 'ii' -> 'll' and added unit test coverage. The patch attached to this bug was rejected as too complex: If conditional treatment of the types is ever needed it should be done at build time via autoconf and not at runtime. |
|||
| msg95916 - (view) | Author: Antoine Pitrou (pitrou) * (Python committer) | Date: 2009年12月02日 20:17 | |
Looking at posixmodule.c, perhaps other instances of parsing an uid_t or a gid_t should have been fixed too (lchown, fchown for example)? |
|||
| msg96832 - (view) | Author: Gregory P. Smith (gregory.p.smith) * (Python committer) | Date: 2009年12月23日 09:47 | |
indeed, those were missed. fixed in trunk r77007 and release26-maint r77008. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022年04月11日 14:56:25 | admin | set | github: 45149 |
| 2009年12月23日 09:47:07 | gregory.p.smith | set | messages: + msg96832 |
| 2009年12月02日 20:17:27 | pitrou | set | nosy:
+ pitrou messages: + msg95916 |
| 2008年03月18日 19:26:32 | gregory.p.smith | set | status: open -> closed keywords: - patch messages: + msg63971 resolution: remind -> fixed versions: - Python 2.5 |
| 2008年03月18日 19:06:45 | gregory.p.smith | set | resolution: remind messages: + msg63964 versions: - Python 2.6 |
| 2008年03月17日 20:58:36 | gregory.p.smith | set | versions: + Python 2.6, Python 3.0 |
| 2008年03月17日 20:58:25 | gregory.p.smith | set | assignee: gregory.p.smith messages: + msg63770 nosy: + gregory.p.smith |
| 2008年01月16日 12:55:27 | owsla | set | messages: + msg59995 |
| 2008年01月16日 08:37:15 | jafo | set | files:
+ posixmodule-chown-dynamic.patch messages: + msg59992 |
| 2008年01月16日 07:40:48 | loewis | set | messages: + msg59991 |
| 2008年01月16日 07:30:32 | jafo | set | messages: - msg59990 |
| 2008年01月16日 07:10:53 | jafo | set | keywords:
+ patch, 64bit nosy: + jafo type: behavior messages: + msg59990 |
| 2007年08月23日 18:49:43 | georg.brandl | link | issue1752703 superseder |
| 2007年07月04日 14:21:37 | nbecker | create | |