Kernel: Track allocated FileDescriptionAndFlag elements in each Process

The way the Process::FileDescriptions::allocate() API works today means
that two callers who allocate back to back without associating a
FileDescription with the allocated FD, will receive the same FD and thus
one will stomp over the other.

Naively tracking which FileDescriptions are allocated and moving onto
the next would introduce other bugs however, as now if you "allocate"
a fd and then return early further down the control flow of the syscall
you would leak that fd.

This change modifies this behavior by tracking which descriptions are
allocated and then having an RAII type to "deallocate" the fd if the
association is not setup the end of it's scope.

1 files changed, 47 insertions, 1 deletions

diff --git a/Kernel/Process.h b/Kernel/Process.h
index ec8fa927a3..02f77a3bd6 100644
--- a/Kernel/Process.h
+++ b/Kernel/Process.h
@@ -597,6 +597,19 @@ public:
         operator bool() const { return !!m_description; }
 
         bool is_valid() const { return !m_description.is_null(); }
+        bool is_allocated() const { return m_is_allocated; }
+        void allocate()
+        {
+            VERIFY(!m_is_allocated);
+            VERIFY(!is_valid());
+            m_is_allocated = true;
+        }
+        void deallocate()
+        {
+            VERIFY(m_is_allocated);
+            VERIFY(!is_valid());
+            m_is_allocated = false;
+        }
 
         FileDescription* description() { return m_description; }
         const FileDescription* description() const { return m_description; }
@@ -610,6 +623,7 @@ public:
 
     private:
         RefPtr<FileDescription> m_description;
+        bool m_is_allocated { false };
         u32 m_flags { 0 };
 
         // Note: This is needed so when we generate inodes for ProcFS, we know that
@@ -617,6 +631,7 @@ public:
         InodeIndex m_global_procfs_inode_index;
     };
 
+    class ScopedDescriptionAllocation;
     class FileDescriptions {
         friend class Process;
 
@@ -641,7 +656,7 @@ public:
         void enumerate(Function<void(const FileDescriptionAndFlags&)>) const;
         void change_each(Function<void(FileDescriptionAndFlags&)>);
 
-        KResultOr<int> allocate(int first_candidate_fd = 0);
+        KResultOr<ScopedDescriptionAllocation> allocate(int first_candidate_fd = 0);
         size_t open_count() const;
 
         bool try_resize(size_t size) { return m_fds_metadatas.try_resize(size); }
@@ -668,6 +683,37 @@ public:
         Vector<FileDescriptionAndFlags> m_fds_metadatas;
     };
 
+    class ScopedDescriptionAllocation {
+        AK_MAKE_NONCOPYABLE(ScopedDescriptionAllocation);
+
+    public:
+        ScopedDescriptionAllocation() = default;
+        ScopedDescriptionAllocation(int tracked_fd, FileDescriptionAndFlags* description)
+            : fd(tracked_fd)
+            , m_description(description)
+        {
+        }
+
+        ScopedDescriptionAllocation(ScopedDescriptionAllocation&& other)
+            : fd(other.fd)
+        {
+            // Take over the responsibility of tracking to deallocation.
+            swap(m_description, other.m_description);
+        }
+
+        ~ScopedDescriptionAllocation()
+        {
+            if (m_description && m_description->is_allocated() && !m_description->is_valid()) {
+                m_description->deallocate();
+            }
+        }
+
+        const int fd { -1 };
+
+    private:
+        FileDescriptionAndFlags* m_description { nullptr };
+    };
+
     FileDescriptions& fds() { return m_fds; }
     const FileDescriptions& fds() const { return m_fds; }