[Windows] Ignore permissions errors on disk_io_counters

[mx-psi]

Summary

• OS: Windows
• Type: core

Description

On some Windows disk configurations psutil.disk_io_counters can raise a PermissionError: [WinError 5] due to a DeviceIoControl trying to access a disk for which it does not have permissions. To be able to access the rest of the disks info, I see two ways out of this

• (Option 1) Ignore WinError 5 like we do with other errors

  psutil/psutil/arch/windows/disk.c

  Lines 137 to 142 in a096704

  	else if (GetLastError() == ERROR_NOT_SUPPORTED) {
  	// Again, let's assume we're dealing with some exotic disk.
  	psutil_debug("DeviceIoControl -> ERROR_NOT_SUPPORTED; "
  	"ignore PhysicalDrive%i", devNum);
  	goto next;
  	}

• (Option 2) add an off-by-default ignore_errors flag to disk_io_counters to ignore the errors from DeviceIOControl on Windows (and no-op on other platforms).

I am happy to do a PR for either of these but I would love to know what maintainers and contributors think is the best option first.

cc @giampaolo, could you give me some feedback as to what would be preferable?

[giampaolo]

Hmm. I understand the pain, but I'm not thrilled at the idea of returning different results depending on what user is logged in. Have you tried searching if there are workarounds? With a quick search I found this (but didn't investigate any further):
https://stackoverflow.com/questions/61562617/deviceiocontrol-returning-error-access-denied

[giampaolo]

I suspect we may avoid access denied if we change the permissions passed to CreateFile. E.g. what happens if you change FILE_SHARE_READ | FILE_SHARE_WRITE to FILE_SHARE_READ?

    for (devNum = 0; devNum <= 32; ++devNum) {
        py_tuple = NULL;
        sprintf_s(szDevice, MAX_PATH, "\\\\.\\PhysicalDrive%d", devNum);
        hDevice = CreateFile(szDevice, 0, FILE_SHARE_READ | FILE_SHARE_WRITE,
                             NULL, OPEN_EXISTING, 0, NULL);
        if (hDevice == INVALID_HANDLE_VALUE)
            continue;

        // DeviceIoControl() sucks!
        i = 0;
        ioctrlSize = sizeof(diskPerformance);
        while (1) {
            i += 1;
            ret = DeviceIoControl(
                hDevice, IOCTL_DISK_PERFORMANCE, NULL, 0, &diskPerformance,
                ioctrlSize, &dwSize, NULL);
            if (ret != 0)

[giampaolo]

Another possible solution: https://community.osr.com/discussion/65313/deviceiocontrol-from-user-mode
In here (it seems) it says IoCreateDeviceSecure could be used as a replacement for CreateFile.

Another possibility is conducting tests by trying passing different parameters to CreateFile.

Question: for what drive do you get ERROR_ACCESS_DENIED? Is it a regular disk? A usb key...?

[mx-psi]

Thanks for having a look, I will try to loop back with the person who reported this on our end (we use psutil on the Datadog Agent) and get the answer to those

[mx-psi]

Hey there @giampaolo, so I haven't been able to get back to the person who originally reported this to us, but I did take a look at your suggestions and generally considered the problem, and I have some comments:

1. what happens if you change FILE_SHARE_READ | FILE_SHARE_WRITE to FILE_SHARE_READ?

   This works fine for me on a VM without the permissions problem. I am not sure how to reproduce the permissions problem, so I don't know if this would fix the issue. However, the 'Remarks' section on the docs for DeviceIoControl says

   You should specify the FILE_SHARE_READ and FILE_SHARE_WRITE access flags when calling CreateFile to open a handle to a device driver

   so I would be worried about breaking things in some unexpected edge case.

2. Another possible solution: https://community.osr.com/discussion/65313/deviceiocontrol-from-user-mode
   In here (it seems) it says IoCreateDeviceSecure could be used as a replacement for CreateFile.

   I checked this with someone with more Windows-fu than I do and it looks like the IoCreateDeviceSecure can only be used in kernel-mode drivers since it needs a PDRIVER_OBJECT (see here for more). So I think this one is a dead-end.

3. I also had a look at how similar projects/forks deal with this issue. I think the overarching issue here is that the disk_io_counters function returns no info about other disks when querying one of them fails. Windows PartitionsWithContext is over cautious with error handling. shirou/gopsutil#572 is roughly the same as this, and No data on one temperature sensor prevents reading any other temp sensor shirou/gopsutil#665 alo deals with a similar problem: what to do if you are iterating over different hardware devices and one of them fails.

   On gopsutil, the favored approach seems to be to return a list of errors on the failed devices and the info for the non-failing devices. This seems harder to translate to Python, given that the usual approach for errors is to raise an exception. Do you think this pattern makes sense? Do you think it can be translated into psutil in some backwards-compatible way?

[giampaolo]

Related: #1977.

[giampaolo]

Similar issue affecting open_files(): #2124 (comment).

[giampaolo]

Related proposal: #2329.