Re: [PATCH] fix bitmap corruption on close_range() with CLOSE_RANGE_UNSHARE

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, 3 Aug 2024 at 16:51, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> In fact, I think the full_fds_bits copy in copy_fd_bitmaps() should
> just be entirely rewritten. Even the initial copy shouldn't be done
> with some byte-wise memcpy/memset, since those are all 'unsigned long'
> arrays, and the size is aligned. So it should be done on words, but
> whatever.
>
> And the full_fds_bits case should use our actual bitmap functions.

Something ENTIRELY UNTESTED like this, IOW.

Am I just completely off my rocker? You decide.

                 Linus
 fs/file.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/fs/file.c b/fs/file.c
index a11e59b5d602..ea7f938905f3 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -46,27 +46,36 @@ static void free_fdtable_rcu(struct rcu_head *rcu)
 #define BITBIT_NR(nr)	BITS_TO_LONGS(BITS_TO_LONGS(nr))
 #define BITBIT_SIZE(nr)	(BITBIT_NR(nr) * sizeof(long))
 
+static inline void copy_bitmap_words(unsigned long *dst, unsigned long *src,
+	unsigned int copy, unsigned int clear)
+{
+	memcpy(dst, src, copy * sizeof(unsigned long));
+	memset(dst + copy, 0, clear * sizeof(unsigned long));
+}
+
 /*
  * Copy 'count' fd bits from the old table to the new table and clear the extra
  * space if any.  This does not copy the file pointers.  Called with the files
  * spinlock held for write.
+ *
+ * Note: the fd bitmaps are always full bitmap words (see sane_fdtable_size()),
+ * but the full_fds_bits bitmap is a bitmap of fd bitmap words, not of fds.
  */
 static void copy_fd_bitmaps(struct fdtable *nfdt, struct fdtable *ofdt,
 			    unsigned int count)
 {
-	unsigned int cpy, set;
+	unsigned int copy, clear;
 
-	cpy = count / BITS_PER_BYTE;
-	set = (nfdt->max_fds - count) / BITS_PER_BYTE;
-	memcpy(nfdt->open_fds, ofdt->open_fds, cpy);
-	memset((char *)nfdt->open_fds + cpy, 0, set);
-	memcpy(nfdt->close_on_exec, ofdt->close_on_exec, cpy);
-	memset((char *)nfdt->close_on_exec + cpy, 0, set);
+	copy = count / BITS_PER_LONG;
+	clear = (nfdt->max_fds - count) / BITS_PER_LONG;
 
-	cpy = BITBIT_SIZE(count);
-	set = BITBIT_SIZE(nfdt->max_fds) - cpy;
-	memcpy(nfdt->full_fds_bits, ofdt->full_fds_bits, cpy);
-	memset((char *)nfdt->full_fds_bits + cpy, 0, set);
+	/* Copy the bitmap words */
+	copy_bitmap_words(nfdt->open_fds, ofdt->open_fds, copy, clear);
+	copy_bitmap_words(nfdt->close_on_exec, ofdt->close_on_exec, copy, clear);
+
+	/* Copy the bits for the words */
+	bitmap_copy(nfdt->full_fds_bits, ofdt->full_fds_bits, copy);
+	bitmap_clear(nfdt->full_fds_bits, copy, clear);
 }
 
 /*

[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux