fix bitmap corruption on close_range(), take 2

-----BEGIN PGP SIGNATURE-----
 
 iHUEABYIAB0WIQQqUNBr3gm4hGXdBJlZ7Krx/gZQ6wUCZrFgeQAKCRBZ7Krx/gZQ
 6w4SAP48jL+Vil7ifIXviasoBrQGzf9lbTcOAmWAoaxjSlvlpAEAw4OyPhJUmKHW
 ykB/yqUMCajsrrTQPN5lmc0W5v0nqQ4=
 =G0ms
 -----END PGP SIGNATURE-----

Merge tag 'pull-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs

Pull fd bitmap fix from Al Viro:
 "Fix bitmap corruption on close_range() by cleaning up
  copy_fd_bitmaps()"

* tag 'pull-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs:
  fix bitmap corruption on close_range() with CLOSE_RANGE_UNSHARE
This commit is contained in:
Linus Torvalds 2024-08-12 08:03:28 -07:00
commit a1460e457e
3 changed files with 59 additions and 16 deletions

View File

@ -46,27 +46,23 @@ 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))
#define fdt_words(fdt) ((fdt)->max_fds / BITS_PER_LONG) // words in ->open_fds
/*
* 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.
*/
static void copy_fd_bitmaps(struct fdtable *nfdt, struct fdtable *ofdt,
unsigned int count)
static inline void copy_fd_bitmaps(struct fdtable *nfdt, struct fdtable *ofdt,
unsigned int copy_words)
{
unsigned int cpy, set;
unsigned int nwords = fdt_words(nfdt);
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);
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);
bitmap_copy_and_extend(nfdt->open_fds, ofdt->open_fds,
copy_words * BITS_PER_LONG, nwords * BITS_PER_LONG);
bitmap_copy_and_extend(nfdt->close_on_exec, ofdt->close_on_exec,
copy_words * BITS_PER_LONG, nwords * BITS_PER_LONG);
bitmap_copy_and_extend(nfdt->full_fds_bits, ofdt->full_fds_bits,
copy_words, nwords);
}
/*
@ -84,7 +80,7 @@ static void copy_fdtable(struct fdtable *nfdt, struct fdtable *ofdt)
memcpy(nfdt->fd, ofdt->fd, cpy);
memset((char *)nfdt->fd + cpy, 0, set);
copy_fd_bitmaps(nfdt, ofdt, ofdt->max_fds);
copy_fd_bitmaps(nfdt, ofdt, fdt_words(ofdt));
}
/*
@ -379,7 +375,7 @@ struct files_struct *dup_fd(struct files_struct *oldf, unsigned int max_fds, int
open_files = sane_fdtable_size(old_fdt, max_fds);
}
copy_fd_bitmaps(new_fdt, old_fdt, open_files);
copy_fd_bitmaps(new_fdt, old_fdt, open_files / BITS_PER_LONG);
old_fds = old_fdt->fd;
new_fds = new_fdt->fd;

View File

@ -270,6 +270,18 @@ static inline void bitmap_copy_clear_tail(unsigned long *dst,
dst[nbits / BITS_PER_LONG] &= BITMAP_LAST_WORD_MASK(nbits);
}
static inline void bitmap_copy_and_extend(unsigned long *to,
const unsigned long *from,
unsigned int count, unsigned int size)
{
unsigned int copy = BITS_TO_LONGS(count);
memcpy(to, from, copy * sizeof(long));
if (count % BITS_PER_LONG)
to[copy - 1] &= BITMAP_LAST_WORD_MASK(count);
memset(to + copy, 0, bitmap_size(size) - copy * sizeof(long));
}
/*
* On 32-bit systems bitmaps are represented as u32 arrays internally. On LE64
* machines the order of hi and lo parts of numbers match the bitmap structure.

View File

@ -589,4 +589,39 @@ TEST(close_range_cloexec_unshare_syzbot)
EXPECT_EQ(close(fd3), 0);
}
TEST(close_range_bitmap_corruption)
{
pid_t pid;
int status;
struct __clone_args args = {
.flags = CLONE_FILES,
.exit_signal = SIGCHLD,
};
/* get the first 128 descriptors open */
for (int i = 2; i < 128; i++)
EXPECT_GE(dup2(0, i), 0);
/* get descriptor table shared */
pid = sys_clone3(&args, sizeof(args));
ASSERT_GE(pid, 0);
if (pid == 0) {
/* unshare and truncate descriptor table down to 64 */
if (sys_close_range(64, ~0U, CLOSE_RANGE_UNSHARE))
exit(EXIT_FAILURE);
ASSERT_EQ(fcntl(64, F_GETFD), -1);
/* ... and verify that the range 64..127 is not
stuck "fully used" according to secondary bitmap */
EXPECT_EQ(dup(0), 64)
exit(EXIT_FAILURE);
exit(EXIT_SUCCESS);
}
EXPECT_EQ(waitpid(pid, &status, 0), pid);
EXPECT_EQ(true, WIFEXITED(status));
EXPECT_EQ(0, WEXITSTATUS(status));
}
TEST_HARNESS_MAIN