Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FreeBSD] psutil.virtual_memory() fails without COMPAT_FREEBSD7 in the kernel #2113

Closed
peterjeremy opened this issue Jun 12, 2022 · 9 comments

Comments

@peterjeremy
Copy link

Summary

  • OS: FreeBSD 13.1-STABLE
  • Architecture: 64bit (amd64)
  • Psutil version: psutil-5.9.1
  • Python version: Python 3.8.13
  • Type: core

Description

I have a FreeBSD 13.1-STABLE system using a custom kernel without COMPAT_FREEBSD7 and psutil.virtual_memory() fails as follows:

aspire5$ python3.8
Python 3.8.13 (default, May 14 2022, 10:07:39) 
[Clang 13.0.0 ([email protected]:llvm/llvm-project.git llvmorg-13.0.0-0-gd7b669b3a on freebsd13
Type "help", "copyright", "credits" or "license" for more information.
>>> import psutil
>>> psutil.virtual_memory().total
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.8/site-packages/psutil/__init__.py", line 1968, in virtual_memory
    ret = _psplatform.virtual_memory()
  File "/usr/local/lib/python3.8/site-packages/psutil/_psbsd.py", line 181, in virtual_memory
    mem = cext.virtual_mem()
OSError: [Errno 12] Cannot allocate memory (originated from sysctlbyname('vfs.bufspace'))

The problem is that the test at

#if __FreeBSD_version > 702101
occurs without previously including <osreldate.h>, hence __FreeBSD_version is undefined (defaulting to 0), meaning that buffers defaults to int, whereas it should be long for recent FreeBSD versions.

I checked this by taking the compilation command:

cc -Wno-unused-result -Wsign-compare -Wunreachable-code -DNDEBUG -O2 -pipe -march=znver1 -fstack-protector-strong -fno-strict-aliasing -O2 -pipe -march=znver1 -fstack-protector-strong -fno-strict-aliasing -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_BSD=1 -DPSUTIL_SIZEOF_PID_T=4 -DPSUTIL_VERSION=591 -DPSUTIL_FREEBSD=1 -I/usr/local/include/python3.8 -c psutil/arch/freebsd/mem.c -o build/temp.freebsd-13.1-STABLE-amd64-cpython-38/psutil/arch/freebsd/mem.o

and converting it to just preprocess (-E -dD instead of -c -o ...), which generates (in part):

# 18 "psutil/arch/freebsd/mem.c" 2







PyObject *
psutil_virtual_mem(PyObject *self, PyObject *args) {
    unsigned long total;
    unsigned int active, inactive, wired, cached, free;
    size_t size = sizeof(total);
    struct vmtotal vm;
    int mib[] = {2, 1};
    long pagesize = psutil_getpagesize();



    int buffers;

    size_t buffers_size = sizeof(buffers);

note the int buffers;. This works for a GENERIC kernel because that includes COMPAT_FREEBSD7, which means that either int or long is accepted. (The sysctl vfs.bufspace is defined at https://cgit.freebsd.org/src/tree/sys/kern/vfs_bio.c?h=stable/13#n208 and the implementation beginning at https://cgit.freebsd.org/src/tree/sys/kern/vfs_bio.c?h=stable/13#n469 tests whether COMPAT_FREEBSD7 is defined).

Searching through the code, there's no include of <osreldate.h> anywhere so I suspect none of the __FreeBSD_version tests work as intended.

@giampaolo
Copy link
Owner

Searching through the code, there's no include of <osreldate.h> anywhere so I suspect none of the __FreeBSD_version tests work as intended.

This is bad. Does adding #include <osreldate.h> in psutil/psutil/arch/freebsd/mem.c fix the issue?
If so I guess we probably have to do that in all the files using __FreeBSD_version.
Could you verify and open a PR if this is the case?

Thanks

@peterjeremy
Copy link
Author

Looking further, either <sys/param.h> or <osreldate.h> will define __FreeBSD_version and #includeing either in psutil/psutil/arch/freebsd/mem.c fixes the issue. The latter is probably preferable because it doesn't define other "baggage".

Looking at the code, there are references to __FreeBSD_version as follows:

psutil/_psutil_bsd.c:77:    #if __FreeBSD_version < 900000
psutil/_psutil_bsd.c:273:#if defined(__FreeBSD_version) && __FreeBSD_version >= 1200031
psutil/_psutil_bsd.c:454:#if (defined(__FreeBSD_version) && __FreeBSD_version >= 700000)
psutil/_psutil_bsd.c:588:#if (defined(__FreeBSD_version) && __FreeBSD_version >= 800000) || PSUTIL_OPENBSD || defined(PSUTIL_NETBSD)
psutil/_psutil_bsd.c:944:#if (defined(__FreeBSD_version) && (__FreeBSD_version < 900000)) || PSUTIL_OPENBSD
psutil/_psutil_bsd.c:974:#if defined(PSUTIL_OPENBSD) || (defined(__FreeBSD_version) && __FreeBSD_version < 900000)
psutil/_psutil_bsd.c:1071:#if defined(__FreeBSD_version) && __FreeBSD_version >= 800000 || PSUTIL_OPENBSD || defined(PSUTIL_NETBSD)
psutil/arch/freebsd/mem.c:33:#if __FreeBSD_version > 702101
psutil/arch/freebsd/proc.c:356:#if defined(__FreeBSD_version) && __FreeBSD_version >= 701000
psutil/arch/freebsd/proc.c:407:#if defined(__FreeBSD_version) && __FreeBSD_version >= 800000
psutil/arch/freebsd/proc_socks.c:102:#if __FreeBSD_version >= 1200026
psutil/arch/freebsd/proc_socks.c:121:#if __FreeBSD_version >= 1200026
psutil/arch/freebsd/proc_socks.c:139:#if __FreeBSD_version < 1200031
psutil/arch/freebsd/proc_socks.c:147:#if __FreeBSD_version < 1200031
psutil/arch/freebsd/proc_socks.c:156:#if __FreeBSD_version < 1200031
psutil/arch/freebsd/proc_socks.c:164:#if __FreeBSD_version < 1200031
psutil/arch/freebsd/proc_socks.c:188:#if __FreeBSD_version >= 1200026
psutil/arch/freebsd/proc_socks.c:265:#if __FreeBSD_version < 1200031
psutil/arch/freebsd/proc_socks.c:275:#if __FreeBSD_version < 1200031
psutil/arch/freebsd/proc_socks.c:283:#if __FreeBSD_version < 1200031
psutil/arch/freebsd/proc_socks.c:289:#if __FreeBSD_version < 1200031
psutil/arch/freebsd/proc_socks.c:325:#if __FreeBSD_version < 1200031
psutil/arch/freebsd/sys_socks.c:19:#if defined(__FreeBSD_version) && __FreeBSD_version < 800000
psutil/arch/freebsd/sys_socks.c:82:#if __FreeBSD_version >= 1200026
psutil/arch/freebsd/sys_socks.c:151:#if __FreeBSD_version >= 1200026
psutil/arch/freebsd/sys_socks.c:166:#if __FreeBSD_version >= 1200026

together with the following references to <sys/param.h>:

psutil/_psutil_bsd.c:#include <sys/param.h>
psutil/arch/openbsd/proc.c:#include <sys/param.h>
psutil/arch/openbsd/mem.c:#include <sys/param.h>
psutil/arch/netbsd/specific.c:#include <sys/param.h>
psutil/arch/freebsd/proc.c:#include <sys/param.h>

so, the delta is psutil/arch/freebsd/mem.c, psutil/arch/freebsd/proc_socks.c and psutil/arch/freebsd/sys_socks.c.

Looking at preprocessor output, psutil/arch/freebsd/proc_socks.c and psutil/arch/freebsd/sys_socks.c both indirectly define __FreeBSD_version via <sys/user.h>, <sys/ucred.h>, <bsm/audit.h>, <sys/param.h>, so the only definite problem is psutil/arch/freebsd/mem.c, though the long indirect chain in the other 2 files is undesirable. I will send a pull request later today.

@koobs
Copy link

koobs commented Jun 19, 2022

@giampaolo
Copy link
Owner

Setting priority = critical. PRs are welcome.

@Torsten-B
Copy link
Contributor

I've been affected by this on FreeBSD as well. Yes, including <sys/param.h> or <osreldate.h> in mem.c fixes the problem.

Please note that COMPAT_FREEBSD7 is in the GENERIC kernel only on Intel. Other platforms, including arm64, came later and thus don't include COMPAT_FREEBSD7.

I've submitted a quick patch to the FreeBSD ports system to un-break the port quickly: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=264807

freebsd-git pushed a commit to freebsd/freebsd-ports that referenced this issue Jun 21, 2022
- Bump PORTREVISION for package change

psutil 5.9.1 does not include <sys/param.h> in arch/freebsd/mem.c, causing the
__FreeBSD_version check there not not work properly and thus breaking the port
on systems without COMPAT_FREEBSD7.

PR:		264807
Reference:	giampaolo/psutil#2113
@koobs
Copy link

koobs commented Jun 23, 2022

@Torsten-B Thanks for your patch, are you able to produce a PR here for

I also see other FreeBSD_version instances in other files (some including sysctl.h as well,. but none also including param.h)

Might we have issues elsewhere in psutil too? cc @giampaolo

@Torsten-B
Copy link
Contributor

@Torsten-B Thanks for your patch, are you able to produce a PR here for

I also see other FreeBSD_version instances in other files (some including sysctl.h as well,. but none also including param.h)
Might we have issues elsewhere in psutil too? cc @giampaolo

I've tested that before submitting the PR to the FreeBSD ports tree and all cases in pstuil/arch/freebsd/ that had a __FreeBSD_version check either already included it directly or ended up pulling in sys/param.h indirectly via other included headers.
That said, even though it's not needed elsewhere right now, it might not be a bad idea to include <sys/param.h> explicitly in all source files that check __FreeBSD_version.

I'll create a PR tonight my time (UTC+0200)

@Torsten-B
Copy link
Contributor

Pull request: #2114

@giampaolo
Copy link
Owner

Fixed by #2114.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants