FDInputStream may be leaking fds

[semenzato]

trafficstars

Hi, we're seeing the usbguard daemon running out of fds, and /proc/pid/fd shows that an extra fd is added every time I unplug and replug a keyboard:

lr-x------. 1 root root 64 May  7 12:53 36 -> /sys/devices/pci0000:00/0000:00:14.0/usb3/3-4/descriptors
lr-x------. 1 root root 64 May  7 12:53 37 -> /sys/devices/pci0000:00/0000:00:14.0/usb3/3-4/descriptors
lr-x------. 1 root root 64 May  7 12:53 38 -> /sys/devices/pci0000:00/0000:00:14.0/usb3/3-4/descriptors
lr-x------. 1 root root 64 May  7 12:54 39 -> /sys/devices/pci0000:00/0000:00:14.0/usb3/3-4/descriptors
lr-x------. 1 root root 64 May  7 12:54 40 -> /sys/devices/pci0000:00/0000:00:14.0/usb3/3-4/descriptors
lr-x------. 1 root root 64 May  7 12:54 41 -> /sys/devices/pci0000:00/0000:00:14.0/usb3/3-4/descriptors
lr-x------. 1 root root 64 May  7 12:58 42 -> /sys/devices/pci0000:00/0000:00:14.0/usb3/3-4/descriptors

I took a quick look at the source, and at first glance it seems like the FDInputStream class doesn't have a destructor, which suggests that the descriptors fd may be left open when the class instance is goes out of scope in DeviceBase.cpp::DeviceBase.

Thanks!

[semenzato]

I think this may be enough. I verified that it stops the leak.

diff --git a/src/Common/FDInputStream.hpp b/src/Common/FDInputStream.hpp
index 8528e84..8dc0485 100644
--- a/src/Common/FDInputStream.hpp
+++ b/src/Common/FDInputStream.hpp
@@ -57,6 +57,7 @@ namespace usbguard
   {
   public:
     FDStreamBuf(int fd) : fd_(fd) { }
+    ~FDStreamBuf() { close(fd_); }
 
     std::streamsize xsgetn(char* s, std::streamsize n)
     {

[semenzato]

Would you like me to formally send a patch? BTW we're patching this for ChromeOS at Google. https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/6521787

[hartwork]

@semenzato (I don't have permissions here but) my vote for creating a pull request, yes please. PS: I wonder if the destructor should be virtual. Probably can be argued both ways.