Fix types for dev_t processing in posix module: use uint64_t (unsigned) instead of int64_t (signed)

[AMDmi3]

BPO	45767
Nosy	@serhiy-storchaka, @koobs, @AMDmi3
PRs	gh-89928: consume dev_t with PyLong_FromUnsignedLongLong #29494 gh-89928: Fix integer conversion of device numbers. #31794
Files	posixmodule.c.patch: Patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2021-11-09.17:13:09.157>
labels = ['extension-modules', 'type-bug', '3.9', '3.10', '3.11']
title = 'Fix types for dev_t processing in posix module'
updated_at = <Date 2022-03-10.14:54:37.748>
user = 'https://github.com/AMDmi3'

bugs.python.org fields:

activity = <Date 2022-03-10.14:54:37.748>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Extension Modules', 'FreeBSD']
creation = <Date 2021-11-09.17:13:09.157>
creator = 'AMDmi3'
dependencies = []
files = ['50433']
hgrepos = []
issue_num = 45767
keywords = ['patch']
message_count = 5.0
messages = ['406030', '406051', '411022', '411036', '414851']
nosy_count = 3.0
nosy_names = ['serhiy.storchaka', 'koobs', 'AMDmi3']
pr_nums = ['29494', '31794']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue45767'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

Linked PRs

• [3.13] gh-89928: Fix integer conversion of device numbers (GH-31794) #120053
• [3.12] gh-89928: Fix integer conversion of device numbers (GH-31794) #120054

[AMDmi3]

So, I was investigating a test failure of python 3.11 and 3.10 on FreeBSD (but it likely applies to all python versions):

======================================================================
FAIL: test_makedev (test.test_posix.PosixTester)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/usr/ports/lang/python311/work/Python-3.11.0a1/Lib/test/test_posix.py", line 686, in test_makedev
    self.assertGreaterEqual(dev, 0)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: -5228656221359548839 not greater than or equal to 0

---

The test checks that posix.stat(somepath).st_dev >= 0, but negative value was returned.

Python uses PyLong_FromLongLong to convert from dev_t C type which st_dev is:

https://github.com/python/cpython/blob/main/Modules/posixmodule.c#L2410
https://github.com/python/cpython/blob/main/Modules/posixmodule.c#L901

POSIX does not seem to define signedness of dev_t,

https://pubs.opengroup.org/onlinepubs/9699919799.2018edition/basedefs/sys_types.h.html

only saying it shall be an "integer type". However on practice on both FreeBSD and Linux it's unsigned:

FreeBSD:
typedef __dev_t dev_t; // sys/types.h
typedef __uint64_t __dev_t; // sys/_types.h

Linux (Ubuntu 18.04):
typedef __dev_t dev_t;             // sys/stat.h
__STD_TYPE __DEV_T_TYPE __dev_t;   // sys/types.h
#define __DEV_T_TYPE            __UQUAD_TYPE;  // sys/typesizes.h

So I suggest the attached patch to switch _PyLong_FromDev to PyLong_FromUnsignedLongLong which also makes it consistent with _Py_Dev_Converter which converts the other way with PyLong_AsUnsignedLongLong.

This change fixes the mentioned test, but another test failure is unmasked:

======================================================================
ERROR: test_makedev (test.test_posix.PosixTester)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/usr/ports/lang/python311/work/Python-3.11.0a1/Lib/test/test_posix.py", line 704, in test_makedev
    self.assertEqual(posix.makedev(major, minor), dev)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
OverflowError: Python int too large to convert to C int

---

This problem needs couple more trivial changes, but I'm not sure how to make them correctly (and I'm also confused by how this file relates with clinic/posixmodule.c).

The problems are that:

• os_major_impl/os_minor_impl and os_makedev_impl are inconsistent in their argument/return types for major/minor dev numbers: the former use unsigned int, while the latter uses int.
• the correct type is platform dependent, for instance Linux uses unsigned int and FreeBSD uses int (from man makedev).

I guess that to fix this failure one needs to add a macro/typedef for the type for minor/major dev numbers like

#if defined(__FreeBSD__)
#define DEVICE_MAJORMINOR_T int
#else
#define DEVICE_MAJORMINOR_T unsigned int
#endif

and use it in the named functions:

static DEVICE_MAJORMINOR_T os_major_impl(PyObject *module, dev_t device)
static DEVICE_MAJORMINOR_T os_minor_impl(PyObject *module, dev_t device)

static dev_t os_makedev_impl(PyObject *module, DEVICE_MAJORMINOR_T major, DEVICE_MAJORMINOR_T minor)

[AMDmi3]

In case you're curious, here are some st_dev values which are encountered on my FreeBSD:

• 0xac2308de99d1f699 - ZFS
• 0x7100ff00 - devfs
• 0xffffffff8700ff01 - tmpfs
• 0x2900ff4e - nullfs
  I suspect Linux uses smaller numbers which do not cause overflows so tese tests hasn't fired on Linux.

[serhiy-storchaka]

Is device number -1 used in any context (for example as "unknown device number")?

[AMDmi3]

Is device number -1 used in any context (for example as "unknown device number")?

Yes, there's NODEV macro in both Linux and FreeBSD which expands to ((dev_t)-1).

[serhiy-storchaka]

PR 31794 supports NODEV and checks for integer overflows.

[vstinner]

I closed the FreeBSD issue #113078 as a duplicate of this issue.

[vstinner]

Thanks for the fix @serhiy-storchaka!