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); } /*